Subzero: Don't use key SSE instructions on potentially unaligned loads.

Subzero: Don't use key SSE instructions on potentially unaligned loads.

The non-mov-like SSE instructions generally require 16-byte aligned memory operands.  The PNaCl bitcode ABI only guarantees 4-byte alignment or less on vector loads and stores.  Subzero maintains stack alignment so stack memory operands are fine.

We handle this by legalizing memory operands into a register wherever there is doubt.

This bug was first discovered on the vector_align scons test.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=4083
BUG= https://code.google.com/p/nativeclient/issues/detail?id=4133
R=jvoung@chromium.org

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

[Jim Stichnoth, 2015-03-22 22:46:51]

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

[JF, 2015-03-23 16:45:21]

As we discussed please file a bug to fix PNaCl to allow vector-sized alignment
as well. We'll need to do a phased rollout of this (translator first, toolchain
second).

This lg to me, but I'm le science dog.

[Jim Stichnoth, 2015-03-23 18:02:51]

On 2015/03/23 16:45:21, JF wrote:
> As we discussed please file a bug to fix PNaCl to allow vector-sized alignment
> as well.

Done - https://code.google.com/p/nativeclient/issues/detail?id=4133

[jvoung (off chromium), 2015-03-23 21:30:35]

lgtm

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

https://codereview.chromium.org/1024253003/diff/1/src/IceTargetLoweringX8632....
src/IceTargetLoweringX8632.cpp:2597: if (llvm::isa<OperandX8632Mem>(Src0RM))
Vs the other places where only Src1RM is legalized here both 0 and 1 are
legalized:

(*) other places already have a _movp(T, Src0RM)
(*) here most cases also have a _movp(T, Src0RM), but sometimes the operands are
swapped and instead _movp(T, Src1RM) is done.
(*) The extra _movp / legalize should be optimized out as redundant.

[Jim Stichnoth, 2015-03-23 22:10:03]

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

https://codereview.chromium.org/1024253003/diff/1/src/IceTargetLoweringX8632....
src/IceTargetLoweringX8632.cpp:2597: if (llvm::isa<OperandX8632Mem>(Src0RM))
On 2015/03/23 21:30:35, jvoung wrote:
> Vs the other places where only Src1RM is legalized here both 0 and 1 are
> legalized:
> 
> (*) other places already have a _movp(T, Src0RM)
> (*) here most cases also have a _movp(T, Src0RM), but sometimes the operands
are
> swapped and instead _movp(T, Src1RM) is done.
> (*) The extra _movp / legalize should be optimized out as redundant.

Done - pushed the appropriate legalization into each specific case, for better
precision.

[Jim Stichnoth, 2015-03-23 22:10:58]

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