Change the Literal class's operator== to be bitwise (#1661)

The change means that nan values will be compared bitwise when writing A == B, and so the float rule of a nan is different from itself would not apply.

I think this is a safer default. In particular this PR fixes a fuzz bug in the rse pass, which placed Literals in a hash table, and due to nan != nan, an infinite loop... Also, looks like we really want a bitwise comparison pretty much everywhere anyhow, as can be seen in the diff here. Really the single place we need a floaty comparison is in the intepreter where we implement f32.eq etc., and there the code was already using the proper code path anyhow.

Author: kripken

src/ir/ExpressionAnalyzer.cpp

@@ -257,7 +257,7 @@ bool ExpressionAnalyzer::flexibleEqual(Expression* left, Expression* right, Expr
         break;
       }
       case Expression::Id::ConstId: {
-        if (!left->cast<Const>()->value.bitwiseEqual(right->cast<Const>()->value)) {
+        if (left->cast<Const>()->value != right->cast<Const>()->value) {
           return false;
         }
         break;

src/literal.h

@@ -71,9 +71,11 @@ class Literal {
   int64_t getInteger() const;
   double getFloat() const;
   int64_t getBits() const;
+  // Equality checks for the type and the bits, so a nan float would
+  // be compared bitwise (which means that a Literal containing a nan
+  // would be equal to itself, if the bits are equal).
   bool operator==(const Literal& other) const;
   bool operator!=(const Literal& other) const;
-  bool bitwiseEqual(const Literal& other) const;
 
   static uint32_t NaNPayload(float f);
   static uint64_t NaNPayload(double f);
@@ -130,6 +132,9 @@ class Literal {
   Literal rotL(const Literal& other) const;
   Literal rotR(const Literal& other) const;
 
+  // Note that these functions perform equality checks based
+  // on the type of the literal, so that (unlike the == operator)
+  // a float nan would not be identical to itself.
   Literal eq(const Literal& other) const;
   Literal ne(const Literal& other) const;
   Literal ltS(const Literal& other) const;

src/passes/Precompute.cpp

@@ -316,7 +316,7 @@ struct Precompute : public WalkerPass<PostWalker<Precompute, UnifiedExpressionVi
             value = curr; // this is the first
             first = false;
           } else {
-            if (!value.bitwiseEqual(curr)) {
+            if (value != curr) {
               // not the same, give up
               value = Literal();
               break;

src/tools/execution-results.h

@@ -78,7 +78,7 @@ struct ExecutionResults {
         abort();
       }
       std::cout << "[fuzz-exec] comparing " << name << '\n';
-      if (!results[name].bitwiseEqual(other.results[name])) {
+      if (results[name] != other.results[name]) {
         std::cout << "not identical!\n";
         abort();
       }

src/tools/wasm-shell.cpp

@@ -201,14 +201,14 @@ static void run_asserts(Name moduleName, size_t* i, bool* checked, Module* wasm,
                                  ->dynCast<Const>()
                                  ->value;
           std::cerr << "seen " << result << ", expected " << expected << '\n';
-          if (!expected.bitwiseEqual(result)) {
+          if (expected != result) {
             std::cout << "unexpected, should be identical\n";
             abort();
           }
         } else {
           Literal expected;
           std::cerr << "seen " << result << ", expected " << expected << '\n';
-          if (!expected.bitwiseEqual(result)) {
+          if (expected != result) {
             std::cout << "unexpected, should be identical\n";
             abort();
           }

src/wasm/literal.cpp

@@ -81,26 +81,14 @@ int64_t Literal::getBits() const {
 
 bool Literal::operator==(const Literal& other) const {
   if (type != other.type) return false;
-  switch (type) {
-    case Type::none: return true;
-    case Type::i32: return i32 == other.i32;
-    case Type::f32: return getf32() == other.getf32();
-    case Type::i64: return i64 == other.i64;
-    case Type::f64: return getf64() == other.getf64();
-    default: abort();
-  }
+  if (type == none) return true;
+  return getBits() == other.getBits();
 }
 
 bool Literal::operator!=(const Literal& other) const {
   return !(*this == other);
 }
 
-bool Literal::bitwiseEqual(const Literal& other) const {
-  if (type != other.type) return false;
-  if (type == none) return true;
-  return getBits() == other.getBits();
-}
-
 uint32_t Literal::NaNPayload(float f) {
   assert(std::isnan(f) && "expected a NaN");
   // SEEEEEEE EFFFFFFF FFFFFFFF FFFFFFFF

test/passes/rse.txt

@@ -430,4 +430,33 @@
    )
   )
  )
+ (func $fuzz-nan (; 18 ;) (type $2)
+  (local $0 f64)
+  (local $1 f64)
+  (block $block
+   (br_if $block
+    (i32.const 0)
+   )
+   (loop $loop
+    (set_local $1
+     (get_local $0)
+    )
+    (set_local $0
+     (f64.const -nan:0xfffffffffff87)
+    )
+    (br_if $loop
+     (i32.const 1)
+    )
+   )
+  )
+  (set_local $0
+   (get_local $1)
+  )
+  (if
+   (i32.const 0)
+   (drop
+    (get_local $0)
+   )
+  )
+ )
 )

test/passes/rse.wast

@@ -248,5 +248,34 @@
     )
    )
   )
+  (func $fuzz-nan
+   (local $0 f64)
+   (local $1 f64)
+   (block $block
+    (br_if $block
+     (i32.const 0)
+    )
+    (loop $loop
+     (set_local $1
+      (get_local $0)
+     )
+     (set_local $0
+      (f64.const -nan:0xfffffffffff87)
+     )
+     (br_if $loop
+      (i32.const 1)
+     )
+    )
+   )
+   (set_local $0 ;; make them equal
+    (get_local $1)
+   )
+   (if
+    (i32.const 0)
+    (set_local $1 ;; we can drop this
+     (get_local $0)
+    )
+   )
+  )
 )