[WinEH] Fix asm in catchpad being turned into unreachable

[Ralender]

No description provided.

[llvmbot]

@llvm/pr-subscribers-platform-windows

Author: None (Ralender)

Changes

---

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

2 Files Affected:

• (modified) llvm/lib/CodeGen/WinEHPrepare.cpp (+2-2)
• (added) llvm/test/CodeGen/WinEH/wineh-asm2.ll (+46)

diff --git a/llvm/lib/CodeGen/WinEHPrepare.cpp b/llvm/lib/CodeGen/WinEHPrepare.cpp
index 74cef6c134736..43dd651a686b9 100644
--- a/llvm/lib/CodeGen/WinEHPrepare.cpp
+++ b/llvm/lib/CodeGen/WinEHPrepare.cpp
@@ -1135,8 +1135,8 @@ void WinEHPrepareImpl::removeImplausibleInstructions(Function &F) {
         // Skip call sites which are nounwind intrinsics or inline asm.
         auto *CalledFn =
             dyn_cast<Function>(CB->getCalledOperand()->stripPointerCasts());
-        if (CalledFn && ((CalledFn->isIntrinsic() && CB->doesNotThrow()) ||
-                         CB->isInlineAsm()))
+        if (CB->isInlineAsm() ||
+            (CalledFn && CalledFn->isIntrinsic() && CB->doesNotThrow()))
           continue;
 
         // This call site was not part of this funclet, remove it.
diff --git a/llvm/test/CodeGen/WinEH/wineh-asm2.ll b/llvm/test/CodeGen/WinEH/wineh-asm2.ll
new file mode 100644
index 0000000000000..2c424536dca09
--- /dev/null
+++ b/llvm/test/CodeGen/WinEH/wineh-asm2.ll
@@ -0,0 +1,46 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt < %s -passes=win-eh-prepare -S | FileCheck %s
+
+target triple = "x86_64-unknown-windows-msvc"
+
+%rtti.TypeDescriptor2 = type { ptr, ptr, [3 x i8] }
+
+$"??_R0H@8" = comdat any
+
+@"??_7type_info@@6B@" = external constant ptr
+@"??_R0H@8" = linkonce_odr global %rtti.TypeDescriptor2 { ptr @"??_7type_info@@6B@", ptr null, [3 x i8] c".H\00" }, comdat
+
+declare dso_local i32 @test0(i32) local_unnamed_addr
+
+define dso_local i32 @test1(i32 %argc) local_unnamed_addr personality ptr @__CxxFrameHandler3 {
+; CHECK-LABEL: define dso_local i32 @test1(
+; CHECK-SAME: i32 [[ARGC:%.*]]) local_unnamed_addr personality ptr @__CxxFrameHandler3 {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[CALL:%.*]] = invoke i32 @test0(i32 [[ARGC]])
+; CHECK-NEXT:            to label %[[RETURN:.*]] unwind label %[[CATCH_DISPATCH:.*]]
+; CHECK:       [[CATCH_DISPATCH]]:
+; CHECK-NEXT:    [[TMP0:%.*]] = catchswitch within none [label %catch] unwind to caller
+; CHECK:       [[CATCH:.*:]]
+; CHECK-NEXT:    [[TMP1:%.*]] = catchpad within [[TMP0]] [ptr @"??_R0H@8", i32 0, ptr null]
+; CHECK-NEXT:    call void asm "", ""()
+; CHECK-NEXT:    catchret from [[TMP1]] to label %[[RETURN]]
+; CHECK:       [[RETURN]]:
+; CHECK-NEXT:    ret i32 0
+;
+entry:
+  %call = invoke i32 @test0(i32 %argc)
+  to label %return unwind label %catch.dispatch
+
+catch.dispatch:                                   ; preds = %entry
+  %0 = catchswitch within none [label %catch] unwind to caller
+
+catch:                                            ; preds = %catch.dispatch
+  %1 = catchpad within %0 [ptr @"??_R0H@8", i32 0, ptr null]
+  call void asm "", ""()
+  catchret from %1 to label %return
+
+return:                                           ; preds = %catch, %entry
+  ret i32 0
+}
+
+declare dso_local i32 @__CxxFrameHandler3(...)

[rnk]

Thanks for the fix! I think this bug has been present since the logic was added in 08dd52d , which has a test (WinEH/wineh-asm.ll). I realized that the original test doesn't fail because we forgot to invoke FileCheck on the output of opt! Can you please add that, and confirm that the original test now passes? I looked at the output , and it is clearly wrong:

$ opt -win-eh-prepare ../llvm/test/CodeGen/WinEH/wineh-asm.ll -o - -S
...
define void @test1() personality ptr @__CxxFrameHandler3 {
entry:
  invoke void @f(i32 1)
          to label %exit unwind label %cleanup

cleanup:                                          ; preds = %entry
  %cp = cleanuppad within none []
  unreachable
...

[rnk]

Thanks! Should I push the squash merge button?

[Ralender]

Thanks! Should I push the squash merge button?

Probably yes