[Wasm GC] Fix RemoveUnusedModuleEffects struct field reads with call.without.effects

src/passes/RemoveUnusedModuleElements.cpp

@@ -38,6 +38,7 @@
 #include <memory>
 
 #include "ir/element-utils.h"
+#include "ir/find_all.h"
 #include "ir/intrinsics.h"
 #include "ir/module-utils.h"
 #include "ir/subtypes.h"
@@ -492,11 +493,36 @@ struct Analyzer {
     for (Index i = 0; i < new_->operands.size(); i++) {
       auto* operand = new_->operands[i];
       auto structField = StructField{type, i};
-      if (readStructFields.count(structField) ||
-          EffectAnalyzer(options, *module, operand).hasSideEffects()) {
-        // This data can be read, so just walk it. Or, this has side effects,
-        // which is tricky to reason about - the side effects must happen even
-        // if we never read the struct field - so give up and consider it used.
+
+      // If this struct field has already been read, then we should use the
+      // contents there now.
+      auto useOperandNow = readStructFields.count(structField);
+
+      // Side effects are tricky to reason about - the side effects must happen
+      // even if we never read the struct field - so give up and consider it
+      // used.
+      if (!useOperandNow) {
+        useOperandNow =
+          EffectAnalyzer(options, *module, operand).hasSideEffects();
+      }
+
+      // We must handle the call.without.effects intrinsic here in a special
+      // manner. That intrinsic is reported as having no side effects in
+      // EffectAnalyzer, but even though for optimization purposes we can ignore
+      // effects, the called code *is* actually reached, and it might have side
+      // effects. In other words, the point of the intrinsic is to temporarily
+      // ignore those effects during one phase of optimization. Or, put another
+      // way, the intrinsic lets us ignore the effects of computing some value,
+      // but we do still need to compute that value if it is received and used
+      // (if it is not received and used, other passes will remove it).
+      if (!useOperandNow) {
+        // To detect this, look for any call. A non-intrinsic call would have
+        // already been detected when we looked for side effects, so this will
+        // only notice intrinsic calls.
+        useOperandNow = !FindAll<Call>(operand).list.empty();
+      }
+
+      if (useOperandNow) {
         use(operand);
       } else {
         // This data does not need to be read now, but might be read later. Note

test/lit/passes/remove-unused-module-elements-refs.wast

@@ -1841,3 +1841,76 @@
   (func $f (type $void)
   )
 )
+
+;; The call.without.effects intrinsic reports itself as having no side effects.
+;; We do still need to consider the target as being called, however, even if it
+;; is in a struct field.
+(module
+  ;; CHECK:      (type $funcref_=>_i32 (func (param funcref) (result i32)))
+
+  ;; CHECK:      (type $none_=>_none (func))
+
+  ;; CHECK:      (type $A (struct (field i32)))
+  ;; OPEN_WORLD:      (type $funcref_=>_i32 (func (param funcref) (result i32)))
+
+  ;; OPEN_WORLD:      (type $none_=>_none (func))
+
+  ;; OPEN_WORLD:      (type $A (struct (field i32)))
+  (type $A (struct (field i32)))
+
+  ;; CHECK:      (type $none_=>_i32 (func (result i32)))
+
+  ;; CHECK:      (import "binaryen-intrinsics" "call.without.effects" (func $call.without.effects (param funcref) (result i32)))
+  ;; OPEN_WORLD:      (type $none_=>_i32 (func (result i32)))
+
+  ;; OPEN_WORLD:      (import "binaryen-intrinsics" "call.without.effects" (func $call.without.effects (param funcref) (result i32)))
+  (import "binaryen-intrinsics" "call.without.effects" (func $call.without.effects (param funcref) (result i32)))
+
+  ;; CHECK:      (elem declare func $getter)
+
+  ;; CHECK:      (export "main" (func $main))
+
+  ;; CHECK:      (func $main (type $none_=>_none)
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (struct.new $A
+  ;; CHECK-NEXT:    (call $call.without.effects
+  ;; CHECK-NEXT:     (ref.func $getter)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  ;; OPEN_WORLD:      (elem declare func $getter)
+
+  ;; OPEN_WORLD:      (export "main" (func $main))
+
+  ;; OPEN_WORLD:      (func $main (type $none_=>_none)
+  ;; OPEN_WORLD-NEXT:  (drop
+  ;; OPEN_WORLD-NEXT:   (struct.new $A
+  ;; OPEN_WORLD-NEXT:    (call $call.without.effects
+  ;; OPEN_WORLD-NEXT:     (ref.func $getter)
+  ;; OPEN_WORLD-NEXT:    )
+  ;; OPEN_WORLD-NEXT:   )
+  ;; OPEN_WORLD-NEXT:  )
+  ;; OPEN_WORLD-NEXT: )
+  (func $main (export "main")
+    (drop
+      (struct.new $A
+        (call $call.without.effects
+          (ref.func $getter)
+        )
+      )
+    )
+  )
+
+  ;; CHECK:      (func $getter (type $none_=>_i32) (result i32)
+  ;; CHECK-NEXT:  (i32.const 42)
+  ;; CHECK-NEXT: )
+  ;; OPEN_WORLD:      (func $getter (type $none_=>_i32) (result i32)
+  ;; OPEN_WORLD-NEXT:  (i32.const 42)
+  ;; OPEN_WORLD-NEXT: )
+  (func $getter (result i32)
+    ;; This function body should not be turned into an unreachable. It is
+    ;; reached from $main, even though the call is marked as not having effects.
+    (i32.const 42)
+  )
+)