[WebAssembly] Remove threwValue comparison after __wasm_setjmp_test

[aheejin]

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)

[llvmbot]

@llvm/pr-subscribers-backend-webassembly

Author: Heejin Ahn (aheejin)

Changes

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)

---

Full diff: https://github.com/llvm/llvm-project/pull/86633.diff

3 Files Affected:

• (modified) llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp (+3-5)
• (modified) llvm/test/CodeGen/WebAssembly/lower-em-ehsjlj.ll (+3-4)
• (modified) llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll (+1-3)

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:

[sbc100]

lgtm assuming all emscripten tests pass.

Nice simplification!

[dschuff]

This may be breaking the emscripten roller: https://ci.chromium.org/ui/p/emscripten-releases/builders/try/linux/b8752285953271496721/overview

[aheejin]

This may be breaking the emscripten roller: https://ci.chromium.org/ui/p/emscripten-releases/builders/try/linux/b8752285953271496721/overview

Thanks. Reverted. Didn't realize that condition was filtering out exceptions...