Interpreter: Don't change NaN bits when dividing by 1

src/wasm/literal.cpp

@@ -868,6 +868,18 @@ Literal Literal::div(const Literal& other) const {
         case FP_INFINITE: // fallthrough
         case FP_NORMAL:   // fallthrough
         case FP_SUBNORMAL:
+          // Special-case division by 1. nan / 1 can change nan bits per the
+          // wasm spec, but it is ok to just return that original nan, and we
+          // do that here so that we are consistent with the optimization of
+          // removing the / 1 and leaving just the nan. That is, if we just
+          // do a normal divide and the CPU decides to change the bits, we'd
+          // give a different result on optimized code, which would look like
+          // it was a bad optimization. So out of all the valid results to
+          // return here, return the simplest one that is consistent with
+          // optimization.
+          if (rhs == 1) {
+            return Literal(lhs);
+          }
           return Literal(lhs / rhs);
         default:
           WASM_UNREACHABLE("invalid fp classification");
@@ -896,6 +908,10 @@ Literal Literal::div(const Literal& other) const {
         case FP_INFINITE: // fallthrough
         case FP_NORMAL:   // fallthrough
         case FP_SUBNORMAL:
+          // See above comment on f32.
+          if (rhs == 1) {
+            return Literal(lhs);
+          }
           return Literal(lhs / rhs);
         default:
           WASM_UNREACHABLE("invalid fp classification");

test/passes/fuzz-exec_O.txt

@@ -30,3 +30,22 @@
 [trap final > memory: 18446744073709551615 > 65514]
 [fuzz-exec] comparing func_0
 [fuzz-exec] comparing func_1
+[fuzz-exec] calling func_113
+[LoggingExternalInterface logging -nan:0x23017a]
+[fuzz-exec] note result: func_113 => 113
+(module
+ (type $f32_=>_none (func (param f32)))
+ (type $none_=>_i64 (func (result i64)))
+ (import "fuzzing-support" "log-f32" (func $fimport$0 (param f32)))
+ (export "func_113" (func $0))
+ (func $0 (; has Stack IR ;) (result i64)
+  (call $fimport$0
+   (f32.const -nan:0x23017a)
+  )
+  (i64.const 113)
+ )
+)
+[fuzz-exec] calling func_113
+[LoggingExternalInterface logging -nan:0x23017a]
+[fuzz-exec] note result: func_113 => 113
+[fuzz-exec] comparing func_113

test/passes/fuzz-exec_O.wast

@@ -20,4 +20,19 @@
   )
  )
 )
+(module
+ (type $f32_=>_none (func (param f32)))
+ (type $none_=>_i64 (func (result i64)))
+ (import "fuzzing-support" "log-f32" (func $fimport$0 (param f32)))
+ (export "func_113" (func $0))
+ (func $0 (result i64)
+  (call $fimport$0
+   (f32.div
+    (f32.const -nan:0x23017a) ;; div by 1 can be removed, leaving this nan
+    (f32.const 1)             ;; as it is. wasm semantics allow nan bits to
+   )                          ;; change, but the interpreter should not do so,
+  )                           ;; so that it does not fail on that opt.
+  (i64.const 113)
+ )
+)
 

test/spec/old_float_exprs.wast

@@ -133,8 +133,10 @@
     (f64.div (local.get $x) (f64.const 1.0)))
 )
 
-(assert_return (invoke "f32.no_fold_div_one" (f32.const nan:0x200000)) (f32.const nan:0x600000))
-(assert_return (invoke "f64.no_fold_div_one" (f64.const nan:0x4000000000000)) (f64.const nan:0xc000000000000))
+;; XXX BINARYEN: disable this test, as we have testing for the more strict property
+;;               of not changing the bits at all in our interpreter
+;; (assert_return (invoke "f32.no_fold_div_one" (f32.const nan:0x200000)) (f32.const nan:arithmetic))
+;; (assert_return (invoke "f64.no_fold_div_one" (f64.const nan:0x4000000000000)) (f64.const nan:arithmetic))
 
 ;; Test that x/-1.0 is not folded to -x.