Lower vector floating point arithmetic operations.

Lower vector floating point arithmetic operations.

This adds lowering code for fadd, fsub, fmul, fdiv, and frem. frem, having no native x86 counterpart, is implemented by making a helper call.

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=8d1072e

[Jim Stichnoth, 2014-07-11 19:56:16]

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

https://codereview.chromium.org/389653002/diff/40001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:1161: Variable *T = makeReg(Dest->getType());
Since you're adding 4 new instances of this pattern, I think it would be nice to
modify the _movp() implementation to "do the right thing" when a NULL Dest arg
is passed, similar to the current _mov() implementation.  Perhaps the same for
_movq() even though there is only one use where it would benefit.

[wala, 2014-07-11 21:59:04]

On 2014/07/11 19:56:16, stichnot wrote:
>
https://codereview.chromium.org/389653002/diff/40001/src/IceTargetLoweringX86...
> File src/IceTargetLoweringX8632.cpp (right):
> 
>
https://codereview.chromium.org/389653002/diff/40001/src/IceTargetLoweringX86...
> src/IceTargetLoweringX8632.cpp:1161: Variable *T = makeReg(Dest->getType());
> Since you're adding 4 new instances of this pattern, I think it would be nice
to
> modify the _movp() implementation to "do the right thing" when a NULL Dest arg
> is passed, similar to the current _mov() implementation.  Perhaps the same for
> _movq() even though there is only one use where it would benefit.

I'm somewhat wary of making our instruction set too intelligent.

_movq is used in a limited context, namely doing transfers between xmm registers
and xmm/mem64. It is not really a general-purpose copy operation like _mov. So
it makes sense for someone who uses _movq to explicitly think about the type of
the destination rather than having _movq try to figure it out.

In the same way, _movp has a more restricted context than _mov. The family of
instructions that _movp represents cannot handle immediates or scalar registers.
I'm not sure if it's appropriate the have _movp try to handle the case when the
source is an immediate or a scalar register (and anyway the legalization code is
incomplete when the destination is an XMM register). On the other hand, if I do
not handle these, then _movp will accept a different set of source operands than
_mov, which might end up being quite confusing.

[Jim Stichnoth, 2014-07-11 22:19:08]

On 2014/07/11 21:59:04, wala wrote:
> On 2014/07/11 19:56:16, stichnot wrote:
> >
>
https://codereview.chromium.org/389653002/diff/40001/src/IceTargetLoweringX86...
> > File src/IceTargetLoweringX8632.cpp (right):
> > 
> >
>
https://codereview.chromium.org/389653002/diff/40001/src/IceTargetLoweringX86...
> > src/IceTargetLoweringX8632.cpp:1161: Variable *T = makeReg(Dest->getType());
> > Since you're adding 4 new instances of this pattern, I think it would be
nice
> to
> > modify the _movp() implementation to "do the right thing" when a NULL Dest
arg
> > is passed, similar to the current _mov() implementation.  Perhaps the same
for
> > _movq() even though there is only one use where it would benefit.
> 
> I'm somewhat wary of making our instruction set too intelligent.
> 
> _movq is used in a limited context, namely doing transfers between xmm
registers
> and xmm/mem64. It is not really a general-purpose copy operation like _mov. So
> it makes sense for someone who uses _movq to explicitly think about the type
of
> the destination rather than having _movq try to figure it out.
> 
> In the same way, _movp has a more restricted context than _mov. The family of
> instructions that _movp represents cannot handle immediates or scalar
registers.
> I'm not sure if it's appropriate the have _movp try to handle the case when
the
> source is an immediate or a scalar register (and anyway the legalization code
is
> incomplete when the destination is an XMM register). On the other hand, if I
do
> not handle these, then _movp will accept a different set of source operands
than
> _mov, which might end up being quite confusing.

That's reasonable, LGTM then.

There are a dozen or so more cases where this "intelligence" could be applied,
mostly for _movsx, _movzx, and _cvt.  But then it's a slippery slope of whether
it should be added to more instructions such as _add, etc.

[jvoung (off chromium), 2014-07-11 22:29:57]

LGTM too

[wala, 2014-07-11 22:43:54]

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

[JF, 2014-07-15 01:47:03]

You'll want to make sure that divide and modulo trap, so that they're consistent
with scalar operations:
  https://code.google.com/p/nativeclient/issues/detail?id=3899

This is still undefined behavior for PNaCl, and we currently don't force LLVM to
trap, but we will at some point so Subzero will have to catch up.

Adding a TODO at least to this code would be good.