Merge pull request #152 from WebAssembly/float-refactor

Float refactoring and nan comparison change

8 files changed, 111 insertions, 91 deletions

diff --git a/src/asm2wasm.h b/src/asm2wasm.h
index 473384f2e..d56bf0cce 100644
--- a/src/asm2wasm.h
+++ b/src/asm2wasm.h
@@ -804,8 +804,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
         ret->offset = 0;
         ret->align = ret->bytes;
         auto ptr = allocator.alloc<Const>();
-        ptr->value.type = WasmType::i32; // XXX for wasm64, need 64
-        ptr->value.i32 = global.address;
+        ptr->value = Literal(int32_t(global.address)); // XXX for wasm64, need 64
         ret->ptr = ptr;
         ret->value = process(ast[3]);
         ret->type = global.type;
@@ -874,14 +873,11 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
       auto ret = allocator.alloc<Const>();
       double num = ast[1]->getNumber();
       if (isSInteger32(num)) {
-        ret->value.type = WasmType::i32;
-        ret->value.i32 = toSInteger32(num);
+        ret->value = Literal(int32_t(toSInteger32(num)));
       } else if (isUInteger32(num)) {
-        ret->value.type = WasmType::i32;
-        ret->value.i32 = toUInteger32(num);
+        ret->value = Literal(uint32_t(toUInteger32(num)));
       } else {
-        ret->value.type = WasmType::f64;
-        ret->value.f64 = num;
+        ret->value = Literal(num);
       }
       ret->type = ret->value.type;
       return ret;
@@ -919,8 +915,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
       ret->offset = 0;
       ret->align = ret->bytes;
       auto ptr = allocator.alloc<Const>();
-      ptr->value.type = WasmType::i32; // XXX for wasm64, need 64
-      ptr->value.i32 = global.address;
+      ptr->value = Literal(int32_t(global.address)); // XXX for wasm64, need 64
       ret->ptr = ptr;
       ret->type = global.type;
       return ret;
@@ -1446,8 +1441,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
       unsigned addr = ptr[1]->getInteger();
       unsigned shifted = addr << shifts;
       auto ret = allocator.alloc<Const>();
-      ret->value.type = WasmType::i32;
-      ret->value.i32 = shifted;
+      ret->value = Literal(int32_t(shifted));
       return (Expression*)ret;
     }
     abort_on("bad processUnshifted", ptr);
diff --git a/src/binaryen-shell.cpp b/src/binaryen-shell.cpp
index a96716a02..9bc672dd7 100644
--- a/src/binaryen-shell.cpp
+++ b/src/binaryen-shell.cpp
@@ -224,6 +224,19 @@ struct Invocation {
   }
 };
 
+static void verify_result(Literal a, Literal b) {
+  if (a == b) return;
+  // accept equal nans if equal in all bits
+  assert(a.type == b.type);
+  if (a.type == f32) {
+    assert(a.reinterpreti32() == b.reinterpreti32());
+  } else if (a.type == f64) {
+    assert(a.reinterpreti64() == b.reinterpreti64());
+  } else {
+    abort();
+  }
+}
+
 static void run_asserts(size_t* i, bool* checked, AllocatingModule* wasm,
                         Element* root,
                         std::unique_ptr<SExpressionWasmBuilder>* builder,
@@ -300,11 +313,11 @@ static void run_asserts(size_t* i, bool* checked, AllocatingModule* wasm,
                                  ->dyn_cast<Const>()
                                  ->value;
           std::cerr << "seen " << result << ", expected " << expected << '\n';
-          assert(expected == result);
+          verify_result(expected, result);
         } else {
           Literal expected;
           std::cerr << "seen " << result << ", expected " << expected << '\n';
-          assert(expected == result);
+          verify_result(expected, result);
         }
       }
       if (id == ASSERT_TRAP) assert(trapped);
