Skip to content

Relax bodies of functions when their signature can't be changed #9380

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

Draft
stevenfontanella wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Relax bodies of functions when their signature can't be changed #9380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/pass.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ class Pass {
// Note: This is called on the registered pass instance concurrently across
// threads, so implementations must be thread-safe and cannot rely on
// getPassRunner() or getPassOptions().
virtual bool runsOnFunction(Function* func) { return true; }
virtual bool runsOnFunction(Function* func) const { return true; }

// Many passes can remove effects, for example, by finding some path is not
// reached and removing a throw or a call there. The few passes that *add*
Expand Down
36 changes: 24 additions & 12 deletions src/passes/RelaxNullability.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@
// preserve the API boundary. Exported functions are implicitly 'referenced'
// (i.e. they may be the target of a call_ref even if they never appear in a
// ref.func), so no need to check that a second time.
//
// TODO: Relax private referenced funcs + bodies of public referenced funcs.
return !func->imported() && !referencedFuncs.contains(func);
}

Expand Down Expand Up @@ -160,6 +158,11 @@
// false if only dead local definitions were changed.
bool changed = false;

// true iff the callee relaxed the params of the particular function that
// we're looking at. If not, then we know not to update the types of
// local.get or local.tee on those params.
bool signatureWasUpdated = false;

LocalsToNullable(const std::unordered_set<Function*>& referencedFuncs)
: referencedFuncs(referencedFuncs) {}

Expand All @@ -169,14 +172,12 @@

bool isFunctionParallel() override { return true; }

