OptimizeAddedConstants: Handle a final added constant properly (#6115)

We had an assert there that was wrong. In fact the assert is just in one of two code paths,
and an optional one: the end situation is we have an expression and a constant to add to it,
and the assert was in the case that the expression is a Const so we can do the add at
compile time (the other code path does the add at runtime). This code path is optional as
Precompute would do such compile-time addition anyhow, but it is nice to fix and leave that
path so that this pass emits fully optimal code.

2 files changed, 36 insertions, 9 deletions

diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp
index feaee4d44..28eaf0fba 100644
--- a/src/passes/OptimizeInstructions.cpp
+++ b/src/passes/OptimizeInstructions.cpp
@@ -2949,19 +2949,21 @@ private:
     if (constant == 0ULL) {
       return walked; // nothing more to do
     }
+    // Add the total constant value we computed to the value remaining here.
+    // Note that if the value is 32 bits then |makeFromInt64| will wrap to 32
+    // bits for us; as all the operations before us and the add below us are
+    // adds and subtracts, any overflow is not a problem.
+    auto toAdd = Literal::makeFromInt64(constant, type);
     if (auto* c = walked->dynCast<Const>()) {
-      assert(c->value.isZero());
-      // Accumulated 64-bit constant value in 32-bit context will be wrapped
-      // during downcasting. So it's valid unification for 32-bit and 64-bit
-      // values.
-      c->value = Literal::makeFromInt64(constant, type);
+      // This is a constant, so just add it immediately (we could also leave
+      // this for Precompute, in principle).
+      c->value = c->value.add(toAdd);
       return c;
     }
     Builder builder(*getModule());
-    return builder.makeBinary(
-      Abstract::getBinary(type, Abstract::Add),
-      walked,
-      builder.makeConst(Literal::makeFromInt64(constant, type)));
+    return builder.makeBinary(Abstract::getBinary(type, Abstract::Add),
+                              walked,
+                              builder.makeConst(toAdd));
   }
 
   // Given an i64.wrap operation, see if we can remove it. If all the things
diff --git a/test/lit/passes/optimize-instructions-mvp.wast b/test/lit/passes/optimize-instructions-mvp.wast
index 953ccd37c..550db4094 100644
--- a/test/lit/passes/optimize-instructions-mvp.wast
+++ b/test/lit/passes/optimize-instructions-mvp.wast
@@ -16949,4 +16949,29 @@
       )
     )
   )
+
+  ;; CHECK:      (func $added-constants-remaining-constant (result i32)
+  ;; CHECK-NEXT:  (i32.const 32)
+  ;; CHECK-NEXT: )
+  (func $added-constants-remaining-constant (result i32)
+    ;; optimizeAddedConstants will simplify this step by step and end up with
+    ;; both an accumulated value and a constant to add it to (the 1 at the
+    ;; bottom). We should not hit an assert here and return the proper value,
+    ;; 32. (This is tricky for optimizeAddedConstants because of the shift that
+    ;; does nothing, which it correctly ignores, but it also leads to having
+    ;; something to add at the very end of the process.)
+    (i32.sub              ;; This subtracts 33 by 1 to get 32.
+      (i32.add            ;; This adds 1 to 32 to get 33.
+        (i32.shl          ;; This shift by 32 does nothing, so it is 1.
+         (i32.const 1)
+         (i32.add         ;; This is 32
+           (i32.const 0)
+           (i32.const 32)
+         )
+       )
+       (i32.const 32)
+      )
+      (i32.const 1)
+    )
+  )
 )