Skip to content

Fix IRBuilder location tracking of If starts #7617

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

Merged
merged 3 commits into from
May 22, 2025
Merged

Fix IRBuilder location tracking of If starts #7617

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
6c4d1e8
fix
kripken May 21, 2025
11f2c9a
finish
kripken May 21, 2025
3cbc86c
feedback
kripken May 21, 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
17 changes: 16 additions & 1 deletion src/wasm/wasm-ir-builder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,18 @@ void IRBuilder::push(Expression* expr) {

applyDebugLoc(expr);
if (binaryPos && func && lastBinaryPos != *binaryPos) {
func->expressionLocations[expr] =
auto span =
BinaryLocations::Span{BinaryLocation(lastBinaryPos - codeSectionOffset),
BinaryLocation(*binaryPos - codeSectionOffset)};
// Some expressions already have their start noted, and we are just seeing
// their last segment (like an Else).
auto [iter, inserted] = func->expressionLocations.insert({expr, span});
if (!inserted) {
// Just update the end.
iter->second.end = span.end;
// The true start from before is before the start of the current segment.
assert(iter->second.start < span.start);
}
lastBinaryPos = *binaryPos;
}

Expand Down Expand Up @@ -933,6 +942,10 @@ Result<> IRBuilder::visitElse() {
if (binaryPos && func) {
func->delimiterLocations[iff][BinaryLocations::Else] =
lastBinaryPos - codeSectionOffset;

// Note the start of the if (which will be lost as the If is closed and the
// Else begins, but the if spans them both).
func->expressionLocations[iff].start = scope.startPos - codeSectionOffset;
Copy link
Member

Choose a reason for hiding this comment

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

Can we either fix this for try-catch as well, or add a TODO to do so?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a TODO. I'm not sure offhand how to handle multiple catches. Anyhow, only If is urgent atm.

}

return pushScope(ScopeCtx::makeElse(std::move(scope)));
Expand Down Expand Up @@ -980,6 +993,8 @@ Result<> IRBuilder::visitCatch(Name tag) {
if (binaryPos && func) {
auto& delimiterLocs = func->delimiterLocations[tryy];
delimiterLocs[delimiterLocs.size()] = lastBinaryPos - codeSectionOffset;
// TODO: As in visitElse, we likely need to stash the Try start. Here we
// also need to account for multiple catches.
}

CHECK_ERR(pushScope(ScopeCtx::makeCatch(std::move(scope), tryy)));
Expand Down
122 changes: 122 additions & 0 deletions test/lit/branch-hinting.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,128 @@
)
)

;; CHECK: (func $branch-hints-if-else (type $0) (param $x i32)
Copy link
Member

Choose a reason for hiding this comment

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

In addition to this test, can we have another test that directly shows the correct binary offsets? Or do we not have a way to print IR annotated with binary offsets?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, we don't have an option to show binary offsets by a flag. We show them when DWARF sections exist, or code annotations, atm. Might be worth adding though.

;; CHECK-NEXT: (@metadata.code.branch_hint "\01")
;; CHECK-NEXT: (if
;; CHECK-NEXT: (local.get $x)
;; CHECK-NEXT: (then
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (i32.const 1)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (else
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; RTRIP: (func $branch-hints-if-else (type $0) (param $x i32)
;; RTRIP-NEXT: (@metadata.code.branch_hint "\01")
;; RTRIP-NEXT: (if
;; RTRIP-NEXT: (local.get $x)
;; RTRIP-NEXT: (then
;; RTRIP-NEXT: (drop
;; RTRIP-NEXT: (i32.const 1)
;; RTRIP-NEXT: )
;; RTRIP-NEXT: )
;; RTRIP-NEXT: (else
;; RTRIP-NEXT: (drop
;; RTRIP-NEXT: (i32.const 0)
;; RTRIP-NEXT: )
;; RTRIP-NEXT: )
;; RTRIP-NEXT: )
;; RTRIP-NEXT: )
;; SRCMP: (func $branch-hints-if-else (type $0) (param $x i32)
;; SRCMP-NEXT: (@metadata.code.branch_hint "\01")
;; SRCMP-NEXT: (if
;; SRCMP-NEXT: (local.get $x)
;; SRCMP-NEXT: (then
;; SRCMP-NEXT: (drop
;; SRCMP-NEXT: (i32.const 1)
;; SRCMP-NEXT: )
;; SRCMP-NEXT: )
;; SRCMP-NEXT: (else
;; SRCMP-NEXT: (drop
;; SRCMP-NEXT: (i32.const 0)
;; SRCMP-NEXT: )
;; SRCMP-NEXT: )
;; SRCMP-NEXT: )
;; SRCMP-NEXT: )
(func $branch-hints-if-else (param $x i32)
(@metadata.code.branch_hint "\01")
(if
(local.get $x)
(then
(drop
(i32.const 1)
)
)
(else
(drop
(i32.const 0)
)
)
)
)

;; CHECK: (func $branch-hints-if-else-result (type $0) (param $x i32)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (@metadata.code.branch_hint "\01")
;; CHECK-NEXT: (if (result i32)
;; CHECK-NEXT: (local.get $x)
;; CHECK-NEXT: (then
;; CHECK-NEXT: (i32.const 1)
;; CHECK-NEXT: )
;; CHECK-NEXT: (else
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; RTRIP: (func $branch-hints-if-else-result (type $0) (param $x i32)
;; RTRIP-NEXT: (drop
;; RTRIP-NEXT: (@metadata.code.branch_hint "\01")
;; RTRIP-NEXT: (if (result i32)
;; RTRIP-NEXT: (local.get $x)
;; RTRIP-NEXT: (then
;; RTRIP-NEXT: (i32.const 1)
;; RTRIP-NEXT: )
;; RTRIP-NEXT: (else
;; RTRIP-NEXT: (i32.const 0)
;; RTRIP-NEXT: )
;; RTRIP-NEXT: )
;; RTRIP-NEXT: )
;; RTRIP-NEXT: )
;; SRCMP: (func $branch-hints-if-else-result (type $0) (param $x i32)
;; SRCMP-NEXT: (drop
;; SRCMP-NEXT: (@metadata.code.branch_hint "\01")
;; SRCMP-NEXT: (if (result i32)
;; SRCMP-NEXT: (local.get $x)
;; SRCMP-NEXT: (then
;; SRCMP-NEXT: (i32.const 1)
;; SRCMP-NEXT: )
;; SRCMP-NEXT: (else
;; SRCMP-NEXT: (i32.const 0)
;; SRCMP-NEXT: )
;; SRCMP-NEXT: )
;; SRCMP-NEXT: )
;; SRCMP-NEXT: )
(func $branch-hints-if-else-result (param $x i32)
(drop
(@metadata.code.branch_hint "\01")
(if (result i32)
(local.get $x)
(then
(i32.const 1)
)
(else
(i32.const 0)
)
)
)
)

;; CHECK: (func $branch-hints-br_on (type $1) (param $x anyref)
;; CHECK-NEXT: (block $out
;; CHECK-NEXT: (drop
Expand Down
Loading