diff --git a/src/parsing.h b/src/parsing.h
index 1ea0ec254..ed2f65b59 100644
--- a/src/parsing.h
+++ b/src/parsing.h
@@ -29,12 +29,12 @@ namespace wasm {
 Expression* parseConst(cashew::IString s, WasmType type, MixedArena& allocator) {
   const char *str = s.str;
   auto ret = allocator.alloc<Const>();
-  ret->type = ret->value.type = type;
+  ret->type = type;
   if (isWasmTypeFloat(type)) {
     if (s == INFINITY_) {
       switch (type) {
-        case f32: ret->value.f32 = std::numeric_limits<float>::infinity(); break;
-        case f64: ret->value.f64 = std::numeric_limits<double>::infinity(); break;
+        case f32: ret->value = Literal(std::numeric_limits<float>::infinity()); break;
+        case f64: ret->value = Literal(std::numeric_limits<double>::infinity()); break;
         default: return nullptr;
       }
       //std::cerr << "make constant " << str << " ==> " << ret->value << '\n';
@@ -42,8 +42,8 @@ Expression* parseConst(cashew::IString s, WasmType type, MixedArena& allocator)
     }
     if (s == NEG_INFINITY) {
       switch (type) {
-        case f32: ret->value.f32 = -std::numeric_limits<float>::infinity(); break;
-        case f64: ret->value.f64 = -std::numeric_limits<double>::infinity(); break;
+        case f32: ret->value = Literal(-std::numeric_limits<float>::infinity()); break;
+        case f64: ret->value = Literal(-std::numeric_limits<double>::infinity()); break;
         default: return nullptr;
       }
       //std::cerr << "make constant " << str << " ==> " << ret->value << '\n';
@@ -51,8 +51,8 @@ Expression* parseConst(cashew::IString s, WasmType type, MixedArena& allocator)
     }
     if (s == NAN_) {
       switch (type) {
-        case f32: ret->value.f32 = std::nan(""); break;
-        case f64: ret->value.f64 = std::nan(""); break;
+        case f32: ret->value = Literal(float(std::nan(""))); break;
+        case f64: ret->value = Literal(double(std::nan(""))); break;
         default: return nullptr;
       }
       //std::cerr << "make constant " << str << " ==> " << ret->value << '\n';
@@ -80,8 +80,7 @@ Expression* parseConst(cashew::IString s, WasmType type, MixedArena& allocator)
           }
           if (negative) pattern |= 0x80000000U;
           if (!isnan(bit_cast<float>(pattern))) pattern |= 1U;
-          ret->value.f32 = bit_cast<float>(pattern);
-          assert(isnan(ret->value.f32));
+          ret->value = Literal(pattern).castToF32();
           break;
         }
         case f64: {
@@ -95,8 +94,7 @@ Expression* parseConst(cashew::IString s, WasmType type, MixedArena& allocator)
           }
           if (negative) pattern |= 0x8000000000000000ULL;
           if (!isnan(bit_cast<double>(pattern))) pattern |= 1ULL;
-          ret->value.f64 = bit_cast<double>(pattern);
-          assert(isnan(ret->value.f64));
+          ret->value = Literal(pattern).castToF64();
           break;
         }
         default: return nullptr;
@@ -106,8 +104,8 @@ Expression* parseConst(cashew::IString s, WasmType type, MixedArena& allocator)
     }
     if (s == NEG_NAN) {
       switch (type) {
-        case f32: ret->value.f32 = -std::nan(""); break;
-        case f64: ret->value.f64 = -std::nan(""); break;
+        case f32: ret->value = Literal(float(-std::nan(""))); break;
+        case f64: ret->value = Literal(double(-std::nan(""))); break;
         default: return nullptr;
       }
       //std::cerr << "make constant " << str << " ==> " << ret->value << '\n';
@@ -122,12 +120,12 @@ Expression* parseConst(cashew::IString s, WasmType type, MixedArena& allocator)
         std::istringstream istr(str);
         uint32_t temp;
         istr >> std::hex >> temp;
-        ret->value.i32 = negative ? -temp : temp;
+        ret->value = Literal(negative ? -temp : temp);
       } else {
         std::istringstream istr(str);
         int32_t temp;
         istr >> temp;
-        ret->value.i32 = temp;
+        ret->value = Literal(temp);
       }
       break;
     }
