ConstraintAnalysis: Fix some trivial asserts (#8893)

Author: kripken

src/ir/constraint.cpp

@@ -288,11 +288,7 @@ void BasicBlockConstraintMap::set(Index index, const Constraint& c) {
 
   // Clear the old state.
   eraseStaleRefs(index);
-  map[index].setProvesNothing();
-  // Note that this last line puts us in a temporarily invalid state: we do not
-  // normally store a constraint of proves-nothing in the map. Doing it this
-  // way, until approximateAnd adds the constraint, is more efficient than
-  // erasing and re-adding.
+  map.erase(index);
 
   // Apply the constraint.
   approximateAnd(index, c);

src/passes/ConstraintAnalysis.cpp

@@ -187,12 +187,14 @@ struct ConstraintAnalysis
       // of course not needed at this stage.)
       auto& constraints = block->contents.startConstraints;
       for (auto** currp : block->contents.actions) {
-        applyToConstraints(*currp, constraints);
-        if (constraints.unreachable) {
+        if (!constraints.unreachable) {
+          applyToConstraints(*currp, constraints);
+          optimizeExpression(currp, constraints);
+        } else {
+          // This is unreachable code: just mark it so.
           *currp = Builder(*getModule()).makeUnreachable();
           refinalize = true;
         }
-        optimizeExpression(currp, constraints);
       }
     }
 

test/lit/passes/constraint-analysis.wast

@@ -3472,4 +3472,62 @@
       (br $loop)
     )
   )
+
+  ;; CHECK:      (func $unreachable-loop (type $1)
+  ;; CHECK-NEXT:  (local $x i32)
+  ;; CHECK-NEXT:  (unreachable)
+  ;; CHECK-NEXT:  (unreachable)
+  ;; CHECK-NEXT: )
+  ;; OPTIN:      (func $unreachable-loop (type $1)
+  ;; OPTIN-NEXT:  (local $x i32)
+  ;; OPTIN-NEXT:  (unreachable)
+  ;; OPTIN-NEXT:  (unreachable)
+  ;; OPTIN-NEXT: )
+  (func $unreachable-loop
+    (local $x i32)
+    ;; The entire loop is unreachable. The control flow here should not cause any
+    ;; internal errors, and we can just optimize this to unreachable.
+    (unreachable)
+    (local.set $x
+      (loop $loop (result i32)
+        (i32.const 0)
+      )
+    )
+  )
+
+  ;; CHECK:      (func $set-silly (type $0) (param $x i32)
+  ;; CHECK-NEXT:  (local.set $x
+  ;; CHECK-NEXT:   (local.get $x)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (if
+  ;; CHECK-NEXT:   (i32.const 0)
+  ;; CHECK-NEXT:   (then
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  ;; OPTIN:      (func $set-silly (type $0) (param $x i32)
+  ;; OPTIN-NEXT:  (local.set $x
+  ;; OPTIN-NEXT:   (local.get $x)
+  ;; OPTIN-NEXT:  )
+  ;; OPTIN-NEXT:  (if
+  ;; OPTIN-NEXT:   (i32.const 0)
+  ;; OPTIN-NEXT:   (then
+  ;; OPTIN-NEXT:   )
+  ;; OPTIN-NEXT:  )
+  ;; OPTIN-NEXT: )
+  (func $set-silly (param $x i32)
+    ;; Setting x to x adds no information, and we do not store it in the IR. This
+    ;; must not lead to internal errors (we have asserts on not storing
+    ;; "proves-nothing" in the internal map, and if we wrote this, it would be
+    ;; that).
+    (local.set $x
+      (local.get $x)
+    )
+    ;; Add some control flow to exercise the assert.
+    (if
+      (i32.const 0)
+      (then)
+    )
+  )
 )
+