Subzero ARM: lowerArguments (GPR), basic legalize(), and lowerRet(i32, i64).

Subzero ARM: lowerArguments (GPR), basic legalize(), and lowerRet(i32, i64).

Adds basic assignment instructions, mov, movn, movw, movt,
ldr, etc. in order to copy around the first few integer
(i32, i64) arguments out of r0 - r3, and then return then.

The "mov" instruction is a bit special and can actually
be a "str" when the dest is a stack slot.

Model the Memory operand types, and the "flexible Operand2".
Add a few tests demonstrating the flexibility of the
immediate encoding.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=4076
R=stichnot@chromium.org

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=b3401d27a236b81393092b4d571df473b5bba64b

[jvoung (off chromium), 2015-05-14 22:22:08]

jvoung@chromium.org changed reviewers:
+ kschimpf@google.com, stichnot@chromium.org

[jvoung (off chromium), 2015-05-14 22:42:07]

ty in advance ヽ(⌐■_■)ノ♪♬

https://codereview.chromium.org/1127963004/diff/40001/src/IceInstARM32.cpp
File src/IceInstARM32.cpp (right):

https://codereview.chromium.org/1127963004/diff/40001/src/IceInstARM32.cpp#ne...
src/IceInstARM32.cpp:112: // Avoid the more expensive test for frequent small
immediate values.
Sort of borrowed from Dart and V8. They are both a bit different from each
other. V8 used to have undef behavior from doing X << 32, so I guess beware.

https://codereview.chromium.org/1127963004/diff/40001/src/IceInstARM32.h
File src/IceInstARM32.h (right):

