summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlon Zakai <azakai@google.com>2024-11-04 15:45:27 -0800
committerGitHub <noreply@github.com>2024-11-04 15:45:27 -0800
commitd71161aedb8ae1d6650111a0de0c2bdf53ec127b (patch)
treeb12120751327292afb198afa3b1ae50ebd22f591
parentc35e9871697b0b103228a96e9e79066e762d5e22 (diff)
downloadbinaryen-d71161aedb8ae1d6650111a0de0c2bdf53ec127b.tar.gz
binaryen-d71161aedb8ae1d6650111a0de0c2bdf53ec127b.tar.bz2
binaryen-d71161aedb8ae1d6650111a0de0c2bdf53ec127b.zip
[GC] Fix GlobalTypeOptimization logic for public types handling (#7051)
This fixes a regression from #7019. That PR fixed an error on situations with mixed public and private types, but it made us stop optimizing in valid cases, including cases with entirely private types. The specific regression was that we checked if we had an entry in the map of "can become immutable", and we thought that was enough. But we may have a private child type with a public parent, and still be able to optimize in the child if the field is not present in the parent. We also did not have exhaustive checking of all the states canBecomeImmutable can be, so add those + testing.
-rw-r--r--src/passes/GlobalTypeOptimization.cpp25
-rw-r--r--test/lit/passes/gto-mutability.wast324
2 files changed, 342 insertions, 7 deletions
diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp
index e35e30ac8..0baecdf43 100644
--- a/src/passes/GlobalTypeOptimization.cpp
+++ b/src/passes/GlobalTypeOptimization.cpp
@@ -203,13 +203,24 @@ struct GlobalTypeOptimization : public Pass {
// visibility, so do that here: we can only become immutable if the
// parent can as well.
auto super = type.getDeclaredSuperType();
- if (super && !canBecomeImmutable.count(*super)) {
- // No entry in canBecomeImmutable means nothing in the parent can
- // become immutable. We don't need to check the specific field index,
- // because visibility affects them all equally (i.e., if it is public
- // then no field can be changed, and if it is private then this field
- // can be changed, and perhaps more).
- continue;
+ if (super) {
+ // The super may not contain the field, which is fine, so only check
+ // here if the field does exist in both.
+ if (i < super->getStruct().fields.size()) {
+ // No entry in canBecomeImmutable means nothing in the parent can
+ // become immutable, so check for both that and for an entry with
+ // "false".
+ auto iter = canBecomeImmutable.find(*super);
+ if (iter == canBecomeImmutable.end()) {
+ continue;
+ }
+ // The vector is grown only when needed to contain a "true" value,
+ // so |i| being out of bounds indicates "false".
+ auto& superVec = iter->second;
+ if (i >= superVec.size() || !superVec[i]) {
+ continue;
+ }
+ }
}
// No set exists. Mark it as something we can make immutable.
diff --git a/test/lit/passes/gto-mutability.wast b/test/lit/passes/gto-mutability.wast
index 3b01572c8..3cf0e7701 100644
--- a/test/lit/passes/gto-mutability.wast
+++ b/test/lit/passes/gto-mutability.wast
@@ -688,3 +688,327 @@
)
)
+;; $sub has a field we can make immutable. That it does not exist in the super
+;; should not confuse us.
+(module
+ (rec
+ ;; CHECK: (rec
+ ;; CHECK-NEXT: (type $super (sub (struct)))
+ (type $super (sub (struct)))
+ ;; CHECK: (type $sub (sub $super (struct (field (ref string)))))
+ (type $sub (sub $super (struct (field (mut (ref string))))))
+ )
+
+ ;; CHECK: (type $2 (func))
+
+ ;; CHECK: (func $test (type $2)
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (struct.get $sub 0
+ ;; CHECK-NEXT: (struct.new $sub
+ ;; CHECK-NEXT: (string.const "foo")
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ (func $test
+ ;; Write and read the field.
+ (drop
+ (struct.get $sub 0
+ (struct.new $sub
+ (string.const "foo")
+ )
+ )
+ )
+ )
+)
+
+;; As above, but with another type in the middle, $mid, which also contains the
+;; field. We can optimize both $mid and $sub.
+(module
+ (rec
+ ;; CHECK: (rec
+ ;; CHECK-NEXT: (type $super (sub (struct)))
+ (type $super (sub (struct)))
+ ;; CHECK: (type $mid (sub $super (struct (field (ref string)))))
+ (type $mid (sub $super (struct (field (mut (ref string))))))
+ ;; CHECK: (type $sub (sub $mid (struct (field (ref string)))))
+ (type $sub (sub $mid (struct (field (mut (ref string))))))
+ )
+
+ ;; CHECK: (type $3 (func))
+
+ ;; CHECK: (func $test (type $3)
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (struct.get $sub 0
+ ;; CHECK-NEXT: (struct.new $sub
+ ;; CHECK-NEXT: (string.const "foo")
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (struct.get $mid 0
+ ;; CHECK-NEXT: (struct.new $mid
+ ;; CHECK-NEXT: (string.const "bar")
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ (func $test
+ (drop
+ (struct.get $sub 0
+ (struct.new $sub
+ (string.const "foo")
+ )
+ )
+ )
+ (drop
+ (struct.get $mid 0
+ (struct.new $mid
+ (string.const "bar")
+ )
+ )
+ )
+ )
+)
+
+;; As above, but add another irrelevant field first. We can still optimize the
+;; string, but the new mutable i32 must remain mutable, as it has a set.
+(module
+ (rec
+ ;; CHECK: (rec
+ ;; CHECK-NEXT: (type $super (sub (struct (field (mut i32)))))
+ (type $super (sub (struct (field (mut i32)))))
+ ;; CHECK: (type $mid (sub $super (struct (field (mut i32)) (field (ref string)))))
+ (type $mid (sub $super (struct (field (mut i32)) (field (mut (ref string))))))
+ ;; CHECK: (type $sub (sub $mid (struct (field (mut i32)) (field (ref string)))))
+ (type $sub (sub $mid (struct (field (mut i32)) (field (mut (ref string))))))
+ )
+
+ ;; CHECK: (type $3 (func))
+
+ ;; CHECK: (func $test (type $3)
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (struct.get $sub 1
+ ;; CHECK-NEXT: (struct.new $sub
+ ;; CHECK-NEXT: (i32.const 42)
+ ;; CHECK-NEXT: (string.const "foo")
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (struct.get $mid 1
+ ;; CHECK-NEXT: (struct.new $mid
+ ;; CHECK-NEXT: (i32.const 1337)
+ ;; CHECK-NEXT: (string.const "bar")
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (struct.set $super 0
+ ;; CHECK-NEXT: (struct.new $super
+ ;; CHECK-NEXT: (i32.const 98765)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (i32.const 42)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (struct.get $super 0
+ ;; CHECK-NEXT: (struct.new $super
+ ;; CHECK-NEXT: (i32.const 999999)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ (func $test
+ (drop
+ (struct.get $sub 1
+ (struct.new $sub
+ (i32.const 42)
+ (string.const "foo")
+ )
+ )
+ )
+ (drop
+ (struct.get $mid 1
+ (struct.new $mid
+ (i32.const 1337)
+ (string.const "bar")
+ )
+ )
+ )
+ ;; A set and get of the first field.
+ (struct.set $super 0
+ (struct.new $super
+ (i32.const 98765)
+ )
+ (i32.const 42)
+ )
+ (drop
+ (struct.get $super 0
+ (struct.new $super
+ (i32.const 999999)
+ )
+ )
+ )
+ )
+)
+
+;; As above, but without a set of the first field. Now we can optimize both
+;; fields.
+(module
+ (rec
+ ;; CHECK: (rec
+ ;; CHECK-NEXT: (type $super (sub (struct (field i32))))
+ (type $super (sub (struct (field (mut i32)))))
+ ;; CHECK: (type $mid (sub $super (struct (field i32) (field (ref string)))))
+ (type $mid (sub $super (struct (field (mut i32)) (field (mut (ref string))))))
+ ;; CHECK: (type $sub (sub $mid (struct (field i32) (field (ref string)))))
+ (type $sub (sub $mid (struct (field (mut i32)) (field (mut (ref string))))))
+ )
+
+ ;; CHECK: (type $3 (func))
+
+ ;; CHECK: (func $test (type $3)
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (struct.get $sub 1
+ ;; CHECK-NEXT: (struct.new $sub
+ ;; CHECK-NEXT: (i32.const 42)
+ ;; CHECK-NEXT: (string.const "foo")
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (struct.get $mid 1
+ ;; CHECK-NEXT: (struct.new $mid
+ ;; CHECK-NEXT: (i32.const 1337)
+ ;; CHECK-NEXT: (string.const "bar")
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (struct.get $super 0
+ ;; CHECK-NEXT: (struct.new $super
+ ;; CHECK-NEXT: (i32.const 999999)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ (func $test
+ (drop
+ (struct.get $sub 1
+ (struct.new $sub
+ (i32.const 42)
+ (string.const "foo")
+ )
+ )
+ )
+ (drop
+ (struct.get $mid 1
+ (struct.new $mid
+ (i32.const 1337)
+ (string.const "bar")
+ )
+ )
+ )
+ ;; Only a get of the first field.
+ (drop
+ (struct.get $super 0
+ (struct.new $super
+ (i32.const 999999)
+ )
+ )
+ )
+ )
+)
+
+;; The super is public, but we can still optimize the field in the sub.
+(module
+ ;; CHECK: (type $super (sub (struct)))
+ (type $super (sub (struct)))
+
+ ;; CHECK: (rec
+ ;; CHECK-NEXT: (type $sub (sub $super (struct (field stringref))))
+ (type $sub (sub $super (struct (field (mut stringref)))))
+
+ ;; CHECK: (type $2 (func))
+
+ ;; CHECK: (global $global (ref $super) (struct.new_default $super))
+ (global $global (ref $super) (struct.new_default $super))
+ ;; CHECK: (export "global" (global $global))
+ (export "global" (global $global))
+
+ ;; CHECK: (func $test (type $2)
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (struct.get $sub 0
+ ;; CHECK-NEXT: (struct.new $sub
+ ;; CHECK-NEXT: (string.const "foo")
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ (func $test
+ ;; Write and read the field.
+ (drop
+ (struct.get $sub 0
+ (struct.new $sub
+ (string.const "foo")
+ )
+ )
+ )
+ )
+)
+
+;; As above, and now the super has the field as well, preventing optimization.
+(module
+ ;; CHECK: (type $super (sub (struct (field (mut stringref)))))
+ (type $super (sub (struct (field (mut stringref)))))
+
+ ;; CHECK: (type $sub (sub $super (struct (field (mut stringref)))))
+ (type $sub (sub $super (struct (field (mut stringref)))))
+
+ ;; CHECK: (type $2 (func))
+
+ ;; CHECK: (global $global (ref $super) (struct.new_default $super))
+ (global $global (ref $super) (struct.new_default $super))
+ ;; CHECK: (export "global" (global $global))
+ (export "global" (global $global))
+
+ ;; CHECK: (func $test (type $2)
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (struct.get $sub 0
+ ;; CHECK-NEXT: (struct.new $sub
+ ;; CHECK-NEXT: (string.const "foo")
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ (func $test
+ ;; Write and read the field.
+ (drop
+ (struct.get $sub 0
+ (struct.new $sub
+ (string.const "foo")
+ )
+ )
+ )
+ )
+)
+
+;; Two mutable fields with a chain of three subtypes. The super is public,
+;; preventing optimization of the field it has (but not the other; the other
+;; is removable anyhow, though, so this just checks for the lack of an error
+;; when deciding not to make the fields immutable or not).
+(module
+ ;; CHECK: (type $super (sub (struct (field (mut i32)))))
+ (type $super (sub (struct (field (mut i32)))))
+ ;; CHECK: (rec
+ ;; CHECK-NEXT: (type $mid (sub $super (struct (field (mut i32)))))
+ (type $mid (sub $super (struct (field (mut i32)) (field (mut f64)))))
+ ;; CHECK: (type $sub (sub $mid (struct (field (mut i32)))))
+ (type $sub (sub $mid (struct (field (mut i32)) (field (mut f64)))))
+
+ ;; CHECK: (global $global (ref $super) (struct.new_default $sub))
+ (global $global (ref $super) (struct.new_default $sub))
+
+ ;; CHECK: (export "global" (global $global))
+ (export "global" (global $global))
+)
+