Subzero: Fix a register allocation issue for "advanced phi lowering".

Subzero: Fix a register allocation issue for "advanced phi lowering".

When the advanced phi lowering handles a phi arg that is Undef, it lowers it to an assignment of a constant zero (or vector of zeroes).  The ad-hoc register allocation was missing the fact that a vector of zeroes is done with "pxor %reg, %reg".  This resulted in a pxor instruction with invalid addressing modes at emission time.

The fix is to tell legalize() to use the dest physical register if dest has one; and if dest lacks a register, take the path where it actually does the ad-hoc register allocation as though the source operand were a memory operand.

Tests are added for these vector undef phi args, and for scalar undef phi args as well for good measure.

BUG= none
R=jvoung@chromium.org

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

[Jim Stichnoth, 2015-03-02 02:43:13]

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

[jvoung (off chromium), 2015-03-02 19:27:56]

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

https://codereview.chromium.org/969703002/diff/1/src/IceTargetLoweringX8632.c...
src/IceTargetLoweringX8632.cpp:1694: // an immediate, otherwise RI can be
anything.
The "can be anything" comment seems to go with Legal_All, but it's no longer
Legal_All?

[Jim Stichnoth, 2015-03-02 23:33:53]

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

https://codereview.chromium.org/969703002/diff/1/src/IceTargetLoweringX8632.c...
src/IceTargetLoweringX8632.cpp:1694: // an immediate, otherwise RI can be
anything.
On 2015/03/02 19:27:56, jvoung wrote:
> The "can be anything" comment seems to go with Legal_All, but it's no longer
> Legal_All?

Done.

[jvoung (off chromium), 2015-03-03 00:46:25]

lgtm

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

https://codereview.chromium.org/969703002/diff/20001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:1694: Operand *RI;
Besides Phi, it does seem like this could affect load/store/bitcast also since
this is the generic lowerAssign(), so I wasn't too sure of the effect that this
would have. I guess worst case is, you get a dead assignment that's optimized
out.

[Jim Stichnoth, 2015-03-03 00:53:50]

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

https://codereview.chromium.org/969703002/diff/20001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:1694: Operand *RI;
On 2015/03/03 00:46:25, jvoung wrote:
> Besides Phi, it does seem like this could affect load/store/bitcast also since
> this is the generic lowerAssign(), so I wasn't too sure of the effect that
this
> would have. I guess worst case is, you get a dead assignment that's optimized
> out.

For what it's worth, the asm code was the same before and after this change,
across Spec2k.  Yeah, the result is that the mov/movp below will be eliminated
as a redundant assignment.  However, assignments with precolored Dest operands
are rare: argument registers, return values, cl shift register, eax/edx
registers for mul/div, etc.

[Jim Stichnoth, 2015-03-03 00:56:34]

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