Skip to content

DeadArgumentElimination: Do not make private types public #7941

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

DeadArgumentElimination: Do not make private types public #7941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
36180d3
fix
kripken Oct 2, 2025
3ee8dbe
fix
kripken Oct 2, 2025
bc118c7
test
kripken Oct 2, 2025
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
22 changes: 22 additions & 0 deletions src/passes/DeadArgumentElimination.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,25 @@ struct DAE : public Pass {
}
}

// We also must not make private types public. That could be a problem
// without custom descriptors (private exact types are ok - they are lowered
// away - but not public ones). And also in general, making more things
// public has downsides to later opts, that this one doesn't justify.
if (isExported) {
if (!privateTypes) {
privateTypes.emplace();
for (auto type : ModuleUtils::getPrivateHeapTypes(*module)) {
Copy link
Member

Choose a reason for hiding this comment

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

It's actually much cheaper to just get the public types, since that only has to look at imports and exports.

privateTypes->insert(type);
}
}

for (auto t : newType) {
if (t.isRef() && privateTypes->count(t.getHeapType())) {
return false;
}
}
}

if (!isExported) {
func->setResults(newType);
} else {
Expand All @@ -632,6 +651,9 @@ struct DAE : public Pass {

return true;
}

// Created on first use.
std::optional<std::unordered_set<HeapType>> privateTypes;
};

Pass* createDAEPass() { return new DAE(); }
Expand Down
27 changes: 25 additions & 2 deletions test/lit/passes/dae-gc-no-cd.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@

(type $A (struct))

;; CHECK: (func $export (type $3) (result (ref $A))
;; CHECK: (func $export (type $func) (result anyref)
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this test needs updating (or at least the comment below does).

;; CHECK-NEXT: (struct.new_default $A)
;; CHECK-NEXT: )
(func $export (export "export") (type $func) (result anyref)
;; We can refine to (ref $A), but not an exact one.
(struct.new $A)
)

;; CHECK: (func $export-tuple (type $4) (result (ref $A) i32)
;; CHECK: (func $export-tuple (type $func2) (result anyref i32)
;; CHECK-NEXT: (tuple.make 2
;; CHECK-NEXT: (struct.new_default $A)
;; CHECK-NEXT: (i32.const 42)
Expand All @@ -36,3 +36,26 @@
)
)

;; We can refine the array result to $array. However, doing so brings in the
;; entire rec group of $array, which includes an exact type, which is not
;; allowed when custom descriptors are disabled.
(module
;; CHECK: (type $func (sub (func (result (ref array)))))
(type $func (sub (func (result (ref array)))))

(rec
;; CHECK: (rec
;; CHECK-NEXT: (type $array (array i8))
(type $array (array i8))
;; CHECK: (type $unused (func (result (ref (exact $array)))))
(type $unused (func (result (ref (exact $array)))))
)

;; CHECK: (func $test (type $func) (result (ref array))
;; CHECK-NEXT: (array.new_fixed $array 0)
;; CHECK-NEXT: )
(func $test (export "test") (type $func) (result (ref array))
(array.new_fixed $array 0)
)
)

46 changes: 46 additions & 0 deletions test/lit/passes/dae-gc.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,25 +312,70 @@

;; CHECK: (type $2 (sub $func (func (result (ref (exact $A))))))

;; CHECK: (type $3 (func (result (ref $A))))

;; CHECK: (export "export" (func $export))

;; CHECK: (export "export2" (func $export2))

;; CHECK: (func $export (type $2) (result (ref (exact $A)))
;; CHECK-NEXT: (struct.new_default $A)
;; CHECK-NEXT: )
(func $export (export "export") (type $func) (result anyref)
(struct.new $A)
)

;; CHECK: (func $export2 (type $3) (result (ref $A))
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
(func $export2 (export "export2") (result (ref $A))
Copy link
Member

Choose a reason for hiding this comment

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

Another option would be to just put $A in the same rec group as $func.

;; Make $A public, so we are ok to refine $export.
(unreachable)
)
)

;; We do not refine the result if the type is final/closed, as we can't
;; subtype it.
(module
;; CHECK: (type $A (struct))

;; CHECK: (type $func (func (result anyref)))
(type $func (func (result anyref)))

(type $A (struct))

;; CHECK: (type $2 (func (result (ref $A))))

;; CHECK: (export "export" (func $export))

;; CHECK: (export "export2" (func $export2))

;; CHECK: (func $export (type $func) (result anyref)
;; CHECK-NEXT: (struct.new_default $A)
;; CHECK-NEXT: )
(func $export (export "export") (type $func) (result anyref)
(struct.new $A)
)

;; CHECK: (func $export2 (type $2) (result (ref $A))
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
(func $export2 (export "export2") (result (ref $A))
;; Make $A public, so we are ok to refine $export.
(unreachable)
)
)

;; We do not refine the result if the type is private, as that would make it
;; public.
(module
;; CHECK: (type $func (sub (func (result anyref))))

;; CHECK: (type $A (struct))
(type $A (struct))

(type $func (sub (func (result anyref))))

;; CHECK: (export "export" (func $export))

;; CHECK: (func $export (type $func) (result anyref)
Expand All @@ -340,3 +385,4 @@
(struct.new $A)
)
)

Loading