https://codereview.chromium.org/1127963004/diff/40001/src/IceInstARM32.h#newc...
src/IceInstARM32.h:92: static OperandARM32Mem *create(Cfg *Func, Type Ty,
Variable *Base,
Not really using these yet (or fancy versions of them), since address mode
optimization isn't done, but thought I'd model them for now to see what
legalize(), etc. need to be aware of.

https://codereview.chromium.org/1127963004/diff/40001/src/IceInstARM32.h#newc...
src/IceInstARM32.h:210: // Register with immediate/reg shift amount and shift
operation.
Not really using this, but technically ARM could support this if we wanted to
use it for some optimization.

https://codereview.chromium.org/1127963004/diff/40001/src/IceTargetLoweringAR...
File src/IceTargetLoweringARM32.cpp (right):

https://codereview.chromium.org/1127963004/diff/40001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.cpp:571: // _mov(Dest, T);
re: commented out code -- I've been playing around with this in a separate CL:
https://codereview.chromium.org/1044423006/, and can get some basic add
instructions lowered.

Originally, I thought of requiring _mov()'s second operand to be
OperandARM32Flex, but I left it as a more general "Operand" to cover Variables.

https://codereview.chromium.org/1127963004/diff/40001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.cpp:1146: inferTwoAddress();
Needed this for "movt" which I modeled as two-address.

https://codereview.chromium.org/1127963004/diff/40001/src/IceUtils.h
File src/IceUtils.h (right):

https://codereview.chromium.org/1127963004/diff/40001/src/IceUtils.h#newcode82
src/IceUtils.h:82: // Precondition: 0 <= shift < 32
borrowed from V8 basically

[Jim Stichnoth, 2015-05-16 00:10:34]

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp
File src/IceInstARM32.cpp (right):

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp#ne...
src/IceInstARM32.cpp:61: Variable *Dest = Inst->getDest();
The x86 version asserts that Dest==Inst->getSrc(0).  Is that valid/useful here?

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp#ne...
src/IceInstARM32.cpp:76: Vars = Func->allocateArrayOf<Variable *>(1);
Could you just set Vars=&Base and avoid the allocation, since there's only one
var?

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp#ne...
src/IceInstARM32.cpp:113: if (Immediate <= (0xFF)) {
Remove parens around 0xFF?

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp#ne...
src/IceInstARM32.cpp:181: Str << "\t"
hmm, surprised this multi-line formatting survived clang-format...

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp#ne...
src/IceInstARM32.cpp:185: Str << ", ";
Just a sanity check - this "mov dest, src" syntax resembles the Intel syntax
which got us in trouble with llvm-mc and forced us to rework it to the AT&T
syntax.  Is there any similar issue with ARM?

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp#ne...
src/IceInstARM32.cpp:194: Str << ", ";
but this instruction uses "str src, dest"?  (╯°□°)╯︵xɐʇuʎs ɯɹ∀

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp#ne...
src/IceInstARM32.cpp:212: Str << "\tldr" << getWidthString(Ty) << "\t";
Split "\t" and "ldr"?

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.h
File src/IceInstARM32.h (right):

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.h#newc...
src/IceInstARM32.h:37: k__Start = Operand::kTarget,
It looks like InstKind and OperandKind value spaces are shared across different
targets (as opposed to some still-hypothetical way of making them unique across
the chosen set of targets).  This seems fine, since it should never be possible
to create an object from one target and pass it to another target.

It would probably be good to briefly clarify this where Inst::Target and
Operand::kTarget are defined.

https://codereview.chromium.org/1127963004/diff/60001/tests_lit/llvm2ice_test...
File tests_lit/llvm2ice_tests/return_immediates.ll (right):

https://codereview.chromium.org/1127963004/diff/60001/tests_lit/llvm2ice_test...
tests_lit/llvm2ice_tests/return_immediates.ll:1: ; Simple test that returns
various immediates. For fix-width instruction
fixed-width ?

[jvoung (off chromium), 2015-05-17 18:06:30]

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp
File src/IceInstARM32.cpp (right):

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp#ne...
src/IceInstARM32.cpp:61: Variable *Dest = Inst->getDest();
On 2015/05/16 00:10:34, stichnot wrote:
> The x86 version asserts that Dest==Inst->getSrc(0).  Is that valid/useful
here?

Good catch -- should be valid/useful here too.

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp#ne...
src/IceInstARM32.cpp:76: Vars = Func->allocateArrayOf<Variable *>(1);
On 2015/05/16 00:10:34, stichnot wrote:
> Could you just set Vars=&Base and avoid the allocation, since there's only one
> var?

Done.

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp#ne...
src/IceInstARM32.cpp:113: if (Immediate <= (0xFF)) {
On 2015/05/16 00:10:34, stichnot wrote:
> Remove parens around 0xFF?

Done.

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp#ne...
src/IceInstARM32.cpp:181: Str << "\t"
On 2015/05/16 00:10:34, stichnot wrote:
> hmm, surprised this multi-line formatting survived clang-format...

Yeah clang-format was the one to make these multi-line =(

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp#ne...
src/IceInstARM32.cpp:185: Str << ", ";
On 2015/05/16 00:10:34, stichnot wrote:
> Just a sanity check - this "mov dest, src" syntax resembles the Intel syntax
> which got us in trouble with llvm-mc and forced us to rework it to the AT&T
> syntax.  Is there any similar issue with ARM?

Hmm was it the "mov dest, src" syntax or something to do with
ConstantRelocatable prefixes (e.g., "ax" without a prefix like "$ax") that was
the problem? I think we're okay with ConstantRelocatables because the address is
usually materialized with prefixes #:lower16:Foo / #:upper16:Foo

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp#ne...
src/IceInstARM32.cpp:194: Str << ", ";
On 2015/05/16 00:10:34, stichnot wrote:
> but this instruction uses "str src, dest"?  (╯°□°)╯︵xɐʇuʎs ɯɹ∀

Yep ⊙︿⊙

For more fun, there's also a "stm" (store multiple) which looks like "str
dest_base, {src0, src1, src2, ...}"

I guess "Dest" is more like "memory operand" so load and store are always going
to have the memory operand on the right.

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp#ne...
src/IceInstARM32.cpp:212: Str << "\tldr" << getWidthString(Ty) << "\t";
On 2015/05/16 00:10:34, stichnot wrote:
> Split "\t" and "ldr"?

Done.

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.h
File src/IceInstARM32.h (right):

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.h#newc...
src/IceInstARM32.h:37: k__Start = Operand::kTarget,
On 2015/05/16 00:10:34, stichnot wrote:
> It looks like InstKind and OperandKind value spaces are shared across
different
> targets (as opposed to some still-hypothetical way of making them unique
across
> the chosen set of targets).  This seems fine, since it should never be
possible
> to create an object from one target and pass it to another target.
> 
> It would probably be good to briefly clarify this where Inst::Target and
> Operand::kTarget are defined.

Done.

https://codereview.chromium.org/1127963004/diff/60001/tests_lit/llvm2ice_test...
File tests_lit/llvm2ice_tests/return_immediates.ll (right):

https://codereview.chromium.org/1127963004/diff/60001/tests_lit/llvm2ice_test...
tests_lit/llvm2ice_tests/return_immediates.ll:1: ; Simple test that returns
various immediates. For fix-width instruction
On 2015/05/16 00:10:34, stichnot wrote:
> fixed-width ?

Fixed.

[Jim Stichnoth, 2015-05-18 02:10:52]

lgtm

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp
File src/IceInstARM32.cpp (right):

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp#ne...
src/IceInstARM32.cpp:185: Str << ", ";
On 2015/05/17 18:06:30, jvoung wrote:
> On 2015/05/16 00:10:34, stichnot wrote:
> > Just a sanity check - this "mov dest, src" syntax resembles the Intel syntax
> > which got us in trouble with llvm-mc and forced us to rework it to the AT&T
> > syntax.  Is there any similar issue with ARM?
> 
> Hmm was it the "mov dest, src" syntax or something to do with
> ConstantRelocatable prefixes (e.g., "ax" without a prefix like "$ax") that was
> the problem? I think we're okay with ConstantRelocatables because the address
is
> usually materialized with prefixes #:lower16:Foo / #:upper16:Foo

I didn't mean the operand order was a problem - just an artifact of the overall
problematic Intel syntax.

Hopefully ARM has just one syntax, in which case I would trust LLVM's parser.

https://codereview.chromium.org/1127963004/diff/120001/src/IceTargetLoweringA...
File src/IceTargetLoweringARM32.cpp (right):

https://codereview.chromium.org/1127963004/diff/120001/src/IceTargetLoweringA...
src/IceTargetLoweringARM32.cpp:1158: void TargetARM32::emit(const ConstantFloat
*C) const {
btw this gives a warning about unused parameter C

https://codereview.chromium.org/1127963004/diff/120001/src/IceTargetLoweringA...
src/IceTargetLoweringARM32.cpp:1162: void TargetARM32::emit(const ConstantDouble
*C) const {
this too

[jvoung (off chromium), 2015-05-18 16:26:47]

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp
File src/IceInstARM32.cpp (right):

https://codereview.chromium.org/1127963004/diff/60001/src/IceInstARM32.cpp#ne...
src/IceInstARM32.cpp:185: Str << ", ";
On 2015/05/18 02:10:52, stichnot wrote:
> On 2015/05/17 18:06:30, jvoung wrote:
> > On 2015/05/16 00:10:34, stichnot wrote:
> > > Just a sanity check - this "mov dest, src" syntax resembles the Intel
syntax
> > > which got us in trouble with llvm-mc and forced us to rework it to the
AT&T
> > > syntax.  Is there any similar issue with ARM?
> > 
> > Hmm was it the "mov dest, src" syntax or something to do with
> > ConstantRelocatable prefixes (e.g., "ax" without a prefix like "$ax") that
was
> > the problem? I think we're okay with ConstantRelocatables because the
address
> is
> > usually materialized with prefixes #:lower16:Foo / #:upper16:Foo
> 
> I didn't mean the operand order was a problem - just an artifact of the
overall
> problematic Intel syntax.
> 
> Hopefully ARM has just one syntax, in which case I would trust LLVM's parser.

Ah right. There are probably older syntaxes, but I haven't encountered any huge
differences lately. There is ".syntax unified", which is supposed to allow you
to write ARM and Thumb instructions with the same syntax. There are some
optional things like "#" before immediates, so the small variations from things
being technically optional might be where LLVM's parser falls short. Not sure
about directives either.

https://codereview.chromium.org/1127963004/diff/120001/src/IceTargetLoweringA...
File src/IceTargetLoweringARM32.cpp (right):

https://codereview.chromium.org/1127963004/diff/120001/src/IceTargetLoweringA...
src/IceTargetLoweringARM32.cpp:1158: void TargetARM32::emit(const ConstantFloat
*C) const {
On 2015/05/18 02:10:52, stichnot wrote:
> btw this gives a warning about unused parameter C

Done.

https://codereview.chromium.org/1127963004/diff/120001/src/IceTargetLoweringA...
src/IceTargetLoweringARM32.cpp:1162: void TargetARM32::emit(const ConstantDouble
*C) const {
On 2015/05/18 02:10:52, stichnot wrote:
> this too

Done.

[jvoung (off chromium), 2015-05-18 16:38:27]

Committed patchset #8 (id:140001) manually as
b3401d27a236b81393092b4d571df473b5bba64b (presubmit successful).