Commit 7ff8d317 authored by ahaas's avatar ahaas Committed by Commit bot

Revert of [wasm] Fix I32ReinterpretF32 and I64ReinterpretF64 on ia32....

Revert of [wasm] Fix I32ReinterpretF32 and I64ReinterpretF64 on ia32. (patchset #3 id:40001 of https://codereview.chromium.org/2639353002/ )

Reason for revert:
compilation problems on mips

Original issue's description:
> [wasm] Fix I32ReinterpretF32 and I64ReinterpretF64 on ia32.
>
> On ia32 return statements in C++ automatically convert signalling NaNs
> to quiet NaNs, even when bit_cast is used. This CL removes all uses of
> bit_cast<float> and bit_cast<double> in the wasm compiler and wasm
> interpreter.
>
> R=titzer@chromium.org, clemensh@chromium.org
>
> Review-Url: https://codereview.chromium.org/2639353002
> Cr-Commit-Position: refs/heads/master@{#42512}
> Committed: https://chromium.googlesource.com/v8/v8/+/7739affa5b57e0d28674d476f63de60d71728fb6

TBR=clemensh@chromium.org,titzer@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review-Url: https://codereview.chromium.org/2645693003
Cr-Commit-Position: refs/heads/master@{#42514}
parent ba2cd169
...@@ -2106,7 +2106,7 @@ void CodeGenerator::AssembleMove(InstructionOperand* source, ...@@ -2106,7 +2106,7 @@ void CodeGenerator::AssembleMove(InstructionOperand* source,
__ Move(dst, g.ToImmediate(source)); __ Move(dst, g.ToImmediate(source));
} else if (src_constant.type() == Constant::kFloat32) { } else if (src_constant.type() == Constant::kFloat32) {
// TODO(turbofan): Can we do better here? // TODO(turbofan): Can we do better here?
uint32_t src = src_constant.ToFloat32AsInt(); uint32_t src = bit_cast<uint32_t>(src_constant.ToFloat32());
if (destination->IsFPRegister()) { if (destination->IsFPRegister()) {
XMMRegister dst = g.ToDoubleRegister(destination); XMMRegister dst = g.ToDoubleRegister(destination);
__ Move(dst, src); __ Move(dst, src);
...@@ -2117,7 +2117,7 @@ void CodeGenerator::AssembleMove(InstructionOperand* source, ...@@ -2117,7 +2117,7 @@ void CodeGenerator::AssembleMove(InstructionOperand* source,
} }
} else { } else {
DCHECK_EQ(Constant::kFloat64, src_constant.type()); DCHECK_EQ(Constant::kFloat64, src_constant.type());
uint64_t src = src_constant.ToFloat64AsInt(); uint64_t src = bit_cast<uint64_t>(src_constant.ToFloat64());
uint32_t lower = static_cast<uint32_t>(src); uint32_t lower = static_cast<uint32_t>(src);
uint32_t upper = static_cast<uint32_t>(src >> 32); uint32_t upper = static_cast<uint32_t>(src >> 32);
if (destination->IsFPRegister()) { if (destination->IsFPRegister()) {
......
...@@ -1065,33 +1065,16 @@ class V8_EXPORT_PRIVATE Constant final { ...@@ -1065,33 +1065,16 @@ class V8_EXPORT_PRIVATE Constant final {
} }
float ToFloat32() const { float ToFloat32() const {
// TODO(ahaas): We should remove this function. If value_ has the bit
// representation of a signalling NaN, then returning it as float can cause
// the signalling bit to flip, and value_ is returned as a quiet NaN.
DCHECK_EQ(kFloat32, type()); DCHECK_EQ(kFloat32, type());
return bit_cast<float>(static_cast<int32_t>(value_)); return bit_cast<float>(static_cast<int32_t>(value_));
} }
uint32_t ToFloat32AsInt() const {
DCHECK_EQ(kFloat32, type());
return bit_cast<uint32_t>(static_cast<int32_t>(value_));
}
double ToFloat64() const { double ToFloat64() const {
// TODO(ahaas): We should remove this function. If value_ has the bit
// representation of a signalling NaN, then returning it as float can cause
// the signalling bit to flip, and value_ is returned as a quiet NaN.
if (type() == kInt32) return ToInt32(); if (type() == kInt32) return ToInt32();
DCHECK_EQ(kFloat64, type()); DCHECK_EQ(kFloat64, type());
return bit_cast<double>(value_); return bit_cast<double>(value_);
} }
uint64_t ToFloat64AsInt() const {
if (type() == kInt32) return ToInt32();
DCHECK_EQ(kFloat64, type());
return bit_cast<uint64_t>(value_);
}
ExternalReference ToExternalReference() const { ExternalReference ToExternalReference() const {
DCHECK_EQ(kExternalReference, type()); DCHECK_EQ(kExternalReference, type());
return bit_cast<ExternalReference>(static_cast<intptr_t>(value_)); return bit_cast<ExternalReference>(static_cast<intptr_t>(value_));
......
...@@ -70,9 +70,7 @@ struct ImmF32Operand { ...@@ -70,9 +70,7 @@ struct ImmF32Operand {
float value; float value;
unsigned length; unsigned length;
inline ImmF32Operand(Decoder* decoder, const byte* pc) { inline ImmF32Operand(Decoder* decoder, const byte* pc) {
// Avoid bit_cast because it might not preserve the signalling bit of a NaN. value = bit_cast<float>(decoder->checked_read_u32(pc, 1, "immf32"));
uint32_t tmp = decoder->checked_read_u32(pc, 1, "immf32");
value = *reinterpret_cast<float*>(&tmp);
length = 4; length = 4;
} }
}; };
...@@ -81,9 +79,7 @@ struct ImmF64Operand { ...@@ -81,9 +79,7 @@ struct ImmF64Operand {
double value; double value;
unsigned length; unsigned length;
inline ImmF64Operand(Decoder* decoder, const byte* pc) { inline ImmF64Operand(Decoder* decoder, const byte* pc) {
// Avoid bit_cast because it might not preserve the signalling bit of a NaN. value = bit_cast<double>(decoder->checked_read_u64(pc, 1, "immf64"));
uint64_t tmp = decoder->checked_read_u64(pc, 1, "immf64");
value = *reinterpret_cast<double*>(&tmp);
length = 8; length = 8;
} }
}; };
......
...@@ -159,6 +159,8 @@ namespace wasm { ...@@ -159,6 +159,8 @@ namespace wasm {
V(F64UConvertI64, uint64_t) \ V(F64UConvertI64, uint64_t) \
V(F64ConvertF32, float) \ V(F64ConvertF32, float) \
V(F64ReinterpretI64, int64_t) \ V(F64ReinterpretI64, int64_t) \
V(I32ReinterpretF32, float) \
V(I64ReinterpretF64, double) \
V(I32AsmjsSConvertF32, float) \ V(I32AsmjsSConvertF32, float) \
V(I32AsmjsUConvertF32, float) \ V(I32AsmjsUConvertF32, float) \
V(I32AsmjsSConvertF64, double) \ V(I32AsmjsSConvertF64, double) \
...@@ -604,12 +606,12 @@ static inline double ExecuteF64ReinterpretI64(int64_t a, TrapReason* trap) { ...@@ -604,12 +606,12 @@ static inline double ExecuteF64ReinterpretI64(int64_t a, TrapReason* trap) {
return bit_cast<double>(a); return bit_cast<double>(a);
} }
static inline int32_t ExecuteI32ReinterpretF32(WasmVal a) { static inline int32_t ExecuteI32ReinterpretF32(float a, TrapReason* trap) {
return a.to_unchecked<int32_t>(); return bit_cast<int32_t>(a);
} }
static inline int64_t ExecuteI64ReinterpretF64(WasmVal a) { static inline int64_t ExecuteI64ReinterpretF64(double a, TrapReason* trap) {
return a.to_unchecked<int64_t>(); return bit_cast<int64_t>(a);
} }
static inline int32_t ExecuteGrowMemory(uint32_t delta_pages, static inline int32_t ExecuteGrowMemory(uint32_t delta_pages,
...@@ -1546,19 +1548,6 @@ class ThreadImpl { ...@@ -1546,19 +1548,6 @@ class ThreadImpl {
len = 1 + operand.length; len = 1 + operand.length;
break; break;
} }
// We need to treat kExprI32ReinterpretF32 and kExprI64ReinterpretF64
// specially to guarantee that the quiet bit of a NaN is preserved on
// ia32 by the reinterpret casts.
case kExprI32ReinterpretF32: {
WasmVal result(ExecuteI32ReinterpretF32(Pop()));
Push(pc, result);
break;
}
case kExprI64ReinterpretF64: {
WasmVal result(ExecuteI64ReinterpretF64(Pop()));
Push(pc, result);
break;
}
#define EXECUTE_SIMPLE_BINOP(name, ctype, op) \ #define EXECUTE_SIMPLE_BINOP(name, ctype, op) \
case kExpr##name: { \ case kExpr##name: { \
WasmVal rval = Pop(); \ WasmVal rval = Pop(); \
......
...@@ -56,21 +56,12 @@ struct WasmVal { ...@@ -56,21 +56,12 @@ struct WasmVal {
#undef DECLARE_CONSTRUCTOR #undef DECLARE_CONSTRUCTOR
template <typename T> template <typename T>
inline T to() { T to() {
UNREACHABLE();
}
template <typename T>
inline T to_unchecked() {
UNREACHABLE(); UNREACHABLE();
} }
}; };
#define DECLARE_CAST(field, localtype, ctype) \ #define DECLARE_CAST(field, localtype, ctype) \
template <> \
inline ctype WasmVal::to_unchecked() { \
return val.field; \
} \
template <> \ template <> \
inline ctype WasmVal::to() { \ inline ctype WasmVal::to() { \
CHECK_EQ(localtype, type); \ CHECK_EQ(localtype, type); \
...@@ -79,6 +70,11 @@ struct WasmVal { ...@@ -79,6 +70,11 @@ struct WasmVal {
FOREACH_UNION_MEMBER(DECLARE_CAST) FOREACH_UNION_MEMBER(DECLARE_CAST)
#undef DECLARE_CAST #undef DECLARE_CAST
template <>
inline void WasmVal::to() {
CHECK_EQ(kWasmStmt, type);
}
// Representation of frames within the interpreter. // Representation of frames within the interpreter.
class WasmFrame { class WasmFrame {
public: public:
......
...@@ -1319,16 +1319,6 @@ WASM_EXEC_TEST(I64ReinterpretF64) { ...@@ -1319,16 +1319,6 @@ WASM_EXEC_TEST(I64ReinterpretF64) {
} }
} }
WASM_EXEC_TEST(SignallingNanSurvivesI64ReinterpretF64) {
REQUIRE(I64ReinterpretF64);
WasmRunner<int64_t> r(execution_mode);
BUILD(r, WASM_I64_REINTERPRET_F64(WASM_SEQ(kExprF64Const, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0xf4, 0x7f)));
// This is a signalling nan.
CHECK_EQ(0x7ff4000000000000, r.Call());
}
WASM_EXEC_TEST(F64ReinterpretI64) { WASM_EXEC_TEST(F64ReinterpretI64) {
REQUIRE(F64ReinterpretI64); REQUIRE(F64ReinterpretI64);
WasmRunner<int64_t, int64_t> r(execution_mode); WasmRunner<int64_t, int64_t> r(execution_mode);
......
...@@ -1048,16 +1048,6 @@ WASM_EXEC_TEST(I32ReinterpretF32) { ...@@ -1048,16 +1048,6 @@ WASM_EXEC_TEST(I32ReinterpretF32) {
} }
} }
WASM_EXEC_TEST(SignallingNanSurvivesI32ReinterpretF32) {
WasmRunner<int32_t> r(execution_mode);
BUILD(r, WASM_I32_REINTERPRET_F32(
WASM_SEQ(kExprF32Const, 0x00, 0x00, 0xa0, 0x7f)));
// This is a signalling nan.
CHECK_EQ(0x7fa00000, r.Call());
}
WASM_EXEC_TEST_WITH_TRAP(LoadMaxUint32Offset) { WASM_EXEC_TEST_WITH_TRAP(LoadMaxUint32Offset) {
WasmRunner<int32_t> r(execution_mode); WasmRunner<int32_t> r(execution_mode);
r.module().AddMemoryElems<int32_t>(8); r.module().AddMemoryElems<int32_t>(8);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment