[Wasm GC] Take advantage of immutable struct fields in effects.h (#4240)

This is the easy part of using immutability more: Just note immutable
fields as such when we read from them, and then a write to a struct
does not interfere with such reads. That is, only a read from a mutable
field can notice the effect of a write.

3 files changed, 89 insertions, 9 deletions

diff --git a/src/ir/effects.h b/src/ir/effects.h
index 3ccffb6f5..7074df755 100644
--- a/src/ir/effects.h
+++ b/src/ir/effects.h
@@ -79,7 +79,8 @@ public:
   bool writesTable = false;
   // TODO: More specific type-based alias analysis, and not just at the
   //       struct/array level.
-  bool readsStruct = false;
+  bool readsMutableStruct = false;
+  bool readsImmutableStruct = false;
   bool writesStruct = false;
   bool readsArray = false;
   bool writesArray = false;
@@ -132,7 +133,12 @@ public:
   }
   bool accessesMemory() const { return calls || readsMemory || writesMemory; }
   bool accessesTable() const { return calls || readsTable || writesTable; }
-  bool accessesStruct() const { return calls || readsStruct || writesStruct; }
+  bool accessesMutableStruct() const {
+    return calls || readsMutableStruct || writesStruct;
+  }
+  bool accessesStruct() const {
+    return accessesMutableStruct() || readsImmutableStruct;
+  }
   bool accessesArray() const { return calls || readsArray || writesArray; }
   // Check whether this may transfer control flow to somewhere outside of this
   // expression (aside from just flowing out normally). That includes a break
