fix optimizing two shifts into one; if the number of effective shifts overflows, it is not vali to just add them

Author: kripken

src/ast/bits.h

@@ -42,11 +42,20 @@ struct Bits {
 
   // gets the number of effective shifts a shift operation does. In
   // wasm, only 5 bits matter for 32-bit shifts, and 6 for 64.
-  static uint32_t getEffectiveShifts(Const* amount) {
+  static Index getEffectiveShifts(Index amount, WasmType type) {
+    if (type == i32) {
+      return amount & 31;
+    } else if (type == i64) {
+      return amount & 63;
+    }
+    WASM_UNREACHABLE();
+  }
+
+  static Index getEffectiveShifts(Const* amount) {
     if (amount->type == i32) {
-      return amount->value.geti32() & 31;
+      return getEffectiveShifts(amount->value.geti32(), i32);
     } else if (amount->type == i64) {
-      return amount->value.geti64() & 63;
+      return getEffectiveShifts(amount->value.geti64(), i64);
     }
     WASM_UNREACHABLE();
   }

src/ast/literal-utils.h

@@ -23,21 +23,31 @@ namespace wasm {
 
 namespace LiteralUtils {
 
-inline Expression* makeZero(WasmType type, Module& wasm) {
-  Literal value;
+inline Literal makeLiteralFromInt32(int32_t x, WasmType type) {
   switch (type) {
-    case i32: value = Literal(int32_t(0)); break;
-    case i64: value = Literal(int64_t(0)); break;
-    case f32: value = Literal(float(0)); break;
-    case f64: value = Literal(double(0)); break;
+    case i32: return Literal(int32_t(x)); break;
+    case i64: return Literal(int64_t(x)); break;
+    case f32: return Literal(float(x)); break;
+    case f64: return Literal(double(x)); break;
     default: WASM_UNREACHABLE();
   }
+}
+
+inline Literal makeLiteralZero(WasmType type) {
+  return makeLiteralFromInt32(0, type);
+}
+
+inline Expression* makeFromInt32(int32_t x, WasmType type, Module& wasm) {
   auto* ret = wasm.allocator.alloc<Const>();
-  ret->value = value;
-  ret->type = value.type;
+  ret->value = makeLiteralFromInt32(x, type);
+  ret->type = type;
   return ret;
 }
 
+inline Expression* makeZero(WasmType type, Module& wasm) {
+  return makeFromInt32(0, type, wasm);
+}
+
 } // namespace LiteralUtils
 
 } // namespace wasm

src/passes/OptimizeInstructions.cpp

@@ -29,6 +29,7 @@
 #include <ast/effects.h>
 #include <ast/manipulation.h>
 #include <ast/properties.h>
+#include <ast/literal-utils.h>
 
 namespace wasm {
 
@@ -533,9 +534,16 @@ struct OptimizeInstructions : public WalkerPass<PostWalker<OptimizeInstructions,
               } else if (left->op == OrInt32) {
                 leftRight->value = leftRight->value.or_(right->value);
                 return left;
-              } else if (left->op == ShlInt32 || left->op == ShrUInt32 || left->op == ShrSInt32) {
-                leftRight->value = leftRight->value.add(right->value);
-                return left;
+              } else if (left->op == ShlInt32 || left->op == ShrUInt32 || left->op == ShrSInt32 ||
+                         left->op == ShlInt64 || left->op == ShrUInt64 || left->op == ShrSInt64) {
+                // shifts only use an effective amount from the constant, so adding must
+                // be done carefully
+                auto total = Bits::getEffectiveShifts(leftRight) + Bits::getEffectiveShifts(right);
+                if (total == Bits::getEffectiveShifts(total, left->type)) {
+                  // no overflow, we can do this
+                  leftRight->value = LiteralUtils::makeLiteralFromInt32(total, left->type);
+                  return left;
+                } // TODO: handle overflows
               }
             }
           }

test/passes/optimize-instructions.txt

@@ -6,6 +6,7 @@
  (type $4 (func (param i32 i32)))
  (type $5 (func (param i32)))
  (type $6 (func (param i32 i32) (result i32)))
+ (type $7 (func (param i64) (result i64)))
  (memory $0 0)
  (export "load-off-2" (func $load-off-2))
  (func $f (type $0) (param $i1 i32) (param $i2 i64)
@@ -734,7 +735,7 @@
    (i32.shr_s
     (i32.shl
      (i32.const 32)
-     (i32.const 59)
+     (i32.const 27)
     )
     (i32.const 24)
    )
@@ -1083,19 +1084,19 @@
   (drop
    (i32.shl
     (get_local $0)
-    (i32.const 211)
+    (i32.const 19)
    )
   )
   (drop
    (i32.shr_s
     (get_local $0)
-    (i32.const 211)
+    (i32.const 19)
    )
   )
   (drop
    (i32.shr_u
     (get_local $0)
-    (i32.const 211)
+    (i32.const 19)
    )
   )
   (drop
@@ -2024,4 +2025,34 @@
    (i32.const 255)
   )
  )
+ (func $shifts-square-overflow (type $3) (param $x i32) (result i32)
+  (i32.shr_u
+   (i32.shr_u
+    (get_local $x)
+    (i32.const 65535)
+   )
+   (i32.const 32767)
+  )
+ )
+ (func $shifts-square-no-overflow-small (type $3) (param $x i32) (result i32)
+  (i32.shr_u
+   (get_local $x)
+   (i32.const 9)
+  )
+ )
+ (func $shifts-square-overflow-64 (type $7) (param $x i64) (result i64)
+  (i64.shr_u
+   (i64.shr_u
+    (get_local $x)
+    (i64.const 65535)
+   )
+   (i64.const 64767)
+  )
+ )
+ (func $shifts-square-no-overflow-small-64 (type $7) (param $x i64) (result i64)
+  (i64.shr_u
+   (get_local $x)
+   (i64.const 9)
+  )
+ )
 )

test/passes/optimize-instructions.wast

@@ -2451,4 +2451,40 @@
     (i32.const 255)
    )
   )
+  (func $shifts-square-overflow (param $x i32) (result i32)
+   (i32.shr_u
+    (i32.shr_u
+     (get_local $x)
+     (i32.const 65535) ;; 31 bits effectively
+    )
+    (i32.const 32767) ;; also 31 bits, so two shifts that force the value into nothing for sure
+   )
+  )
+  (func $shifts-square-no-overflow-small (param $x i32) (result i32)
+   (i32.shr_u
+    (i32.shr_u
+     (get_local $x)
+     (i32.const 1031) ;; 7 bits effectively
+    )
+    (i32.const 4098) ;; 2 bits effectively
+   )
+  )
+  (func $shifts-square-overflow-64 (param $x i64) (result i64)
+   (i64.shr_u
+    (i64.shr_u
+     (get_local $x)
+     (i64.const 65535) ;; 63 bits effectively
+    )
+    (i64.const 64767) ;; also 63 bits, so two shifts that force the value into nothing for sure
+   )
+  )
+  (func $shifts-square-no-overflow-small-64 (param $x i64) (result i64)
+   (i64.shr_u
+    (i64.shr_u
+     (get_local $x)
+     (i64.const 1031) ;; 7 bits effectively
+    )
+    (i64.const 4098) ;; 2 bits effectively
+   )
+  )
 )