Fix RemoveUnusedBrs bug on removed br_if value where condition interferes (#7586)

Fixes a regression from #7506 . That PR checked for effects, but we
forgot to also check for interactions between effects.

Author: kripken

src/passes/RemoveUnusedBrs.cpp

@@ -1290,14 +1290,32 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
           auto* last = curr->list[size - 1];
           if (auto* drop = secondLast->dynCast<Drop>()) {
             if (auto* br = drop->value->dynCast<Break>();
-                br && br->value && br->condition) {
-              if (br->name == curr->name) {
-                if (!EffectAnalyzer(passOptions, *getModule(), br->value)
-                       .hasUnremovableSideEffects()) {
-                  if (ExpressionAnalyzer::equal(br->value, last)) {
-                    // All conditions met, perform the update.
-                    drop->value = br->condition;
-                  }
+                br && br->value && br->condition && br->name == curr->name &&
+                ExpressionAnalyzer::equal(br->value, last)) {
+              // The value must have no effects, as we are removing one copy
+              // of it. Also, the condition must not interfere with that
+              // value, or it might change, e.g.
+              //
+              //   (drop
+              //     (br_if $block
+              //       (read a value)     ;; this original value is returned,
+              //       (write that value) ;; if we branch
+              //     )
+              //   )
+              //   (read a value)
+              // =>
+              //   (drop
+              //     (write that value)
+              //   )
+              //   (read a value)         ;; now the written value is used
+              auto valueEffects =
+                EffectAnalyzer(passOptions, *getModule(), br->value);
+              if (!valueEffects.hasUnremovableSideEffects()) {
+                auto conditionEffects =
+                  EffectAnalyzer(passOptions, *getModule(), br->condition);
+                if (!conditionEffects.invalidates(valueEffects)) {
+                  // All conditions met, perform the update.
+                  drop->value = br->condition;
                 }
               }
             }

test/lit/passes/remove-unused-brs.wast

@@ -466,6 +466,37 @@
     )
   )
 
+  ;; CHECK:      (func $restructure-br_if-condition-invalidates-6 (type $2) (result i32)
+  ;; CHECK-NEXT:  (local $temp i32)
+  ;; CHECK-NEXT:  (block $block (result i32)
+  ;; CHECK-NEXT:   (drop
+  ;; CHECK-NEXT:    (br_if $block
+  ;; CHECK-NEXT:     (local.get $temp)
+  ;; CHECK-NEXT:     (local.tee $temp
+  ;; CHECK-NEXT:      (i32.const 1)
+  ;; CHECK-NEXT:     )
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:   (local.get $temp)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $restructure-br_if-condition-invalidates-6 (result i32)
+    (local $temp i32)
+    ;; The br value is syntactically identical to the value at the end of the
+    ;; block, however, the local.tee changes that value so we cannot optimize.
+    (block $block (result i32)
+      (drop
+        (br_if $block
+          (local.get $temp)
+          (local.tee $temp
+            (i32.const 1)
+          )
+        )
+      )
+      (local.get $temp)
+    )
+  )
+
   ;; CHECK:      (func $restructure-select-no-multivalue (type $1)
   ;; CHECK-NEXT:  (tuple.drop 2
   ;; CHECK-NEXT:   (block $block (type $3) (result i32 i32)