Skip to content

Commit 7607db6

Browse files
kripkenkripken
authored
Fix IRBuilder location tracking of If starts (#7617)
The old code forgot the If scope when it started the Else, so any if-else would get a binary location span that started in the else (and ends in the right place). To fix this, note the if start location when we have it, and don't trample. Fixes branch hints on if-elses.
1 parent a19040d commit 7607db6

File tree

2 files changed

+138
-1
lines changed

2 files changed

+138
-1
lines changed

‎src/wasm/wasm-ir-builder.cpp

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,9 +151,18 @@ void IRBuilder::push(Expression* expr) {
151151

152152
applyDebugLoc(expr);
153153
if (binaryPos && func && lastBinaryPos != *binaryPos) {
154-
func->expressionLocations[expr] =
154+
auto span =
155155
BinaryLocations::Span{BinaryLocation(lastBinaryPos - codeSectionOffset),
156156
BinaryLocation(*binaryPos - codeSectionOffset)};
157+
// Some expressions already have their start noted, and we are just seeing
158+
// their last segment (like an Else).
159+
auto [iter, inserted] = func->expressionLocations.insert({expr, span});
160+
if (!inserted) {
161+
// Just update the end.
162+
iter->second.end = span.end;
163+
// The true start from before is before the start of the current segment.
164+
assert(iter->second.start < span.start);
165+
}
157166
lastBinaryPos = *binaryPos;
158167
}
159168

@@ -933,6 +942,10 @@ Result<> IRBuilder::visitElse() {
933942
if (binaryPos && func) {
934943
func->delimiterLocations[iff][BinaryLocations::Else] =
935944
lastBinaryPos - codeSectionOffset;
945+
946+
// Note the start of the if (which will be lost as the If is closed and the
947+
// Else begins, but the if spans them both).
948+
func->expressionLocations[iff].start = scope.startPos - codeSectionOffset;
936949
}
937950

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

9851000
CHECK_ERR(pushScope(ScopeCtx::makeCatch(std::move(scope), tryy)));

‎test/lit/branch-hinting.wast

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,128 @@
259259
)
260260
)
261261

262+
;; CHECK: (func $branch-hints-if-else (type $0) (param $x i32)
263+
;; CHECK-NEXT: (@metadata.code.branch_hint "\01")
264+
;; CHECK-NEXT: (if
265+
;; CHECK-NEXT: (local.get $x)
266+
;; CHECK-NEXT: (then
267+
;; CHECK-NEXT: (drop
268+
;; CHECK-NEXT: (i32.const 1)
269+
;; CHECK-NEXT: )
270+
;; CHECK-NEXT: )
271+
;; CHECK-NEXT: (else
272+
;; CHECK-NEXT: (drop
273+
;; CHECK-NEXT: (i32.const 0)
274+
;; CHECK-NEXT: )
275+
;; CHECK-NEXT: )
276+
;; CHECK-NEXT: )
277+
;; CHECK-NEXT: )
278+
;; RTRIP: (func $branch-hints-if-else (type $0) (param $x i32)
279+
;; RTRIP-NEXT: (@metadata.code.branch_hint "\01")
280+
;; RTRIP-NEXT: (if
281+
;; RTRIP-NEXT: (local.get $x)
282+
;; RTRIP-NEXT: (then
283+
;; RTRIP-NEXT: (drop
284+
;; RTRIP-NEXT: (i32.const 1)
285+
;; RTRIP-NEXT: )
286+
;; RTRIP-NEXT: )
287+
;; RTRIP-NEXT: (else
288+
;; RTRIP-NEXT: (drop
289+
;; RTRIP-NEXT: (i32.const 0)
290+
;; RTRIP-NEXT: )
291+
;; RTRIP-NEXT: )
292+
;; RTRIP-NEXT: )
293+
;; RTRIP-NEXT: )
294+
;; SRCMP: (func $branch-hints-if-else (type $0) (param $x i32)
295+
;; SRCMP-NEXT: (@metadata.code.branch_hint "\01")
296+
;; SRCMP-NEXT: (if
297+
;; SRCMP-NEXT: (local.get $x)
298+
;; SRCMP-NEXT: (then
299+
;; SRCMP-NEXT: (drop
300+
;; SRCMP-NEXT: (i32.const 1)
301+
;; SRCMP-NEXT: )
302+
;; SRCMP-NEXT: )
303+
;; SRCMP-NEXT: (else
304+
;; SRCMP-NEXT: (drop
305+
;; SRCMP-NEXT: (i32.const 0)
306+
;; SRCMP-NEXT: )
307+
;; SRCMP-NEXT: )
308+
;; SRCMP-NEXT: )
309+
;; SRCMP-NEXT: )
310+
(func $branch-hints-if-else (param $x i32)
311+
(@metadata.code.branch_hint "\01")
312+
(if
313+
(local.get $x)
314+
(then
315+
(drop
316+
(i32.const 1)
317+
)
318+
)
319+
(else
320+
(drop
321+
(i32.const 0)
322+
)
323+
)
324+
)
325+
)
326+
327+
;; CHECK: (func $branch-hints-if-else-result (type $0) (param $x i32)
328+
;; CHECK-NEXT: (drop
329+
;; CHECK-NEXT: (@metadata.code.branch_hint "\01")
330+
;; CHECK-NEXT: (if (result i32)
331+
;; CHECK-NEXT: (local.get $x)
332+
;; CHECK-NEXT: (then
333+
;; CHECK-NEXT: (i32.const 1)
334+
;; CHECK-NEXT: )
335+
;; CHECK-NEXT: (else
336+
;; CHECK-NEXT: (i32.const 0)
337+
;; CHECK-NEXT: )
338+
;; CHECK-NEXT: )
339+
;; CHECK-NEXT: )
340+
;; CHECK-NEXT: )
341+
;; RTRIP: (func $branch-hints-if-else-result (type $0) (param $x i32)
342+
;; RTRIP-NEXT: (drop
343+
;; RTRIP-NEXT: (@metadata.code.branch_hint "\01")
344+
;; RTRIP-NEXT: (if (result i32)
345+
;; RTRIP-NEXT: (local.get $x)
346+
;; RTRIP-NEXT: (then
347+
;; RTRIP-NEXT: (i32.const 1)
348+
;; RTRIP-NEXT: )
349+
;; RTRIP-NEXT: (else
350+
;; RTRIP-NEXT: (i32.const 0)
351+
;; RTRIP-NEXT: )
352+
;; RTRIP-NEXT: )
353+
;; RTRIP-NEXT: )
354+
;; RTRIP-NEXT: )
355+
;; SRCMP: (func $branch-hints-if-else-result (type $0) (param $x i32)
356+
;; SRCMP-NEXT: (drop
357+
;; SRCMP-NEXT: (@metadata.code.branch_hint "\01")
358+
;; SRCMP-NEXT: (if (result i32)
359+
;; SRCMP-NEXT: (local.get $x)
360+
;; SRCMP-NEXT: (then
361+
;; SRCMP-NEXT: (i32.const 1)
362+
;; SRCMP-NEXT: )
363+
;; SRCMP-NEXT: (else
364+
;; SRCMP-NEXT: (i32.const 0)
365+
;; SRCMP-NEXT: )
366+
;; SRCMP-NEXT: )
367+
;; SRCMP-NEXT: )
368+
;; SRCMP-NEXT: )
369+
(func $branch-hints-if-else-result (param $x i32)
370+
(drop
371+
(@metadata.code.branch_hint "\01")
372+
(if (result i32)
373+
(local.get $x)
374+
(then
375+
(i32.const 1)
376+
)
377+
(else
378+
(i32.const 0)
379+
)
380+
)
381+
)
382+
)
383+
262384
;; CHECK: (func $branch-hints-br_on (type $1) (param $x anyref)
263385
;; CHECK-NEXT: (block $out
264386
;; CHECK-NEXT: (drop

0 commit comments

Comments
 (0)