bool runsOnFunction(Function* func) override {
// If we didn't relax the params, don't update local.gets / local.tee which
// might touch unrelaxed params.
// TODO: Relax locals even if the params weren't relaxed.
return isRelaxable(func, referencedFuncs);
bool runsOnFunction(Function* func) const override {
return func->vars.size() > 0 || isRelaxable(func, referencedFuncs);

@github-actions github-actions Bot Nov 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[MEDIUM | Confidence: 5/5 | PERFORMANCE]

Checking func->vars.size() > 0 causes runsOnFunction to return true for every unrelaxable function (e.g. exported or referenced functions) that declares any local variables, even if all declared locals are primitive types (i32, i64, f32, f64) or already nullable.

For these functions, PassRunner instantiates the pass and performs a full AST traversal in Super::doWalkFunction(func). Because signatureWasUpdated is false and none of func->vars are non-nullable references, makeNullable does nothing and changed remains false, making the entire AST walk a redundant no-op.

To avoid traversing functions with only primitive or nullable locals, check whether any variable in func->vars is a non-nullable reference:

Suggested change
return func->vars.size() > 0 || isRelaxable(func, referencedFuncs);
bool runsOnFunction(Function* func) const override {
return isRelaxable(func, referencedFuncs) ||
std::any_of(func->vars.begin(), func->vars.end(), [](Type t) {
return t.isNonNullableRef();
});
}

}

void doWalkFunction(Function* func) {
signatureWasUpdated = isRelaxable(func, referencedFuncs);
// 1. Make all local definitions nullable and update gets and sets.
// 2. Refinalize to fixup block types
// 3. Insert ref.as_non_null where a parent demands it, e.g. return values,
Expand All @@ -185,13 +186,14 @@
// Refinalize is not needed after step 3 because a block never demands a
// non-null and thus step 3 makes no difference to block types.

bool changedLocals = false;
for (Index i = 0; i < func->vars.size(); ++i) {
std::ignore = makeNullable(func->vars[i]);
changedLocals |= makeNullable(func->vars[i]);
}
if (!changedLocals && !signatureWasUpdated) {

Check warning on line 193 in src/passes/RelaxNullability.cpp

View workflow job for this annotation

GitHub Actions / coverage

Uncovered MC/DC condition 

Decision `!changedLocals && !signatureWasUpdated` -> missing MC/DC independence pair for C1, C2

Check warning on line 193 in src/passes/RelaxNullability.cpp

View workflow job for this annotation

GitHub Actions / coverage

One-sided branch in diff 

Branch `!signatureWasUpdated` -> missing TRUE branch (True: 0x, False: 11x)
return;

Check warning on line 194 in src/passes/RelaxNullability.cpp

View workflow job for this annotation

GitHub Actions / coverage

Uncovered diff lines 

Line 194 not executed by tests (0x)
}

// TODO: could consider doing this only if locals changed
// but a function param could have changed and we can't tell that easily
// from here.
Super::doWalkFunction(func);

if (!changed) {
Expand All @@ -203,9 +205,19 @@
fixupReturn(&func->body, getModule(), func);
}

void visitLocalGet(LocalGet* curr) { changed |= makeNullable(curr->type); }
void visitLocalGet(LocalGet* curr) {
if (!signatureWasUpdated && getFunction()->isParam(curr->index)) {
return;
}

changed |= makeNullable(curr->type);
}

void visitLocalSet(LocalSet* curr) {
if (!signatureWasUpdated && getFunction()->isParam(curr->index)) {

Check warning on line 217 in src/passes/RelaxNullability.cpp

View workflow job for this annotation

GitHub Actions / coverage

Uncovered MC/DC condition 

Decision `!signatureWasUpdated && getFunction()->isParam(curr->index)` -> missing MC/DC independence pair for C1, C2

Check warning on line 217 in src/passes/RelaxNullability.cpp

View workflow job for this annotation

GitHub Actions / coverage

One-sided branch in diff 

Branch `getFunction()->isParam(curr->index)` -> missing TRUE branch (True: 0x, False: 4x)
return;
}

Check warning on line 219 in src/passes/RelaxNullability.cpp

View workflow job for this annotation

GitHub Actions / coverage

Uncovered diff lines 

Lines 218-219 not executed by tests (0x)

if (curr->isTee()) {
changed |= makeNullable(curr->type);
}
Expand Down
2 changes: 1 addition & 1 deletion src/passes/pass-utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ struct FilteredPass : public Pass {
return pass->requiresNonNullableLocalFixups();
}

bool runsOnFunction(Function* func) override {
bool runsOnFunction(Function* func) const override {
return relevantFuncs.contains(func) && pass->runsOnFunction(func);
}

Expand Down
55 changes: 47 additions & 8 deletions test/lit/passes/relax-nullability.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,26 +213,65 @@
;; CHECK: (export "exported-not-relaxed" (func $exported-not-relaxed))

;; CHECK: (func $exported-not-relaxed (type $shared-sig) (param $ref (ref any)) (result (ref any))
;; CHECK-NEXT: (local.get $ref)
;; CHECK-NEXT: (local $l anyref)
;; CHECK-NEXT: (ref.as_non_null
;; CHECK-NEXT: (block (result anyref)
;; CHECK-NEXT: (local.set $l
;; CHECK-NEXT: (local.get $ref)
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (local.tee $l
;; CHECK-NEXT: (local.get $ref)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $l)
;; CHECK-NEXT: )

@github-actions github-actions Bot Nov 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[HIGH | Confidence: 5/5 | BUG]

The lit test assertions for $exported-not-relaxed will fail under FileCheck. Line 231 adds (drop (local.tee $l (local.get $ref))) to the test input, but lines 222-223 in the CHECK directives expect CHECK-NEXT: (local.get $l) immediately after the local.set, omitting the dropped tee.

Because CHECK-NEXT enforces contiguous lines, FileCheck will fail when encountering (drop ...) in the output. Please update the test assertions using scripts/update_lit_checks.py -a (or include the expected (drop (local.tee ...)) checks before (local.get $l)).

;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $exported-not-relaxed (export "exported-not-relaxed") (param $ref (ref any)) (result (ref any))
(local.get $ref)
;; Function is exported, so we can't mutate its signature. Relax its locals
;; but not its params.
(local $l (ref any))
(local.set $l (local.get $ref))
(drop (local.tee $l (local.get $ref)))
(local.get $l)
)

;; CHECK: (func $referenced-not-relaxed (type $shared-sig) (param $ref (ref any)) (result (ref any))
;; CHECK-NEXT: (local.get $ref)
;; CHECK-NEXT: (local $l anyref)
;; CHECK-NEXT: (ref.as_non_null
;; CHECK-NEXT: (block (result anyref)
;; CHECK-NEXT: (local.set $l
;; CHECK-NEXT: (local.get $ref)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $l)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $referenced-not-relaxed (type $shared-sig) (param $ref (ref any)) (result (ref any))
;; Function referenced via ref.func: cannot be relaxed
(local.get $ref)
;; Function referenced via ref.func. Relax its locals but not its params.
(local $l (ref any))
(local.set $l (local.get $ref))
(local.get $l)
)

;; CHECK: (func $referenced-via-table-not-relaxed (type $shared-sig) (param $ref (ref any)) (result (ref any))
;; CHECK-NEXT: (local.get $ref)
;; CHECK-NEXT: (local $l anyref)
;; CHECK-NEXT: (ref.as_non_null
;; CHECK-NEXT: (block (result anyref)
;; CHECK-NEXT: (local.set $l
;; CHECK-NEXT: (local.get $ref)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $l)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $referenced-via-table-not-relaxed (type $shared-sig) (param $ref (ref any)) (result (ref any))
;; Function referenced via elem declare: cannot be relaxed
(local.get $ref)
;; Function referenced via elem declare. Relax its locals but not its
;; params.
(local $l (ref any))
(local.set $l (local.get $ref))
(local.get $l)
)

(elem (i32.const 0) $referenced-via-table-not-relaxed)
Expand Down
Loading