Subzero: Use scalar arithmetic when no vector instruction exists.

Subzero: Use scalar arithmetic when no vector instruction exists.

Implement scalarizeArithmetic() which extracts the components of the
input vectors, performs the operation with scalar instructions, and
builds the output vector component by component.

Fix the lowering of sdiv and srem.  These were previously emitting a
wrong instruction (cdq) for i8 and i16 inputs (needing cbw, cwd).

In the test_arith crosstest, mask the inputs to vector shift
operations to ensure that the shifts are in range.  Otherwise the
Subzero output is not identical to the llc output in some (undefined)
cases.

BUG=none
R=stichnot@chromium.org

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

[Jim Stichnoth, 2014-08-07 19:58:10]

https://codereview.chromium.org/443203003/diff/20001/crosstest/test_arith_mai...
File crosstest/test_arith_main.cpp (left):

https://codereview.chromium.org/443203003/diff/20001/crosstest/test_arith_mai...
crosstest/test_arith_main.cpp:364: // Subzero helpers
Cool!

https://codereview.chromium.org/443203003/diff/20001/crosstest/test_arith_mai...
File crosstest/test_arith_main.cpp (right):

https://codereview.chromium.org/443203003/diff/20001/crosstest/test_arith_mai...
crosstest/test_arith_main.cpp:207: Element1 &= Mask;
You only want to mask Element2, right?

https://codereview.chromium.org/443203003/diff/20001/src/IceInstX8632.cpp
File src/IceInstX8632.cpp (right):

https://codereview.chromium.org/443203003/diff/20001/src/IceInstX8632.cpp#new...
src/IceInstX8632.cpp:727: assert(llvm::isa<Variable>(Dest));
This is redundant, since Dest is types as a Variable (unless you're asserting
that Dest is non-NULL).

https://codereview.chromium.org/443203003/diff/20001/src/IceInstX8632.cpp#new...
src/IceInstX8632.cpp:735: assert(llvm::cast<Variable>(Dest)->getRegNum() ==
TargetX8632::Reg_eax);
Don't need to cast Dest to Variable, here and below.

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

https://codereview.chromium.org/443203003/diff/20001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:1299: scalarizeArithmetic(InstArithmetic::Mul,
Dest, Src0, Src1);
Consider using Inst->getOp() instead of ::Mul?

https://codereview.chromium.org/443203003/diff/20001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:1336: scalarizeArithmetic(InstArithmetic::Frem,
Dest, Src0, Src1);
Consider using Inst->getOp() instead of ::Frem?

https://codereview.chromium.org/443203003/diff/20001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:3699: void
TargetX8632::scalarizeArithmetic(InstArithmetic::OpKind K, Variable *Dest,
Name the arg "Kind", or something else more descriptive than "K"?

[wala, 2014-08-07 20:30:36]

https://codereview.chromium.org/443203003/diff/20001/crosstest/test_arith_mai...
File crosstest/test_arith_main.cpp (left):

https://codereview.chromium.org/443203003/diff/20001/crosstest/test_arith_mai...
crosstest/test_arith_main.cpp:364: // Subzero helpers
On 2014/08/07 19:58:10, stichnot wrote:
> Cool!

Thanks!

https://codereview.chromium.org/443203003/diff/20001/crosstest/test_arith_mai...
File crosstest/test_arith_main.cpp (right):

https://codereview.chromium.org/443203003/diff/20001/crosstest/test_arith_mai...
crosstest/test_arith_main.cpp:207: Element1 &= Mask;
On 2014/08/07 19:58:10, stichnot wrote:
> You only want to mask Element2, right?

Woops, done.

https://codereview.chromium.org/443203003/diff/20001/src/IceInstX8632.cpp
File src/IceInstX8632.cpp (right):

https://codereview.chromium.org/443203003/diff/20001/src/IceInstX8632.cpp#new...
src/IceInstX8632.cpp:727: assert(llvm::isa<Variable>(Dest));
On 2014/08/07 19:58:10, stichnot wrote:
> This is redundant, since Dest is types as a Variable (unless you're asserting
> that Dest is non-NULL).

Woops, done.

https://codereview.chromium.org/443203003/diff/20001/src/IceInstX8632.cpp#new...
src/IceInstX8632.cpp:735: assert(llvm::cast<Variable>(Dest)->getRegNum() ==
TargetX8632::Reg_eax);
On 2014/08/07 19:58:10, stichnot wrote:
> Don't need to cast Dest to Variable, here and below.

Done.

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

https://codereview.chromium.org/443203003/diff/20001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:1299: scalarizeArithmetic(InstArithmetic::Mul,
Dest, Src0, Src1);
On 2014/08/07 19:58:10, stichnot wrote:
> Consider using Inst->getOp() instead of ::Mul?

Done.

https://codereview.chromium.org/443203003/diff/20001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:1336: scalarizeArithmetic(InstArithmetic::Frem,
Dest, Src0, Src1);
On 2014/08/07 19:58:10, stichnot wrote:
> Consider using Inst->getOp() instead of ::Frem?

Done.

https://codereview.chromium.org/443203003/diff/20001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:3699: void
TargetX8632::scalarizeArithmetic(InstArithmetic::OpKind K, Variable *Dest,
On 2014/08/07 19:58:10, stichnot wrote:
> Name the arg "Kind", or something else more descriptive than "K"?

Done.

[Jim Stichnoth, 2014-08-07 20:35:17]

lgtm

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

https://codereview.chromium.org/443203003/diff/40001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:3725: // TODO: Use postLower() in -Om1 mode to
avoid buildup of infinite
TODO(stichnot) :)

[wala, 2014-08-07 20:47:22]

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

https://codereview.chromium.org/443203003/diff/40001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:3725: // TODO: Use postLower() in -Om1 mode to
avoid buildup of infinite
On 2014/08/07 20:35:17, stichnot wrote:
> TODO(stichnot) :)

Done.

[wala, 2014-08-07 20:47:34]

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