Skip to content

Commit 52431fd

Browse files
authored
[WebAssembly] Remove threwValue comparison after __wasm_setjmp_test (#86633)
Currently the code thinks a `longjmp` occurred if both `__THREW__` and `__threwValue` are nonzero. But `__threwValue` can be 0, and the `longjmp` library function should change it to 1 in case it is 0: https://en.cppreference.com/w/c/program/longjmp Emscripten libraries were not consistent about that, but after emscripten-core/emscripten#21493 and emscripten-core/emscripten#21502, we correctly pass 1 in case the input is 0. So there will be no case `__threwValue` is 0. And regardless of what `longjmp` library function does, treating `longjmp`'s 0 input to its second argument as "not longjmping" doesn't seem right. I'm not sure where that `__threwValue` checking came from, but probably I was porting then fastcomp's implementation and moved this part just verbatim: https://github.com/emscripten-core/emscripten-fastcomp/blob/9bdc7bb4fc595fe05a021b06fe350e8494a741a1/lib/Target/JSBackend/CallHandlers.h#L274-L278 Just for the context, how this was discovered: emscripten-core/emscripten#21502 (review)
1 parent 5a7341a commit 52431fd

File tree

3 files changed

+7
-12
lines changed

3 files changed

+7
-12
lines changed

llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@
153153
/// %__THREW__.val = __THREW__;
154154
/// __THREW__ = 0;
155155
/// %__threwValue.val = __threwValue;
156-
/// if (%__THREW__.val != 0 & %__threwValue.val != 0) {
156+
/// if (%__THREW__.val != 0) {
157157
/// %label = __wasm_setjmp_test(%__THREW__.val, functionInvocationId);
158158
/// if (%label == 0)
159159
/// emscripten_longjmp(%__THREW__.val, %__threwValue.val);
@@ -712,12 +712,10 @@ void WebAssemblyLowerEmscriptenEHSjLj::wrapTestSetjmp(
712712
BasicBlock *ThenBB1 = BasicBlock::Create(C, "if.then1", F);
713713
BasicBlock *ElseBB1 = BasicBlock::Create(C, "if.else1", F);
714714
BasicBlock *EndBB1 = BasicBlock::Create(C, "if.end", F);
715-
Value *ThrewCmp = IRB.CreateICmpNE(Threw, getAddrSizeInt(M, 0));
716715
Value *ThrewValue = IRB.CreateLoad(IRB.getInt32Ty(), ThrewValueGV,
717716
ThrewValueGV->getName() + ".val");
718-
Value *ThrewValueCmp = IRB.CreateICmpNE(ThrewValue, IRB.getInt32(0));
719-
Value *Cmp1 = IRB.CreateAnd(ThrewCmp, ThrewValueCmp, "cmp1");
720-
IRB.CreateCondBr(Cmp1, ThenBB1, ElseBB1);
717+
Value *ThrewCmp = IRB.CreateICmpNE(Threw, getAddrSizeInt(M, 0));
718+
IRB.CreateCondBr(ThrewCmp, ThenBB1, ElseBB1);
721719

722720
// Generate call.em.longjmp BB once and share it within the function
723721
if (!CallEmLongjmpBB) {

llvm/test/CodeGen/WebAssembly/lower-em-ehsjlj.ll

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,8 @@ entry:
2222
to label %try.cont unwind label %lpad
2323

2424
; CHECK: entry.split.split:
25-
; CHECK: %[[CMP0:.*]] = icmp ne i32 %__THREW__.val, 0
26-
; CHECK-NEXT: %__threwValue.val = load i32, ptr @__threwValue
27-
; CHECK-NEXT: %[[CMP1:.*]] = icmp ne i32 %__threwValue.val, 0
28-
; CHECK-NEXT: %[[CMP:.*]] = and i1 %[[CMP0]], %[[CMP1]]
25+
; CHECK: %__threwValue.val = load i32, ptr @__threwValue
26+
; CHECK-NEXT: %[[CMP:.*]] = icmp ne i32 %__THREW__.val, 0
2927
; CHECK-NEXT: br i1 %[[CMP]], label %if.then1, label %if.else1
3028

3129
; This is exception checking part. %if.else1 leads here
@@ -121,6 +119,7 @@ if.end: ; preds = %entry
121119
; CHECK-NEXT: unreachable
122120

123121
; CHECK: normal:
122+
; CHECK-NEXT: %__threwValue.val = load i32, ptr @__threwValue, align 4
124123
; CHECK-NEXT: icmp ne i32 %__THREW__.val, 0
125124

126125
return: ; preds = %if.end, %entry

llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,8 @@ entry:
3737
; CHECK-NEXT: call cc{{.*}} void @__invoke_void_[[PTR]]_i32(ptr @emscripten_longjmp, [[PTR]] %[[JMPBUF]], i32 1)
3838
; CHECK-NEXT: %[[__THREW__VAL:.*]] = load [[PTR]], ptr @__THREW__
3939
; CHECK-NEXT: store [[PTR]] 0, ptr @__THREW__
40-
; CHECK-NEXT: %[[CMP0:.*]] = icmp ne [[PTR]] %__THREW__.val, 0
4140
; CHECK-NEXT: %[[THREWVALUE_VAL:.*]] = load i32, ptr @__threwValue
42-
; CHECK-NEXT: %[[CMP1:.*]] = icmp ne i32 %[[THREWVALUE_VAL]], 0
43-
; CHECK-NEXT: %[[CMP:.*]] = and i1 %[[CMP0]], %[[CMP1]]
41+
; CHECK-NEXT: %[[CMP:.*]] = icmp ne [[PTR]] %__THREW__.val, 0
4442
; CHECK-NEXT: br i1 %[[CMP]], label %if.then1, label %if.else1
4543

4644
; CHECK: entry.split.split.split:

0 commit comments

Comments
 (0)