MIPS: Followup '[turbofan] Introduce new operators Float32SubPreserveNan and Float64SubPreserveNan'.

MIPS: Followup '[turbofan] Introduce new operators Float32SubPreserveNan and Float64SubPreserveNan'.

Port 481502dad9b45a38c86387d50cebe52f00b072ad

Float32SubMinusZero and Float64SubMinusZero tests are failing because MIPS does not preserve NaN payload according to Wasm spec. Implemented macro-assembler methods that check for NaN operands, and return the qNaN value with preserved payload and sign bits.

TEST=cctest/test-run-wasm/Run_WasmFloat32SubMinusZero, cctest/test-run-wasm/Run_WasmFloat64SubMinusZero

BUG=

patch from issue 2019693002 at patchset 140001 (http://crrev.com/2019693002#ps140001)

R=ahaas@chromium.org

Committed: https://crrev.com/eff959bb5593668a82dfb5c04c464ddcb8a3315c
Cr-Commit-Position: refs/heads/master@{#37105}

[balazs.kilvady, 2016-06-15 17:32:28]

Description was changed from

==========
MIPS: Followup '[turbofan] Introduce new operators Float32SubPreserveNan and
Float64SubPreserveNan'.

Port 481502dad9b45a38c86387d50cebe52f00b072ad

Float32SubMinusZero and Float64SubMinusZero tests are failing because MIPS does
not preserve NaN payload according to Wasm spec. Implemented macro-assembler
methods that check for NaN operands, and return the qNaN value with preserved
payload and sign bits.

TEST=cctest/test-run-wasm/Run_WasmFloat32SubMinusZero,
cctest/test-run-wasm/Run_WasmFloat64SubMinusZero

BUG=

patch from issue 2019693002 at patchset 140001
(http://crrev.com/2019693002#ps140001)

R=ahaas@chromium.org
==========

to

==========
MIPS: Followup '[turbofan] Introduce new operators Float32SubPreserveNan and
Float64SubPreserveNan'.

Port 481502dad9b45a38c86387d50cebe52f00b072ad

Float32SubMinusZero and Float64SubMinusZero tests are failing because MIPS does
not preserve NaN payload according to Wasm spec. Implemented macro-assembler
methods that check for NaN operands, and return the qNaN value with preserved
payload and sign bits.

TEST=cctest/test-run-wasm/Run_WasmFloat32SubMinusZero,
cctest/test-run-wasm/Run_WasmFloat64SubMinusZero

BUG=

patch from issue 2019693002 at patchset 140001
(http://crrev.com/2019693002#ps140001)

R=ahaas@chromium.org
==========

[balazs.kilvady, 2016-06-15 17:32:28]

balazs.kilvady@imgtec.com changed reviewers:
+ akos.palfi@imgtec.com, ivica.bogosavljevic@imgtec.com,
marija.antic@imgtec.com, titzer@chromium.org

[balazs.kilvady, 2016-06-15 18:25:12]

We need an lgtm from a v8-team member for est/cctest/wasm/test-run-wasm.cc,
PTAL.

[ahaas, 2016-06-15 19:08:44]

On 2016/06/15 at 18:25:12, balazs.kilvady wrote:
> We need an lgtm from a v8-team member for est/cctest/wasm/test-run-wasm.cc,
PTAL.

test-run-wasm.cc lgtm

[balazs.kilvady, 2016-06-16 10:39:55]

After applying the nan patch for qemu it seems we don't need the 'make quiet'
blocks for MIPSr6. PTAL.

[balazs.kilvady, 2016-06-16 10:40:23]

The CQ bit was checked by balazs.kilvady@imgtec.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-16 10:40:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2066483008/40001

[commit-bot: I haz the power, 2016-06-16 11:09:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-16 11:09:57]

Dry run: This issue passed the CQ dry run.

[Marija Antic, 2016-06-16 11:25:27]

https://codereview.chromium.org/2066483008/diff/40001/src/mips/macro-assemble...
File src/mips/macro-assembler-mips.cc (right):

https://codereview.chromium.org/2066483008/diff/40001/src/mips/macro-assemble...
src/mips/macro-assembler-mips.cc:4754: And(scratch2, scratch2,
Operand(kSingleNaNMask));
This will set the sign bit to 0 (ci20 board was changing the sign of the NaN
when subtracting from -0). Have you checked what happens with the negative NaN
input?

[balazs.kilvady, 2016-06-16 12:00:03]

https://codereview.chromium.org/2066483008/diff/40001/src/mips/macro-assemble...
File src/mips/macro-assembler-mips.cc (right):

https://codereview.chromium.org/2066483008/diff/40001/src/mips/macro-assemble...
src/mips/macro-assembler-mips.cc:4754: And(scratch2, scratch2,
Operand(kSingleNaNMask));
On 2016/06/16 11:25:26, Marija Antic wrote:
> This will set the sign bit to 0 (ci20 board was changing the sign of the NaN
> when subtracting from -0). Have you checked what happens with the negative NaN
> input?
Yes. The test expects + quiet NaN with payload (Intel behavior?). So that is why
I clear the sign bit here.

[Marija Antic, 2016-06-16 12:21:23]

On 2016/06/16 12:00:03, balazs.kilvady wrote:
>
https://codereview.chromium.org/2066483008/diff/40001/src/mips/macro-assemble...
> File src/mips/macro-assembler-mips.cc (right):
> 
>
https://codereview.chromium.org/2066483008/diff/40001/src/mips/macro-assemble...
> src/mips/macro-assembler-mips.cc:4754: And(scratch2, scratch2,
> Operand(kSingleNaNMask));
> On 2016/06/16 11:25:26, Marija Antic wrote:
> > This will set the sign bit to 0 (ci20 board was changing the sign of the NaN
> > when subtracting from -0). Have you checked what happens with the negative
NaN
> > input?
> Yes. The test expects + quiet NaN with payload (Intel behavior?). So that is
why
> I clear the sign bit here.

The original test was using a + signaling Intel NaN as the input, expecting a +
quiet Intel NaN as the output, so I am not sure whether this meant that a
positive NaN is expected, or the sign should be preserved. Wasm spec says that
one of the input NaNs should be returned (it also says that its signaling bit
should be set to 1, but my guess is that it means "NaN should be quiet", based
on the way Min and Max are handled in wasm interpreter). With this
implementation, in case of a negative input NaN we will return a positive NaN
with the same payload. This is not covered with a test case, we should probably
check with wasm owners if a positive NaN is expected, or the original operand.

[balazs.kilvady, 2016-06-16 12:57:44]

On 2016/06/16 12:21:23, Marija Antic wrote:
> On 2016/06/16 12:00:03, balazs.kilvady wrote:
> >
>
https://codereview.chromium.org/2066483008/diff/40001/src/mips/macro-assemble...
> > File src/mips/macro-assembler-mips.cc (right):
> > 
> >
>
https://codereview.chromium.org/2066483008/diff/40001/src/mips/macro-assemble...
> > src/mips/macro-assembler-mips.cc:4754: And(scratch2, scratch2,
> > Operand(kSingleNaNMask));
> > On 2016/06/16 11:25:26, Marija Antic wrote:
> > > This will set the sign bit to 0 (ci20 board was changing the sign of the
NaN
> > > when subtracting from -0). Have you checked what happens with the negative
> NaN
> > > input?
> > Yes. The test expects + quiet NaN with payload (Intel behavior?). So that is
> why
> > I clear the sign bit here.
> 
> The original test was using a + signaling Intel NaN as the input, expecting a
+
> quiet Intel NaN as the output, so I am not sure whether this meant that a
> positive NaN is expected, or the sign should be preserved. Wasm spec says that
> one of the input NaNs should be returned (it also says that its signaling bit
> should be set to 1, but my guess is that it means "NaN should be quiet", based
> on the way Min and Max are handled in wasm interpreter). With this
> implementation, in case of a negative input NaN we will return a positive NaN
> with the same payload. This is not covered with a test case, we should
probably
> check with wasm owners if a positive NaN is expected, or the original operand.

The executed instruction on all the architectures is: -0.0 - (+signaledNaN with
payload) and the expected result is + quietNaN with payload. For example on
simulator the sub_d executes directly an Intel sub:
  0x00000001082e85f8   46220081       sub.d   f2, f0, f2
sim> p f2
 f2: 0x7ff0123456789abc               nan
sim> p f0
 f0: 0x8000000000000000       -0.0000e+00
and the result is:
sim> p f2
 f2: 0x7ff8123456789abc               nan
Simulator does not do any trick, the result on Intel:
(lldb) p/x value
(double) $1 = 0x7ff8123456789abc

On MIPS our sub.fmt does not save the payload so we have to implement/simulate a
behavior of other archs. So in this case I think it is absolutely correct if we
simulate the same behavior others have.

[ahaas, 2016-06-16 13:41:54]

On 2016/06/16 at 12:57:44, balazs.kilvady wrote:
> On 2016/06/16 12:21:23, Marija Antic wrote:
> > On 2016/06/16 12:00:03, balazs.kilvady wrote:
> > >
> >
https://codereview.chromium.org/2066483008/diff/40001/src/mips/macro-assemble...
> > > File src/mips/macro-assembler-mips.cc (right):
> > > 
> > >
> >
https://codereview.chromium.org/2066483008/diff/40001/src/mips/macro-assemble...
> > > src/mips/macro-assembler-mips.cc:4754: And(scratch2, scratch2,
> > > Operand(kSingleNaNMask));
> > > On 2016/06/16 11:25:26, Marija Antic wrote:
> > > > This will set the sign bit to 0 (ci20 board was changing the sign of the
NaN
> > > > when subtracting from -0). Have you checked what happens with the
negative
> > NaN
> > > > input?
> > > Yes. The test expects + quiet NaN with payload (Intel behavior?). So that
is
> > why
> > > I clear the sign bit here.
> > 
> > The original test was using a + signaling Intel NaN as the input, expecting
a +
> > quiet Intel NaN as the output, so I am not sure whether this meant that a
> > positive NaN is expected, or the sign should be preserved. Wasm spec says
that
> > one of the input NaNs should be returned (it also says that its signaling
bit
> > should be set to 1, but my guess is that it means "NaN should be quiet",
based
> > on the way Min and Max are handled in wasm interpreter). With this
> > implementation, in case of a negative input NaN we will return a positive
NaN
> > with the same payload. This is not covered with a test case, we should
probably
> > check with wasm owners if a positive NaN is expected, or the original
operand.
> 
> The executed instruction on all the architectures is: -0.0 - (+signaledNaN
with payload) and the expected result is + quietNaN with payload. For example on
simulator the sub_d executes directly an Intel sub:
>   0x00000001082e85f8   46220081       sub.d   f2, f0, f2
> sim> p f2
>  f2: 0x7ff0123456789abc               nan
> sim> p f0
>  f0: 0x8000000000000000       -0.0000e+00
> and the result is:
> sim> p f2
>  f2: 0x7ff8123456789abc               nan
> Simulator does not do any trick, the result on Intel:
> (lldb) p/x value
> (double) $1 = 0x7ff8123456789abc
> 
> On MIPS our sub.fmt does not save the payload so we have to implement/simulate
a behavior of other archs. So in this case I think it is absolutely correct if
we simulate the same behavior others have.

If x = NaN, then the result of -0.0 - x should be quiet(x), where quiet(x) is a
quiet NaN, and where all bits of quiet(x) are the same as in x, except for the
signalling bit.

[balazs.kilvady, 2016-06-16 15:20:32]

On 2016/06/16 13:41:54, ahaas wrote:
> On 2016/06/16 at 12:57:44, balazs.kilvady wrote:
> > On 2016/06/16 12:21:23, Marija Antic wrote:
> > > On 2016/06/16 12:00:03, balazs.kilvady wrote:
> > > >
> > >
>
https://codereview.chromium.org/2066483008/diff/40001/src/mips/macro-assemble...
> > > > File src/mips/macro-assembler-mips.cc (right):
> > > > 
> > > >
> > >
>
https://codereview.chromium.org/2066483008/diff/40001/src/mips/macro-assemble...
> > > > src/mips/macro-assembler-mips.cc:4754: And(scratch2, scratch2,
> > > > Operand(kSingleNaNMask));
> > > > On 2016/06/16 11:25:26, Marija Antic wrote:
> > > > > This will set the sign bit to 0 (ci20 board was changing the sign of
the
> NaN
> > > > > when subtracting from -0). Have you checked what happens with the
> negative
> > > NaN
> > > > > input?
> > > > Yes. The test expects + quiet NaN with payload (Intel behavior?). So
that
> is
> > > why
> > > > I clear the sign bit here.
> > > 
> > > The original test was using a + signaling Intel NaN as the input,
expecting
> a +
> > > quiet Intel NaN as the output, so I am not sure whether this meant that a
> > > positive NaN is expected, or the sign should be preserved. Wasm spec says
> that
> > > one of the input NaNs should be returned (it also says that its signaling
> bit
> > > should be set to 1, but my guess is that it means "NaN should be quiet",
> based
> > > on the way Min and Max are handled in wasm interpreter). With this
> > > implementation, in case of a negative input NaN we will return a positive
> NaN
> > > with the same payload. This is not covered with a test case, we should
> probably
> > > check with wasm owners if a positive NaN is expected, or the original
> operand.
> > 
> > The executed instruction on all the architectures is: -0.0 - (+signaledNaN
> with payload) and the expected result is + quietNaN with payload. For example
on
> simulator the sub_d executes directly an Intel sub:
> >   0x00000001082e85f8   46220081       sub.d   f2, f0, f2
> > sim> p f2
> >  f2: 0x7ff0123456789abc               nan
> > sim> p f0
> >  f0: 0x8000000000000000       -0.0000e+00
> > and the result is:
> > sim> p f2
> >  f2: 0x7ff8123456789abc               nan
> > Simulator does not do any trick, the result on Intel:
> > (lldb) p/x value
> > (double) $1 = 0x7ff8123456789abc
> > 
> > On MIPS our sub.fmt does not save the payload so we have to
implement/simulate
> a behavior of other archs. So in this case I think it is absolutely correct if
> we simulate the same behavior others have.
> 
> If x = NaN, then the result of -0.0 - x should be quiet(x), where quiet(x) is
a
> quiet NaN, and where all bits of quiet(x) are the same as in x, except for the
> signalling bit.

Thanks for the clarifying, Andreas. That is what my implementation does. If x is
a +NaN then -0.0 - x  must produce a +quietNaN(x) (0x7ff.....) not a
-quietNaN(x) (0xfff....).

[ahaas, 2016-06-16 16:18:34]

On 2016/06/16 at 13:41:54, ahaas wrote:
> On 2016/06/16 at 12:57:44, balazs.kilvady wrote:
> > On 2016/06/16 12:21:23, Marija Antic wrote:
> > > On 2016/06/16 12:00:03, balazs.kilvady wrote:
> > > >
> > >
https://codereview.chromium.org/2066483008/diff/40001/src/mips/macro-assemble...
> > > > File src/mips/macro-assembler-mips.cc (right):
> > > > 
> > > >
> > >
https://codereview.chromium.org/2066483008/diff/40001/src/mips/macro-assemble...
> > > > src/mips/macro-assembler-mips.cc:4754: And(scratch2, scratch2,
> > > > Operand(kSingleNaNMask));
> > > > On 2016/06/16 11:25:26, Marija Antic wrote:
> > > > > This will set the sign bit to 0 (ci20 board was changing the sign of
the NaN
> > > > > when subtracting from -0). Have you checked what happens with the
negative
> > > NaN
> > > > > input?
> > > > Yes. The test expects + quiet NaN with payload (Intel behavior?). So
that is
> > > why
> > > > I clear the sign bit here.
> > > 
> > > The original test was using a + signaling Intel NaN as the input,
expecting a +
> > > quiet Intel NaN as the output, so I am not sure whether this meant that a
> > > positive NaN is expected, or the sign should be preserved. Wasm spec says
that
> > > one of the input NaNs should be returned (it also says that its signaling
bit
> > > should be set to 1, but my guess is that it means "NaN should be quiet",
based
> > > on the way Min and Max are handled in wasm interpreter). With this
> > > implementation, in case of a negative input NaN we will return a positive
NaN
> > > with the same payload. This is not covered with a test case, we should
probably
> > > check with wasm owners if a positive NaN is expected, or the original
operand.
> > 
> > The executed instruction on all the architectures is: -0.0 - (+signaledNaN
with payload) and the expected result is + quietNaN with payload. For example on
simulator the sub_d executes directly an Intel sub:
> >   0x00000001082e85f8   46220081       sub.d   f2, f0, f2
> > sim> p f2
> >  f2: 0x7ff0123456789abc               nan
> > sim> p f0
> >  f0: 0x8000000000000000       -0.0000e+00
> > and the result is:
> > sim> p f2
> >  f2: 0x7ff8123456789abc               nan
> > Simulator does not do any trick, the result on Intel:
> > (lldb) p/x value
> > (double) $1 = 0x7ff8123456789abc
> > 
> > On MIPS our sub.fmt does not save the payload so we have to
implement/simulate a behavior of other archs. So in this case I think it is
absolutely correct if we simulate the same behavior others have.
> 
> If x = NaN, then the result of -0.0 - x should be quiet(x), where quiet(x) is
a quiet NaN, and where all bits of quiet(x) are the same as in x, except for the
signalling bit.

I extended your test case to cover -NaN, and with the current implementation of
the simulator the mips build does not pass on my machine.

WASM_EXEC_TEST(Float32SubMinusZero) {
  WasmRunner<float> r(execution_mode, MachineType::Float32());
  BUILD(r, WASM_F32_SUB(WASM_F32(-0.0), WASM_GET_LOCAL(0)));

  uint32_t sNanValue =
      bit_cast<uint32_t>(std::numeric_limits<float>::signaling_NaN());
  uint32_t qNanValue =
      bit_cast<uint32_t>(std::numeric_limits<float>::quiet_NaN());
  uint32_t payload = 0x00200000;

  uint32_t expected = (qNanValue & 0xffc00000) | payload;
  uint32_t operand = (sNanValue & 0xffc00000) | payload;
  CHECK_EQ(expected, bit_cast<uint32_t>(r.Call(bit_cast<float>(operand))));

  // Change the sign of the NaN.
  expected |= 0x80000000;
  operand |= 0x80000000;
  CHECK_EQ(expected, bit_cast<uint32_t>(r.Call(bit_cast<float>(operand))));
}

[balazs.kilvady, 2016-06-16 18:10:57]

On 2016/06/16 16:18:34, ahaas wrote:
> On 2016/06/16 at 13:41:54, ahaas wrote:
> > On 2016/06/16 at 12:57:44, balazs.kilvady wrote:
> > > On 2016/06/16 12:21:23, Marija Antic wrote:
> > > > On 2016/06/16 12:00:03, balazs.kilvady wrote:
> > > > >
> > > >
>
https://codereview.chromium.org/2066483008/diff/40001/src/mips/macro-assemble...
> > > > > File src/mips/macro-assembler-mips.cc (right):
> > > > > 
> > > > >
> > > >
>
https://codereview.chromium.org/2066483008/diff/40001/src/mips/macro-assemble...
> > > > > src/mips/macro-assembler-mips.cc:4754: And(scratch2, scratch2,
> > > > > Operand(kSingleNaNMask));
> > > > > On 2016/06/16 11:25:26, Marija Antic wrote:
> > > > > > This will set the sign bit to 0 (ci20 board was changing the sign of
> the NaN
> > > > > > when subtracting from -0). Have you checked what happens with the
> negative
> > > > NaN
> > > > > > input?
> > > > > Yes. The test expects + quiet NaN with payload (Intel behavior?). So
> that is
> > > > why
> > > > > I clear the sign bit here.
> > > > 
> > > > The original test was using a + signaling Intel NaN as the input,
> expecting a +
> > > > quiet Intel NaN as the output, so I am not sure whether this meant that
a
> > > > positive NaN is expected, or the sign should be preserved. Wasm spec
says
> that
> > > > one of the input NaNs should be returned (it also says that its
signaling
> bit
> > > > should be set to 1, but my guess is that it means "NaN should be quiet",
> based
> > > > on the way Min and Max are handled in wasm interpreter). With this
> > > > implementation, in case of a negative input NaN we will return a
positive
> NaN
> > > > with the same payload. This is not covered with a test case, we should
> probably
> > > > check with wasm owners if a positive NaN is expected, or the original
> operand.
> > > 
> > > The executed instruction on all the architectures is: -0.0 - (+signaledNaN
> with payload) and the expected result is + quietNaN with payload. For example
on
> simulator the sub_d executes directly an Intel sub:
> > >   0x00000001082e85f8   46220081       sub.d   f2, f0, f2
> > > sim> p f2
> > >  f2: 0x7ff0123456789abc               nan
> > > sim> p f0
> > >  f0: 0x8000000000000000       -0.0000e+00
> > > and the result is:
> > > sim> p f2
> > >  f2: 0x7ff8123456789abc               nan
> > > Simulator does not do any trick, the result on Intel:
> > > (lldb) p/x value
> > > (double) $1 = 0x7ff8123456789abc
> > > 
> > > On MIPS our sub.fmt does not save the payload so we have to
> implement/simulate a behavior of other archs. So in this case I think it is
> absolutely correct if we simulate the same behavior others have.
> > 
> > If x = NaN, then the result of -0.0 - x should be quiet(x), where quiet(x)
is
> a quiet NaN, and where all bits of quiet(x) are the same as in x, except for
the
> signalling bit.
> 
> I extended your test case to cover -NaN, and with the current implementation
of
> the simulator the mips build does not pass on my machine.
> 
> WASM_EXEC_TEST(Float32SubMinusZero) {
>   WasmRunner<float> r(execution_mode, MachineType::Float32());
>   BUILD(r, WASM_F32_SUB(WASM_F32(-0.0), WASM_GET_LOCAL(0)));
> 
>   uint32_t sNanValue =
>       bit_cast<uint32_t>(std::numeric_limits<float>::signaling_NaN());
>   uint32_t qNanValue =
>       bit_cast<uint32_t>(std::numeric_limits<float>::quiet_NaN());
>   uint32_t payload = 0x00200000;
> 
>   uint32_t expected = (qNanValue & 0xffc00000) | payload;
>   uint32_t operand = (sNanValue & 0xffc00000) | payload;
>   CHECK_EQ(expected, bit_cast<uint32_t>(r.Call(bit_cast<float>(operand))));
> 
>   // Change the sign of the NaN.
>   expected |= 0x80000000;
>   operand |= 0x80000000;
>   CHECK_EQ(expected, bit_cast<uint32_t>(r.Call(bit_cast<float>(operand))));
> }

Thanks, I will add your changes to the test and change the implementation to
pass it.

[Marija Antic, 2016-06-17 08:09:08]

https://codereview.chromium.org/2066483008/diff/1/src/wasm/wasm-interpreter.cc
File src/wasm/wasm-interpreter.cc (left):

https://codereview.chromium.org/2066483008/diff/1/src/wasm/wasm-interpreter.c...
src/wasm/wasm-interpreter.cc:63: V(F32Sub, float, -)           \
Have you run the interpreter tests on the board? Without the changes in
interpreter, RunWasmInterpreted_Float32SubMinusZero and
RunWasmInterpreted_Float32SubMinusZero were failing on the board.

[balazs.kilvady, 2016-06-19 11:53:27]

The CQ bit was checked by balazs.kilvady@imgtec.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-19 11:53:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2066483008/100001

[commit-bot: I haz the power, 2016-06-19 12:18:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-19 12:18:11]

Dry run: This issue passed the CQ dry run.

[balazs.kilvady, 2016-06-19 12:53:17]

Andreas, please take an other look to wasm codes. I added your change to both
float and double tests, fixed the MIPS code to pass them and tested them on r2,
r6 MIPS boards and simulators. I don't know if
 '#if V8_TARGET_ARCH_MIPS || V8_TARGET_ARCH_MIPS64'
conditionals are acceptable in wasm-interpreter.cc but as I see Intel and ARM
don't need those fixes.

[ahaas, 2016-06-20 09:17:02]

On 2016/06/19 at 12:53:17, balazs.kilvady wrote:
> Andreas, please take an other look to wasm codes. I added your change to both
float and double tests, fixed the MIPS code to pass them and tested them on r2,
r6 MIPS boards and simulators. I don't know if
>  '#if V8_TARGET_ARCH_MIPS || V8_TARGET_ARCH_MIPS64'
> conditionals are acceptable in wasm-interpreter.cc but as I see Intel and ARM
don't need those fixes.

lgtm!

Instead of the {#if V8_TARGET_ARCH_MIPS || V8_TARGET_ARCH_MIPS64} in the
interpreter, could you just add a comment why this extra code is necessary on
some platforms? I think we can keep this code for all platforms at the moment.

[balazs.kilvady, 2016-06-20 12:55:08]

The CQ bit was checked by balazs.kilvady@imgtec.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-20 12:55:14]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2066483008/120001

[balazs.kilvady, 2016-06-20 12:56:00]

On 2016/06/20 09:17:02, ahaas wrote:
> On 2016/06/19 at 12:53:17, balazs.kilvady wrote:
> > Andreas, please take an other look to wasm codes. I added your change to
both
> float and double tests, fixed the MIPS code to pass them and tested them on
r2,
> r6 MIPS boards and simulators. I don't know if
> >  '#if V8_TARGET_ARCH_MIPS || V8_TARGET_ARCH_MIPS64'
> > conditionals are acceptable in wasm-interpreter.cc but as I see Intel and
ARM
> don't need those fixes.
> 
> lgtm!
> 
> Instead of the {#if V8_TARGET_ARCH_MIPS || V8_TARGET_ARCH_MIPS64} in the
> interpreter, could you just add a comment why this extra code is necessary on
> some platforms? I think we can keep this code for all platforms at the moment.

Thanks for reviewing, comment: done.

[commit-bot: I haz the power, 2016-06-20 13:24:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-20 13:24:28]

Dry run: This issue passed the CQ dry run.

[balazs.kilvady, 2016-06-20 13:26:19]

The CQ bit was checked by balazs.kilvady@imgtec.com

[balazs.kilvady, 2016-06-20 13:26:19]

The patchset sent to the CQ was uploaded after l-g-t-m from ahaas@chromium.org
Link to the patchset: https://codereview.chromium.org/2066483008/#ps120001
(title: "Comment the iterpreter.")

[commit-bot: I haz the power, 2016-06-20 13:26:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2066483008/120001

[commit-bot: I haz the power, 2016-06-20 13:28:27]

Description was changed from

==========
MIPS: Followup '[turbofan] Introduce new operators Float32SubPreserveNan and
Float64SubPreserveNan'.

Port 481502dad9b45a38c86387d50cebe52f00b072ad

Float32SubMinusZero and Float64SubMinusZero tests are failing because MIPS does
not preserve NaN payload according to Wasm spec. Implemented macro-assembler
methods that check for NaN operands, and return the qNaN value with preserved
payload and sign bits.

TEST=cctest/test-run-wasm/Run_WasmFloat32SubMinusZero,
cctest/test-run-wasm/Run_WasmFloat64SubMinusZero

BUG=

patch from issue 2019693002 at patchset 140001
(http://crrev.com/2019693002#ps140001)

R=ahaas@chromium.org
==========

to

==========
MIPS: Followup '[turbofan] Introduce new operators Float32SubPreserveNan and
Float64SubPreserveNan'.

Port 481502dad9b45a38c86387d50cebe52f00b072ad

Float32SubMinusZero and Float64SubMinusZero tests are failing because MIPS does
not preserve NaN payload according to Wasm spec. Implemented macro-assembler
methods that check for NaN operands, and return the qNaN value with preserved
payload and sign bits.

TEST=cctest/test-run-wasm/Run_WasmFloat32SubMinusZero,
cctest/test-run-wasm/Run_WasmFloat64SubMinusZero

BUG=

patch from issue 2019693002 at patchset 140001
(http://crrev.com/2019693002#ps140001)

R=ahaas@chromium.org
==========

[commit-bot: I haz the power, 2016-06-20 13:28:28]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-06-20 13:29:00]

Description was changed from

==========
MIPS: Followup '[turbofan] Introduce new operators Float32SubPreserveNan and
Float64SubPreserveNan'.

Port 481502dad9b45a38c86387d50cebe52f00b072ad

Float32SubMinusZero and Float64SubMinusZero tests are failing because MIPS does
not preserve NaN payload according to Wasm spec. Implemented macro-assembler
methods that check for NaN operands, and return the qNaN value with preserved
payload and sign bits.

TEST=cctest/test-run-wasm/Run_WasmFloat32SubMinusZero,
cctest/test-run-wasm/Run_WasmFloat64SubMinusZero

BUG=

patch from issue 2019693002 at patchset 140001
(http://crrev.com/2019693002#ps140001)

R=ahaas@chromium.org
==========

to

==========
MIPS: Followup '[turbofan] Introduce new operators Float32SubPreserveNan and
Float64SubPreserveNan'.

Port 481502dad9b45a38c86387d50cebe52f00b072ad

Float32SubMinusZero and Float64SubMinusZero tests are failing because MIPS does
not preserve NaN payload according to Wasm spec. Implemented macro-assembler
methods that check for NaN operands, and return the qNaN value with preserved
payload and sign bits.

TEST=cctest/test-run-wasm/Run_WasmFloat32SubMinusZero,
cctest/test-run-wasm/Run_WasmFloat64SubMinusZero

BUG=

patch from issue 2019693002 at patchset 140001
(http://crrev.com/2019693002#ps140001)

R=ahaas@chromium.org

Committed: https://crrev.com/eff959bb5593668a82dfb5c04c464ddcb8a3315c
Cr-Commit-Position: refs/heads/master@{#37105}
==========

[commit-bot: I haz the power, 2016-06-20 13:29:01]

Patchset 7 (id:??) landed as
https://crrev.com/eff959bb5593668a82dfb5c04c464ddcb8a3315c
Cr-Commit-Position: refs/heads/master@{#37105}