Subzero: Fix some legalization issues involving immediates.

Subzero: Fix some legalization issues involving immediates.

Some lowering sequences were incorrectly allowing immediate operands in native instructions.  This includes 32-bit icmp, 64-bit icmp, select, switch, and 64-bit mul.

BUG= none
R=jvoung@chromium.org

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

[Jim Stichnoth, 2014-08-26 20:14:56]

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

[jvoung (off chromium), 2014-08-26 21:59:46]

Otherwise LGTM

https://codereview.chromium.org/511543002/diff/40001/src/IceTargetLoweringX86...
File src/IceTargetLoweringX8632.cpp (right):

https://codereview.chromium.org/511543002/diff/40001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:2629: Operand *Src0LoRM =
legalize(loOperand(Src0), Legal_All & ~Legal_Imm);
There is a difference between Legal_All & ~Legal_Imm and Legal_Reg | Legal_Mem,
but I wasn't sure if it made a difference here. Just wanted to check. Otherwise,
more places seem to just use "Legal_Reg | Legal_Mem"

https://codereview.chromium.org/511543002/diff/40001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:2661: Operand *Src0New = legalize(
Maybe it could be call "...RM" instead of "...New" ?

https://codereview.chromium.org/511543002/diff/40001/tests_lit/llvm2ice_tests...
File tests_lit/llvm2ice_tests/bool-opt.ll (right):

https://codereview.chromium.org/511543002/diff/40001/tests_lit/llvm2ice_tests...
tests_lit/llvm2ice_tests/bool-opt.ll:27: ; CHECK-NOT: cmp {{[0-9]+}}, {{[0-9]+}}
Do you need the second pair of ints after the comma (the other tests just check
the first src)

[Jim Stichnoth, 2014-08-27 05:16:33]

Committed patchset #4 manually as ef8cf0e (presubmit successful).

[Jim Stichnoth, 2014-08-27 06:19:47]

https://codereview.chromium.org/511543002/diff/40001/src/IceTargetLoweringX86...
File src/IceTargetLoweringX8632.cpp (right):

https://codereview.chromium.org/511543002/diff/40001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:2629: Operand *Src0LoRM =
legalize(loOperand(Src0), Legal_All & ~Legal_Imm);
On 2014/08/26 21:59:46, jvoung wrote:
> There is a difference between Legal_All & ~Legal_Imm and Legal_Reg |
Legal_Mem,
> but I wasn't sure if it made a difference here. Just wanted to check.
Otherwise,
> more places seem to just use "Legal_Reg | Legal_Mem"

Good point, I forgot about the Legal_Reloc hack.  I'll change all those to the
usual whitelist.

https://codereview.chromium.org/511543002/diff/40001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:2661: Operand *Src0New = legalize(
On 2014/08/26 21:59:45, jvoung wrote:
> Maybe it could be call "...RM" instead of "...New" ?

Done.

https://codereview.chromium.org/511543002/diff/40001/tests_lit/llvm2ice_tests...
File tests_lit/llvm2ice_tests/bool-opt.ll (right):

https://codereview.chromium.org/511543002/diff/40001/tests_lit/llvm2ice_tests...
tests_lit/llvm2ice_tests/bool-opt.ll:27: ; CHECK-NOT: cmp {{[0-9]+}}, {{[0-9]+}}
On 2014/08/26 21:59:46, jvoung wrote:
> Do you need the second pair of ints after the comma (the other tests just
check
> the first src)

Done.