Subzero: Fix some issues related to legalization and undef handling.

Subzero: Fix some issues related to legalization and undef handling.

1. Much of the lowering code for vector operations was not properly
checking that the input operand was in a register or memory. This
problem could be exhibited by passing undef values as inputs.

=> Change the vector legalization code to legalize input operands to
register or memory before producing instructions that use the
operands. Also, append a suffix to the variable names in the vector
legalization code to clarify the legalization status of the values.

2. Undef values should never be emitted directly. Rather, they should
have been appropriately legalized to a zero value.

=> To enforce this, make ConstantUndef::emit() issue an error
message. Do this in the x86 backend, as other backends may decide to
treat undef values differently.

3. The regalloc_evict_non_overlap test was loading from an undef
pointer. Subzero was not handling this correctly (the undef pointer was
being emitted without being legalized), but it does not have to handle
this case since PNaCl IR disallows undef pointers.

=> Fix the regalloc_evict_non_overlap test to use an inttoptr instead of
directly loading from the undef pointer. Also, add an assert in
IceTargetLoweringX8632::FormMemoryOperand() to make sure that undef
pointers are never encountered.

BUG=none
R=jvoung@chromium.org, stichnot@chromium.org

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

[Jim Stichnoth, 2014-07-31 00:52:43]

lgtm

https://codereview.chromium.org/432613002/diff/1/src/IceTargetLoweringX8632.cpp
File src/IceTargetLoweringX8632.cpp (right):

https://codereview.chromium.org/432613002/diff/1/src/IceTargetLoweringX8632.c...
src/IceTargetLoweringX8632.cpp:4073: (void)Ctx;
Can you declare the function as "void ConstantUndef::emit(GlobalContext *)" and
omit this statement?

[jvoung (off chromium), 2014-07-31 15:42:45]

otherwise LGTM too

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

https://codereview.chromium.org/432613002/diff/20001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:2269: Operand *Src0RM = legalizeToVar(Src0,
Legal_Reg | Legal_Mem);
Shouldn't this be legalize() instead of legalizeToVar(), since you're using the
flags Legal_Reg | Legal_Mem ?

[wala, 2014-07-31 16:06:04]

https://codereview.chromium.org/432613002/diff/1/src/IceTargetLoweringX8632.cpp
File src/IceTargetLoweringX8632.cpp (right):

https://codereview.chromium.org/432613002/diff/1/src/IceTargetLoweringX8632.c...
src/IceTargetLoweringX8632.cpp:4073: (void)Ctx;
On 2014/07/31 00:52:42, stichnot wrote:
> Can you declare the function as "void ConstantUndef::emit(GlobalContext *)"
and
> omit this statement?

Done.

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

https://codereview.chromium.org/432613002/diff/20001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:2269: Operand *Src0RM = legalizeToVar(Src0,
Legal_Reg | Legal_Mem);
On 2014/07/31 15:42:44, jvoung wrote:
> Shouldn't this be legalize() instead of legalizeToVar(), since you're using
the
> flags Legal_Reg | Legal_Mem ?

Yes.

Done.

Thanks for catching that!

[wala, 2014-07-31 16:06:21]

Committed patchset #3 manually as re377767 (presubmit successful).