Skip to content

Do not rewrite supertypes in AbstractTypeRefining #7720

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: unsubtyping-rewrite
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 12 additions & 15 deletions src/passes/AbstractTypeRefining.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -275,29 +275,26 @@ struct AbstractTypeRefining : public Pass {
return;
}

// A TypeMapper that handles the patterns we have in our mapping, where we
// end up mapping a type to a *subtype*. We need to properly create
// supertypes while doing this rewriting. For example, say we have this:
// Rewriting types can usually rewrite subtype relationships. For example,
// if we have this:
//
// A :> B :> C
// C <: B <: A
//
// Say we see B is never created, so we want to map B to its subtype C. C's
// supertype must now be A.
// And we see that B is never created, we would naively map B to its subtype
// C. But if we rewrote C's supertype, C would declare itself to be its own
// supertype, which is not allowed. We could fix this by walking up the
// supertype chain to find a supertype that is not being rewritten, but
// changing subtype relationships and keeping descriptor chains valid is
// nontrivial. Instead, avoid changing subtype relationships entirely: leave
// that for Unsubtyping.
class AbstractTypeRefiningTypeMapper : public TypeMapper {
public:
AbstractTypeRefiningTypeMapper(Module& wasm, const TypeUpdates& mapping)
: TypeMapper(wasm, mapping) {}

std::optional<HeapType> getDeclaredSuperType(HeapType oldType) override {
auto super = oldType.getDeclaredSuperType();

// Go up the chain of supertypes, skipping things we are mapping away,
// as those things will not appear in the output. This skips B in the
// example above.
while (super && mapping.count(*super)) {
super = super->getDeclaredSuperType();
}
return super;
// We do not want to update subtype relationships.
return oldType.getDeclaredSuperType();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is identical to the function being overridden, so it can be removed. We do not need to declare AbstractTypeRefiningTypeMapper at all, I think.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the function being overridden will rewrite the supertype according to the type mapping.

}
};