@@ -138,29 +136,28 @@ Expression* parseConst(cashew::IString s, WasmType type, MixedArena& allocator)
         std::istringstream istr(str);
         uint64_t temp;
         istr >> std::hex >> temp;
-        ret->value.i64 = negative ? -temp : temp;
+        ret->value = Literal(negative ? -temp : temp);
       } else {
         std::istringstream istr(str);
         int64_t temp;
         istr >> temp;
-        ret->value.i64 = temp;
+        ret->value = Literal(temp);
       }
       break;
     }
     case f32: {
       char *end;
-      ret->value.f32 = strtof(str, &end);
-      assert(!isnan(ret->value.f32));
+      ret->value = Literal(strtof(str, &end));
       break;
     }
     case f64: {
       char *end;
-      ret->value.f64 = strtod(str, &end);
-      assert(!isnan(ret->value.f64));
+      ret->value = Literal(strtod(str, &end));
       break;
     }
     default: return nullptr;
   }
+  assert(ret->value.type == type);
   //std::cerr << "make constant " << str << " ==> " << ret->value << '\n';
   return ret;
 }
diff --git a/src/s2wasm.h b/src/s2wasm.h
index abfb4a92d..2a2a36028 100644
--- a/src/s2wasm.h
+++ b/src/s2wasm.h
@@ -744,7 +744,7 @@ class S2WasmBuilder {
               // may be a relocation
               auto curr = allocator.alloc<Const>();
               curr->type = curr->value.type = i32;
-              getConst((uint32_t*)&curr->value.i32);
+              getConst((uint32_t*)curr->value.geti32Ptr());
               setOutput(curr, assign);
             } else {
               cashew::IString str = getStr();
diff --git a/src/wasm-binary.h b/src/wasm-binary.h
index 1cc6f0673..de4747d84 100644
--- a/src/wasm-binary.h
+++ b/src/wasm-binary.h
@@ -647,7 +647,14 @@ public:
         return breakStack.size() - 1 - i;
       }
     }
+printf("bad!\n");
     std::cerr << "bad break: " << name << std::endl;
+
+
+
+assert(0);
+
+
     abort();
   }
 