@@ -151,8 +157,9 @@ public:
            writesStruct || writesArray || isAtomic || calls;
   }
   bool readsGlobalState() const {
-    return globalsRead.size() || readsMemory || readsTable || readsStruct ||
-           readsArray || isAtomic || calls;
+    return globalsRead.size() || readsMemory || readsTable ||
+           readsMutableStruct || readsImmutableStruct || readsArray ||
+           isAtomic || calls;
   }
 
   bool hasNonTrapSideEffects() const {
@@ -203,8 +210,8 @@ public:
         ((other.writesMemory || other.calls) && accessesMemory()) ||
         ((writesTable || calls) && other.accessesTable()) ||
         ((other.writesTable || other.calls) && accessesTable()) ||
-        ((writesStruct || calls) && other.accessesStruct()) ||
-        ((other.writesStruct || other.calls) && accessesStruct()) ||
+        ((writesStruct || calls) && other.accessesMutableStruct()) ||
+        ((other.writesStruct || other.calls) && accessesMutableStruct()) ||
         ((writesArray || calls) && other.accessesArray()) ||
         ((other.writesArray || other.calls) && accessesArray()) ||
         (danglingPop || other.danglingPop)) {
@@ -268,7 +275,8 @@ public:
     writesMemory = writesMemory || other.writesMemory;
     readsTable = readsTable || other.readsTable;
     writesTable = writesTable || other.writesTable;
-    readsStruct = readsStruct || other.readsStruct;
+    readsMutableStruct = readsMutableStruct || other.readsMutableStruct;
+    readsImmutableStruct = readsImmutableStruct || other.readsImmutableStruct;
     writesStruct = writesStruct || other.writesStruct;
     readsArray = readsArray || other.readsArray;
     writesArray = writesArray || other.writesArray;
@@ -650,7 +658,17 @@ private:
     void visitRttSub(RttSub* curr) {}
     void visitStructNew(StructNew* curr) {}
     void visitStructGet(StructGet* curr) {
-      parent.readsStruct = true;
+      if (curr->ref->type == Type::unreachable) {
+        return;
+      }
+      if (curr->ref->type.getHeapType()
+            .getStruct()
+            .fields[curr->index]
+            .mutable_ == Mutable) {
+        parent.readsMutableStruct = true;
+      } else {
+        parent.readsImmutableStruct = true;
+      }
       // traps when the arg is null
       if (curr->ref->type.isNullable()) {
         parent.implicitTrap = true;
diff --git a/test/example/cpp-unit.cpp b/test/example/cpp-unit.cpp
index 961013bc3..7c32478c6 100644
--- a/test/example/cpp-unit.cpp
+++ b/test/example/cpp-unit.cpp
@@ -611,7 +611,8 @@ void test_effects() {
     assert_equal(effects.trap, true);
     assert_equal(effects.readsArray, true);
     assert_equal(effects.writesArray, true);
-    assert_equal(effects.readsStruct, false);
+    assert_equal(effects.readsMutableStruct, false);
+    assert_equal(effects.readsImmutableStruct, false);
     assert_equal(effects.writesStruct, false);
   }
 }
diff --git a/test/lit/passes/simplify-locals-gc.wast b/test/lit/passes/simplify-locals-gc.wast
index 912c3a688..97891aca4 100644
--- a/test/lit/passes/simplify-locals-gc.wast
+++ b/test/lit/passes/simplify-locals-gc.wast
@@ -8,6 +8,9 @@
   ;; CHECK:      (type $struct (struct (field (mut i32))))
   (type $struct (struct (field (mut i32))))
 
+  ;; CHECK:      (type $struct-immutable (struct (field i32)))
+  (type $struct-immutable (struct (field i32)))
+
   ;; Writes to heap objects cannot be reordered with reads.
   ;; CHECK:      (func $no-reorder-past-write (param $x (ref $struct)) (result i32)
   ;; CHECK-NEXT:  (local $temp i32)
@@ -36,6 +39,64 @@
     (local.get $temp)
   )
 
+  ;; CHECK:      (func $reorder-past-write-if-immutable (param $x (ref $struct)) (param $y (ref $struct-immutable)) (result i32)
+  ;; CHECK-NEXT:  (local $temp i32)
+  ;; CHECK-NEXT:  (nop)
+  ;; CHECK-NEXT:  (struct.set $struct 0
+  ;; CHECK-NEXT:   (local.get $x)
+  ;; CHECK-NEXT:   (i32.const 42)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (struct.get $struct-immutable 0
+  ;; CHECK-NEXT:   (local.get $y)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $reorder-past-write-if-immutable (param $x (ref $struct)) (param $y (ref $struct-immutable)) (result i32)
+    (local $temp i32)
+    (local.set $temp
+      (struct.get $struct-immutable 0
+        (local.get $y)
+      )
+    )
+    (struct.set $struct 0
+      (local.get $x)
+      (i32.const 42)
+    )
+    (local.get $temp)
+  )
+
+  ;; CHECK:      (func $unreachable-struct.get (param $x (ref $struct)) (param $y (ref $struct-immutable)) (result i32)
+  ;; CHECK-NEXT:  (local $temp i32)
+  ;; CHECK-NEXT:  (local.tee $temp
+  ;; CHECK-NEXT:   (block ;; (replaces something unreachable we can't emit)
+  ;; CHECK-NEXT:    (drop
+  ;; CHECK-NEXT:     (unreachable)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (struct.set $struct 0
+  ;; CHECK-NEXT:   (local.get $x)
+  ;; CHECK-NEXT:   (i32.const 42)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (local.get $temp)
+  ;; CHECK-NEXT: )
+  (func $unreachable-struct.get (param $x (ref $struct)) (param $y (ref $struct-immutable)) (result i32)
+    (local $temp i32)
+    ;; As above, but the get's ref is unreachable. This tests we do not hit an
+    ;; assertion on the get's type not having a heap type (as we depend on
+    ;; finding the heap type there in the reachable case).
+    ;; We simply do not handle this case, leaving it for DCE.
+    (local.set $temp
+      (struct.get $struct-immutable 0
+        (unreachable)
+      )
+    )
+    (struct.set $struct 0
+      (local.get $x)
+      (i32.const 42)
+    )
+    (local.get $temp)
+  )
+
   ;; CHECK:      (func $no-block-values-if-br_on
   ;; CHECK-NEXT:  (local $temp anyref)
   ;; CHECK-NEXT:  (block $block