Expand Down
1 change: 1 addition & 0 deletions src/passes/pass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,7 @@ void PassRunner::addDefaultGlobalOptimizationPrePasses() {
addIfNoDWARFIssues("cfp");
addIfNoDWARFIssues("gsi");
addIfNoDWARFIssues("abstract-type-refining");
addIfNoDWARFIssues("unsubtyping");
}
}
// TODO: generate-global-effects here, right before function passes, then
Expand Down
59 changes: 33 additions & 26 deletions test/lit/passes/abstract-type-refining.wast
Original file line number Diff line number Diff line change
Expand Up @@ -13,29 +13,31 @@
;; actually refer to a subtype of them (that has a struct.new). As a result, in
;; TNH mode $A and $D will also not be emitted in the output anymore.
(module
;; YESTNH: (rec
;; YESTNH-NEXT: (type $A (sub (struct)))
;; NO_TNH: (rec
;; NO_TNH-NEXT: (type $A (sub (struct)))
(type $A (sub (struct)))

;; YESTNH: (rec
;; YESTNH-NEXT: (type $B (sub (struct)))
;; YESTNH: (type $B (sub $A (struct)))
;; NO_TNH: (type $B (sub $A (struct)))
(type $B (sub $A (struct)))

;; YESTNH: (type $C (sub $B (struct)))
;; NO_TNH: (type $C (sub $B (struct)))
(type $C (sub $B (struct)))

;; YESTNH: (type $D (sub $C (struct)))
;; NO_TNH: (type $D (sub $C (struct)))
(type $D (sub $C (struct)))

;; YESTNH: (type $E (sub $C (struct)))
;; YESTNH: (type $E (sub $D (struct)))
;; NO_TNH: (type $E (sub $D (struct)))
(type $E (sub $D (struct)))

;; YESTNH: (type $3 (func (param anyref)))
;; YESTNH: (type $5 (func (param anyref)))

;; YESTNH: (type $4 (func))
;; YESTNH: (type $6 (func))

;; YESTNH: (global $global anyref (struct.new_default $B))
;; NO_TNH: (type $5 (func (param anyref)))
Expand All @@ -45,7 +47,7 @@
;; NO_TNH: (global $global anyref (struct.new_default $B))
(global $global anyref (struct.new $B))

;; YESTNH: (func $new (type $3) (param $x anyref)
;; YESTNH: (func $new (type $5) (param $x anyref)
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (struct.new_default $C)
;; YESTNH-NEXT: )
Expand All @@ -70,7 +72,7 @@
)
)

;; YESTNH: (func $ref.cast (type $3) (param $x anyref)
;; YESTNH: (func $ref.cast (type $5) (param $x anyref)
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (ref.cast (ref $B)
;; YESTNH-NEXT: (local.get $x)
Expand Down Expand Up @@ -154,7 +156,7 @@
)
)

;; YESTNH: (func $ref.test (type $3) (param $x anyref)
;; YESTNH: (func $ref.test (type $5) (param $x anyref)
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (ref.test (ref $B)
;; YESTNH-NEXT: (local.get $x)
Expand All @@ -176,7 +178,7 @@
)
)

;; YESTNH: (func $br_on (type $3) (param $x anyref)
;; YESTNH: (func $br_on (type $5) (param $x anyref)
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (block $block (result (ref $B))
;; YESTNH-NEXT: (drop
Expand Down Expand Up @@ -213,7 +215,7 @@
)
)

;; YESTNH: (func $basic (type $3) (param $x anyref)
;; YESTNH: (func $basic (type $5) (param $x anyref)
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (ref.cast (ref struct)
;; YESTNH-NEXT: (local.get $x)
Expand All @@ -236,7 +238,7 @@
)
)

;; YESTNH: (func $locals (type $4)
;; YESTNH: (func $locals (type $6)
;; YESTNH-NEXT: (local $A (ref $B))
;; YESTNH-NEXT: (local $B (ref $B))
;; YESTNH-NEXT: (local $C (ref $C))
Expand Down Expand Up @@ -359,21 +361,22 @@
;; $B1.
(module
(rec
;; YESTNH: (rec
;; YESTNH-NEXT: (type $A (sub (struct)))
;; NO_TNH: (rec
;; NO_TNH-NEXT: (type $A (sub (struct)))
(type $A (sub (struct)))

(type $B (sub $A (struct)))

;; YESTNH: (rec
;; YESTNH-NEXT: (type $B1 (sub (struct)))
;; YESTNH: (type $B1 (sub $A (struct)))
;; NO_TNH: (type $B1 (sub $A (struct)))
(type $B1 (sub $A (struct))) ;; this is a new type
)

;; YESTNH: (type $1 (func (param anyref)))
;; YESTNH: (type $2 (func (param anyref)))

;; YESTNH: (func $new (type $1) (param $x anyref)
;; YESTNH: (func $new (type $2) (param $x anyref)
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (struct.new_default $B1)
;; YESTNH-NEXT: )
Expand All @@ -391,7 +394,7 @@
)
)

;; YESTNH: (func $ref.cast (type $1) (param $x anyref)
;; YESTNH: (func $ref.cast (type $2) (param $x anyref)
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (ref.cast (ref $B1)
;; YESTNH-NEXT: (local.get $x)
Expand Down Expand Up @@ -446,21 +449,23 @@

;; A chain, $A :> $B :> $C, where we can optimize $A all the way to $C.
(module
;; YESTNH: (rec
;; YESTNH-NEXT: (type $A (sub (struct)))
;; NO_TNH: (rec
;; NO_TNH-NEXT: (type $A (sub (struct)))
(type $A (sub (struct)))

;; YESTNH: (type $B (sub $A (struct)))
;; NO_TNH: (type $B (sub $A (struct)))
(type $B (sub $A (struct)))

;; YESTNH: (rec
;; YESTNH-NEXT: (type $C (sub (struct)))
;; YESTNH: (type $C (sub $B (struct)))
;; NO_TNH: (type $C (sub $B (struct)))
(type $C (sub $B (struct)))

;; YESTNH: (type $1 (func (param anyref)))
;; YESTNH: (type $3 (func (param anyref)))

;; YESTNH: (func $new (type $1) (param $x anyref)
;; YESTNH: (func $new (type $3) (param $x anyref)
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (struct.new_default $C)
;; YESTNH-NEXT: )
Expand All @@ -478,7 +483,7 @@
)
)

;; YESTNH: (func $ref.cast (type $1) (param $x anyref)
;; YESTNH: (func $ref.cast (type $3) (param $x anyref)
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (ref.cast (ref $C)
;; YESTNH-NEXT: (local.get $x)
Expand Down Expand Up @@ -817,30 +822,32 @@
;; As above, but now $C1 is created.
(module
(rec
;; YESTNH: (rec
;; YESTNH-NEXT: (type $A (sub (struct)))
;; NO_TNH: (rec
;; NO_TNH-NEXT: (type $A (sub (struct)))
(type $A (sub (struct)))

;; YESTNH: (type $B (sub $A (struct)))
;; NO_TNH: (type $B (sub $A (struct)))
(type $B (sub $A (struct)))

;; YESTNH: (rec
;; YESTNH-NEXT: (type $C1 (sub (struct)))
;; YESTNH: (type $C1 (sub $B (struct)))
;; NO_TNH: (type $C1 (sub $B (struct)))
(type $C1 (sub $B (struct)))

(type $C2 (sub $B (struct)))
)

;; YESTNH: (type $1 (func (param anyref)))
;; YESTNH: (type $3 (func (param anyref)))

;; YESTNH: (global $global anyref (struct.new_default $C1))
;; NO_TNH: (type $3 (func (param anyref)))

;; NO_TNH: (global $global anyref (struct.new_default $C1))
(global $global anyref (struct.new $C1))

;; YESTNH: (func $ref.cast (type $1) (param $x anyref)
;; YESTNH: (func $ref.cast (type $3) (param $x anyref)
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (ref.cast (ref $C1)
;; YESTNH-NEXT: (local.get $x)
Expand Down Expand Up @@ -909,7 +916,7 @@
)
)

;; YESTNH: (func $ref.cast.null (type $1) (param $x anyref)
;; YESTNH: (func $ref.cast.null (type $3) (param $x anyref)
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (ref.cast (ref null $C1)
;; YESTNH-NEXT: (local.get $x)
Expand Down
Loading