@@ -782,10 +789,10 @@ public:
     recurse(curr->value);
   }
   void visitConst(Const *curr) {
-    if (debug) std::cerr << "zz node: Const" << std::endl;
+    if (debug) std::cerr << "zz node: Const" << curr << " : " << curr->type << std::endl;
     switch (curr->type) {
       case i32: {
-        uint32_t value = curr->value.i32;
+        uint32_t value = curr->value.geti32();
         if (value <= 255) {
           o << int8_t(BinaryConsts::I8Const) << uint8_t(value);
           break;
@@ -794,19 +801,20 @@ public:
         break;
       }
       case i64: {
-        o << int8_t(BinaryConsts::I64Const) << curr->value.i64;
+        o << int8_t(BinaryConsts::I64Const) << curr->value.geti64();
         break;
       }
       case f32: {
-        o << int8_t(BinaryConsts::F32Const) << curr->value.f32;
+        o << int8_t(BinaryConsts::F32Const) << curr->value.getf32();
         break;
       }
       case f64: {
-        o << int8_t(BinaryConsts::F64Const) << curr->value.f64;
+        o << int8_t(BinaryConsts::F64Const) << curr->value.getf64();
         break;
       }
       default: abort();
     }
+    if (debug) std::cerr << "zz const node done.\n";
   }
   void visitUnary(Unary *curr) {
     if (debug) std::cerr << "zz node: Unary" << std::endl;
@@ -1480,14 +1488,14 @@ public:
   }
   bool maybeVisitImpl(Const *curr, uint8_t code) {
     switch (code) {
-      case BinaryConsts::I8Const:  curr->value.i32 = getInt8();    curr->type = i32; break;
-      case BinaryConsts::I32Const: curr->value.i32 = getInt32();   curr->type = i32; break;
-      case BinaryConsts::I64Const: curr->value.i64 = getInt64();   curr->type = i64; break;
-      case BinaryConsts::F32Const: curr->value.f32 = getFloat32(); curr->type = f32; break;
-      case BinaryConsts::F64Const: curr->value.f64 = getFloat64(); curr->type = f64; break;
+      case BinaryConsts::I8Const:  curr->value = Literal(int32_t(getInt8())); break;
+      case BinaryConsts::I32Const: curr->value = Literal(getInt32()); break;
+      case BinaryConsts::I64Const: curr->value = Literal(getInt64()); break;
+      case BinaryConsts::F32Const: curr->value = Literal(getFloat32()); break;
+      case BinaryConsts::F64Const: curr->value = Literal(getFloat64()); break;
       default: return false;
     }
-    curr->value.type = curr->type;
+    curr->type = curr->value.type;
     if (debug) std::cerr << "zz node: Const" << std::endl;
     return true;
   }
diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h
index e0006f37b..693135962 100644
--- a/src/wasm-interpreter.h
+++ b/src/wasm-interpreter.h
@@ -378,13 +378,7 @@ private:
             case Clz: return Literal((int32_t)CountLeadingZeroes(v));
             case Ctz: return Literal((int32_t)CountTrailingZeroes(v));
             case Popcnt: return Literal((int32_t)PopCount(v));
-            case ReinterpretInt: {
-              float v = value.reinterpretf32();
-              if (isnan(v)) {
-                return Literal(Literal(value.geti32() | 0x7f800000).reinterpretf32());
-              }
-              return Literal(value.reinterpretf32());
-            }
+            case ReinterpretInt: return value.castToF32();
             case ExtendSInt32: return Literal(int64_t(value.geti32()));
             case ExtendUInt32: return Literal(uint64_t((uint32_t)value.geti32()));
             case ConvertUInt32: return curr->type == f32 ? Literal(float(uint32_t(value.geti32()))) : Literal(double(uint32_t(value.geti32())));
@@ -399,9 +393,7 @@ private:
             case Ctz: return Literal((int64_t)CountTrailingZeroes(v));
             case Popcnt: return Literal((int64_t)PopCount(v));
             case WrapInt64: return Literal(int32_t(value.geti64()));
-            case ReinterpretInt: {
-              return Literal(value.reinterpretf64());
-            }
+            case ReinterpretInt: return value.castToF64();
             case ConvertUInt64: return curr->type == f32 ? Literal(float((uint64_t)value.geti64())) : Literal(double((uint64_t)value.geti64()));
             case ConvertSInt64: return curr->type == f32 ? Literal(float(value.geti64())) : Literal(double(value.geti64()));
             default: abort();
@@ -411,8 +403,9 @@ private:
           float v = value.getf32();
           float ret;
           switch (curr->op) {
-            case Neg:     ret = -v; break;
-            case Abs:     ret = std::abs(v); break;
+            // operate on bits directly, to avoid signalling bit being set on a float
+            case Neg:     return Literal(value.reinterpreti32() ^ 0x80000000).castToF32(); break;
+            case Abs:     return Literal(value.reinterpreti32() & 0x7fffffff).castToF32(); break;
             case Ceil:    ret = std::ceil(v); break;
             case Floor:   ret = std::floor(v); break;
             case Trunc:   ret = std::trunc(v); break;
@@ -430,8 +423,9 @@ private:
           double v = value.getf64();
           double ret;
           switch (curr->op) {
-            case Neg:     ret = -v; break;
-            case Abs:     ret = std::abs(v); break;
+            // operate on bits directly, to avoid signalling bit being set on a float
+            case Neg:     return Literal(value.reinterpreti64() ^ 0x8000000000000000ULL).castToF64(); break;
+            case Abs:     return Literal(value.reinterpreti64() & 0x7fffffffffffffffULL).castToF64(); break;
             case Ceil:    ret = std::ceil(v); break;
             case Floor:   ret = std::floor(v); break;
             case Trunc:   ret = std::trunc(v); break;
@@ -568,10 +562,8 @@ private:
             case Sub:      ret = l - r; break;
             case Mul:      ret = l * r; break;
             case Div:      ret = l / r; break;
-            case CopySign: {
-              ret = std::copysign(l, r);
-              return Literal(ret);
-            }
+            // operate on bits directly, to avoid signalling bit being set on a float
+            case CopySign: return Literal((left.reinterpreti32() & 0x7fffffff) | (right.reinterpreti32() & 0x80000000)).castToF32(); break;
             case Min: {
               if (l == r && l == 0) ret = 1/l < 0 ? l : r;
               else ret = std::min(l, r);
@@ -599,10 +591,8 @@ private:
             case Sub:      ret = l - r; break;
             case Mul:      ret = l * r; break;
             case Div:      ret = l / r; break;
-            case CopySign: {
-              ret = std::copysign(l, r);
-              return Literal(ret);
-            }
+            // operate on bits directly, to avoid signalling bit being set on a float
+            case CopySign: return Literal((left.reinterpreti64() & 0x7fffffffffffffffUL) | (right.reinterpreti64() & 0x8000000000000000UL)).castToF64(); break;
             case Min: {
               if (l == r && l == 0) ret = 1/l < 0 ? l : r;
               else ret = std::min(l, r);
diff --git a/src/wasm.h b/src/wasm.h
index 6f889737b..7a792fda4 100644
--- a/src/wasm.h
+++ b/src/wasm.h
@@ -140,33 +140,52 @@ inline WasmType getReachableWasmType(WasmType a, WasmType b) {
 
 // Literals
 
-struct Literal {
+class Literal {
+public:
   WasmType type;
+
+private:
+  // store only integers, whose bits are deterministic. floats
+  // can have their signalling bit set, for example.
   union {
     int32_t i32;
     int64_t i64;
-    float f32;
-    double f64;
   };
 
-  Literal() : Literal(WasmType::none) {}
-  explicit Literal(WasmType type) : type(type) { memset(&f64, 0, sizeof(f64)); }
+public:
+  Literal() : type(WasmType::none), i64(0) {}
+  explicit Literal(WasmType type) : type(type), i64(0) {}
   explicit Literal(int32_t  init) : type(WasmType::i32), i32(init) {}
   explicit Literal(uint32_t init) : type(WasmType::i32), i32(init) {}
   explicit Literal(int64_t  init) : type(WasmType::i64), i64(init) {}
   explicit Literal(uint64_t init) : type(WasmType::i64), i64(init) {}
-  explicit Literal(float    init) : type(WasmType::f32), f32(init) {}
-  explicit Literal(double   init) : type(WasmType::f64), f64(init) {}
+  explicit Literal(float    init) : type(WasmType::f32), i32(bit_cast<int32_t>(init)) {}
+  explicit Literal(double   init) : type(WasmType::f64), i64(bit_cast<int64_t>(init)) {}
+
+  Literal castToF32() {
+    assert(type == WasmType::i32);
+    Literal ret(i32);
+    ret.type = f32;
+    return ret;
+  }
+  Literal castToF64() {
+    assert(type == WasmType::i64);
+    Literal ret(i64);
+    ret.type = f64;
+    return ret;
+  }
 
   int32_t geti32() { assert(type == WasmType::i32); return i32; }
   int64_t geti64() { assert(type == WasmType::i64); return i64; }
-  float   getf32() { assert(type == WasmType::f32); return f32; }
-  double  getf64() { assert(type == WasmType::f64); return f64; }
+  float   getf32() { assert(type == WasmType::f32); return bit_cast<float>(i32); }
+  double  getf64() { assert(type == WasmType::f64); return bit_cast<double>(i64); }
+
+  int32_t* geti32Ptr() { assert(type == WasmType::i32); return &i32; } // careful!
 
   int32_t reinterpreti32() { assert(type == WasmType::f32); return i32; }
   int64_t reinterpreti64() { assert(type == WasmType::f64); return i64; }
-  float   reinterpretf32() { assert(type == WasmType::i32); return f32; }
-  double  reinterpretf64() { assert(type == WasmType::i64); return f64; }
+  float   reinterpretf32() { assert(type == WasmType::i32); return bit_cast<float>(i32); }
+  double  reinterpretf64() { assert(type == WasmType::i64); return bit_cast<double>(i64); }
 
   int64_t getInteger() {
     switch (type) {
@@ -178,8 +197,8 @@ struct Literal {
 
   double getFloat() {
     switch (type) {
-      case WasmType::f32: return f32;
-      case WasmType::f64: return f64;
+      case WasmType::f32: return getf32();
+      case WasmType::f64: return getf64();
       default: abort();
     }
   }
@@ -189,10 +208,9 @@ struct Literal {
     switch (type) {
       case WasmType::none: return true;
       case WasmType::i32: return i32 == other.i32;
+      case WasmType::f32: return getf32() == other.getf32();
       case WasmType::i64: return i64 == other.i64;
-      // reinterpret floating-point, to avoid nan != nan
-      case WasmType::f32: return reinterpreti32() == other.reinterpreti32();
-      case WasmType::f64: return reinterpreti64() == other.reinterpreti64();
+      case WasmType::f64: return getf64() == other.getf64();
       default: abort();
     }
   }
@@ -244,8 +262,8 @@ struct Literal {
       case none: o << "?"; break;
       case WasmType::i32: o << literal.i32; break;
       case WasmType::i64: o << literal.i64; break;
-      case WasmType::f32: literal.printFloat(o, literal.f32); break;
-      case WasmType::f64: literal.printDouble(o, literal.f64); break;
+      case WasmType::f32: literal.printFloat(o, literal.getf32()); break;
+      case WasmType::f64: literal.printDouble(o, literal.getf64()); break;
       default: WASM_UNREACHABLE();
     }
     restoreNormalColor(o);
diff --git a/src/wasm2asm.h b/src/wasm2asm.h
index 76ccbf8b6..c87e81eec 100644
--- a/src/wasm2asm.h
+++ b/src/wasm2asm.h
@@ -915,21 +915,21 @@ Ref Wasm2AsmBuilder::processFunctionBody(Expression* curr, IString result) {
     }
     Ref visitConst(Const *curr) {
       switch (curr->type) {
-        case i32: return ValueBuilder::makeInt(curr->value.i32);
+        case i32: return ValueBuilder::makeInt(curr->value.geti32());
         case f32: {
           Ref ret = ValueBuilder::makeCall(MATH_FROUND);
           Const fake;
-          fake.value = Literal(double(curr->value.f32));
+          fake.value = Literal(double(curr->value.getf32()));
           fake.type = f64;
           ret[2]->push_back(visitConst(&fake));
           return ret;
         }
         case f64: {
-          double d = curr->value.f64;
+          double d = curr->value.getf64();
           if (d == 0 && std::signbit(d)) { // negative zero
             return ValueBuilder::makeUnary(PLUS, ValueBuilder::makeUnary(MINUS, ValueBuilder::makeDouble(0)));
           }
-          return ValueBuilder::makeUnary(PLUS, ValueBuilder::makeDouble(curr->value.f64));
+          return ValueBuilder::makeUnary(PLUS, ValueBuilder::makeDouble(curr->value.getf64()));
         }
         default: abort();
       }