diff options
author | Alon Zakai <azakai@google.com> | 2024-11-04 15:45:27 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-11-04 15:45:27 -0800 |
commit | d71161aedb8ae1d6650111a0de0c2bdf53ec127b (patch) | |
tree | b12120751327292afb198afa3b1ae50ebd22f591 | |
parent | c35e9871697b0b103228a96e9e79066e762d5e22 (diff) | |
download | binaryen-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.cpp | 25 | ||||
-rw-r--r-- | test/lit/passes/gto-mutability.wast | 324 |
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)) +) + |