Implement ARM32 switch lowering.

Implement ARM32 switch lowering.

A naive implementation of switch lowering using sequential tests for each of the cases.

BUG= none
TEST=switch-opt.ll
R=jvoung@chromium.org

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

[ascull, 2015-06-25 20:45:57]

ascull@google.com changed reviewers:
+ jvoung@chromium.org

[jvoung (off chromium), 2015-06-25 21:07:38]

jvoung@chromium.org changed reviewers:
+ jpp@chromium.org, stichnot@chromium.org

[jvoung (off chromium), 2015-06-25 21:24:50]

+Jim and John in case they wanted to take a look too.

Can you clarify in the CL description that this is the first step (naive
lowering)?

Usually the CL description is formatted like:

"""
One line short description.

Optional further description, which may span multiple lines.
Multiple lines I say!

BUG= xyz
TEST=switch-opt.ll
"""

https://codereview.chromium.org/1213593002/diff/1/src/IceTargetLoweringARM32.cpp
File src/IceTargetLoweringARM32.cpp (right):

https://codereview.chromium.org/1213593002/diff/1/src/IceTargetLoweringARM32....
src/IceTargetLoweringARM32.cpp:1933: Variable *Src0Lo =
legalizeToVar(loOperand(Src0));
Hmm maybe why the X86 version had "legalize(Src0);" before this, is if Src0 is
undef.

https://codereview.chromium.org/1213593002/diff/1/src/IceTargetLoweringARM32....
src/IceTargetLoweringARM32.cpp:1941: _cmp(Src0Hi, ValueHi, CondARM32::EQ);
nice, use that predication =)

https://codereview.chromium.org/1213593002/diff/1/tests_lit/llvm2ice_tests/sw...
File tests_lit/llvm2ice_tests/switch-opt.ll (right):

https://codereview.chromium.org/1213593002/diff/1/tests_lit/llvm2ice_tests/sw...
tests_lit/llvm2ice_tests/switch-opt.ll:8: ; RUN:   --disassemble --target arm32
-i %s --args -Om1 --skip-unimplemented \
Can you leave a "TODO(jvoung): to switch this to O2, once the phi assignments is
done for ARM"?

[jvoung (off chromium), 2015-06-29 16:51:47]

https://codereview.chromium.org/1213593002/diff/1/src/IceTargetLoweringARM32.cpp
File src/IceTargetLoweringARM32.cpp (right):

https://codereview.chromium.org/1213593002/diff/1/src/IceTargetLoweringARM32....
src/IceTargetLoweringARM32.cpp:1933: Variable *Src0Lo =
legalizeToVar(loOperand(Src0));
On 2015/06/25 21:24:50, jvoung wrote:
> Hmm maybe why the X86 version had "legalize(Src0);" before this, is if Src0 is
> undef.


Well, there are other places in the ARM code which might not have done
legalize(X) prior to loOperand(X) or hiOperand(X), which I can audit and fix
later, if you want to leave a "TODO(jvoung): handle and test undef".


The problem is that legalize(X) for ARM is unlike the X86 version, where if X is
a large immediate then that is not considered legal as-is/pass-through. It only
checks for 32-bit constants that way, and does something silly for 64-bit
constants: it will try to move any 64-bit constant into a register. However,
with 64-bit constants that blows up because there aren't 64-bit registers. I.e.,
legalize() is really expecting loOperand/hiOperand to split the 64-bit thing
first, but loOperand() is expecting legalize() to handle undef.


If the motivation for calling legalize() prior to loOperand() is to handle
undef, perhaps we could just make loOperand()/hiOperand() also aware of undef.
The downside is that undef-legalization-knowledge leaks to two other methods (in
case we wanted to change the behavior from initializing to 0 to not initializing
at all, which is cheaper but non-deterministic). Either that or we split out the
undef legalization into a separate method "legalizeUndef" and call that for the
purposes of legalizing undef (instead of the more general legalize()), prior to
calling loOperand/hiOperand. This is assuming that the x86 cases where
legalize() is called prior to lo/hiOperand are only for legalizing undef and
nothing else.

[ascull, 2015-06-29 17:00:03]

https://codereview.chromium.org/1213593002/diff/1/src/IceTargetLoweringARM32.cpp
File src/IceTargetLoweringARM32.cpp (right):

https://codereview.chromium.org/1213593002/diff/1/src/IceTargetLoweringARM32....
src/IceTargetLoweringARM32.cpp:1933: Variable *Src0Lo =
legalizeToVar(loOperand(Src0));
On 2015/06/29 16:51:47, jvoung wrote:
> On 2015/06/25 21:24:50, jvoung wrote:
> > Hmm maybe why the X86 version had "legalize(Src0);" before this, is if Src0
is
> > undef.
> 
> 
> Well, there are other places in the ARM code which might not have done
> legalize(X) prior to loOperand(X) or hiOperand(X), which I can audit and fix
> later, if you want to leave a "TODO(jvoung): handle and test undef".
> 
> 
> The problem is that legalize(X) for ARM is unlike the X86 version, where if X
is
> a large immediate then that is not considered legal as-is/pass-through. It
only
> checks for 32-bit constants that way, and does something silly for 64-bit
> constants: it will try to move any 64-bit constant into a register. However,
> with 64-bit constants that blows up because there aren't 64-bit registers.
I.e.,
> legalize() is really expecting loOperand/hiOperand to split the 64-bit thing
> first, but loOperand() is expecting legalize() to handle undef.
> 
> 
> If the motivation for calling legalize() prior to loOperand() is to handle
> undef, perhaps we could just make loOperand()/hiOperand() also aware of undef.
> The downside is that undef-legalization-knowledge leaks to two other methods
(in
> case we wanted to change the behavior from initializing to 0 to not
initializing
> at all, which is cheaper but non-deterministic). Either that or we split out
the
> undef legalization into a separate method "legalizeUndef" and call that for
the
> purposes of legalizing undef (instead of the more general legalize()), prior
to
> calling loOperand/hiOperand. This is assuming that the x86 cases where
> legalize() is called prior to lo/hiOperand are only for legalizing undef and
> nothing else.

Done.

https://codereview.chromium.org/1213593002/diff/1/tests_lit/llvm2ice_tests/sw...
File tests_lit/llvm2ice_tests/switch-opt.ll (right):

https://codereview.chromium.org/1213593002/diff/1/tests_lit/llvm2ice_tests/sw...
tests_lit/llvm2ice_tests/switch-opt.ll:8: ; RUN:   --disassemble --target arm32
-i %s --args -Om1 --skip-unimplemented \
On 2015/06/25 21:24:50, jvoung wrote:
> Can you leave a "TODO(jvoung): to switch this to O2, once the phi assignments
is
> done for ARM"?

Done.

[jvoung (off chromium), 2015-06-29 18:19:12]

lgtm

[ascull, 2015-06-29 18:21:22]

Committed patchset #2 (id:20001) manually as
fdc54db71f2478ca17128f82e781aa605ab5a2e4 (presubmit successful).