From a5201f175d8c9abbe90054f2075e6cc0169d4d92 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Mon, 18 Mar 2024 09:24:15 +0000 Subject: [PATCH] [WebAssembly] Remove threwValue comparison after __wasm_setjmp_test 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 https://github.com/emscripten-core/emscripten/pull/21493 and https://github.com/emscripten-core/emscripten/pull/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: https://github.com/emscripten-core/emscripten/pull/21502#pullrequestreview-1942160300 --- .../WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp | 8 +++----- llvm/test/CodeGen/WebAssembly/lower-em-ehsjlj.ll | 7 +++---- llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll | 4 +--- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp index 027ee1086bf4e0..0788d0c3a72136 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp @@ -153,7 +153,7 @@ /// %__THREW__.val = __THREW__; /// __THREW__ = 0; /// %__threwValue.val = __threwValue; -/// if (%__THREW__.val != 0 & %__threwValue.val != 0) { +/// if (%__THREW__.val != 0) { /// %label = __wasm_setjmp_test(%__THREW__.val, functionInvocationId); /// if (%label == 0) /// emscripten_longjmp(%__THREW__.val, %__threwValue.val); @@ -712,12 +712,10 @@ void WebAssemblyLowerEmscriptenEHSjLj::wrapTestSetjmp( BasicBlock *ThenBB1 = BasicBlock::Create(C, "if.then1", F); BasicBlock *ElseBB1 = BasicBlock::Create(C, "if.else1", F); BasicBlock *EndBB1 = BasicBlock::Create(C, "if.end", F); - Value *ThrewCmp = IRB.CreateICmpNE(Threw, getAddrSizeInt(M, 0)); Value *ThrewValue = IRB.CreateLoad(IRB.getInt32Ty(), ThrewValueGV, ThrewValueGV->getName() + ".val"); - Value *ThrewValueCmp = IRB.CreateICmpNE(ThrewValue, IRB.getInt32(0)); - Value *Cmp1 = IRB.CreateAnd(ThrewCmp, ThrewValueCmp, "cmp1"); - IRB.CreateCondBr(Cmp1, ThenBB1, ElseBB1); + Value *ThrewCmp = IRB.CreateICmpNE(Threw, getAddrSizeInt(M, 0)); + IRB.CreateCondBr(ThrewCmp, ThenBB1, ElseBB1); // Generate call.em.longjmp BB once and share it within the function if (!CallEmLongjmpBB) { diff --git a/llvm/test/CodeGen/WebAssembly/lower-em-ehsjlj.ll b/llvm/test/CodeGen/WebAssembly/lower-em-ehsjlj.ll index 32942cd92e684f..d88f42a4dc5847 100644 --- a/llvm/test/CodeGen/WebAssembly/lower-em-ehsjlj.ll +++ b/llvm/test/CodeGen/WebAssembly/lower-em-ehsjlj.ll @@ -22,10 +22,8 @@ entry: to label %try.cont unwind label %lpad ; CHECK: entry.split.split: -; CHECK: %[[CMP0:.*]] = icmp ne i32 %__THREW__.val, 0 -; CHECK-NEXT: %__threwValue.val = load i32, ptr @__threwValue -; CHECK-NEXT: %[[CMP1:.*]] = icmp ne i32 %__threwValue.val, 0 -; CHECK-NEXT: %[[CMP:.*]] = and i1 %[[CMP0]], %[[CMP1]] +; CHECK: %__threwValue.val = load i32, ptr @__threwValue +; CHECK-NEXT: %[[CMP:.*]] = icmp ne i32 %__THREW__.val, 0 ; CHECK-NEXT: br i1 %[[CMP]], label %if.then1, label %if.else1 ; This is exception checking part. %if.else1 leads here @@ -121,6 +119,7 @@ if.end: ; preds = %entry ; CHECK-NEXT: unreachable ; CHECK: normal: +; CHECK-NEXT: %__threwValue.val = load i32, ptr @__threwValue, align 4 ; CHECK-NEXT: icmp ne i32 %__THREW__.val, 0 return: ; preds = %if.end, %entry diff --git a/llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll b/llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll index 27ec95a2c462ab..dca4c59d7c8740 100644 --- a/llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll +++ b/llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll @@ -37,10 +37,8 @@ entry: ; CHECK-NEXT: call cc{{.*}} void @__invoke_void_[[PTR]]_i32(ptr @emscripten_longjmp, [[PTR]] %[[JMPBUF]], i32 1) ; CHECK-NEXT: %[[__THREW__VAL:.*]] = load [[PTR]], ptr @__THREW__ ; CHECK-NEXT: store [[PTR]] 0, ptr @__THREW__ -; CHECK-NEXT: %[[CMP0:.*]] = icmp ne [[PTR]] %__THREW__.val, 0 ; CHECK-NEXT: %[[THREWVALUE_VAL:.*]] = load i32, ptr @__threwValue -; CHECK-NEXT: %[[CMP1:.*]] = icmp ne i32 %[[THREWVALUE_VAL]], 0 -; CHECK-NEXT: %[[CMP:.*]] = and i1 %[[CMP0]], %[[CMP1]] +; CHECK-NEXT: %[[CMP:.*]] = icmp ne [[PTR]] %__THREW__.val, 0 ; CHECK-NEXT: br i1 %[[CMP]], label %if.then1, label %if.else1 ; CHECK: entry.split.split.split: