Skip to content

[clang][CodeGen] The eh_typeid_for intrinsic needs special care too #65699

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 6 commits into from
Sep 20, 2023

Conversation

AlexVlx
Copy link
Contributor

@AlexVlx AlexVlx commented Sep 7, 2023

This change is symmetric with the one reviewed in https://reviews.llvm.org/D157452 and handles the exception handling specific intrinsic, which slipped through the cracks, in the same way, by inserting an address-space cast iff RTTI is in a non-default AS.

@AlexVlx AlexVlx added the clang:codegen IR generation bugs: mangling, exceptions, etc. label Sep 7, 2023
@AlexVlx AlexVlx requested a review from a team as a code owner September 7, 2023 23:31
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Sep 10, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 10, 2023

@llvm/pr-subscribers-clang

Changes

This change is symmetric with the one reviewed in https://reviews.llvm.org/D157452 and handles the exception handling specific intrinsic, which slipped through the cracks, in the same way, by inserting an address-space cast iff RTTI is in a non-default AS.

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

2 Files Affected:

  • (modified) clang/lib/CodeGen/CGException.cpp (+4-1)
  • (added) clang/test/CodeGenCXX/try-catch-with-address-space.cpp (+25)
diff --git a/clang/lib/CodeGen/CGException.cpp b/clang/lib/CodeGen/CGException.cpp
index 3996f2948349cb5..49cf4ec4b84307b 100644
--- a/clang/lib/CodeGen/CGException.cpp
+++ b/clang/lib/CodeGen/CGException.cpp
@@ -1149,7 +1149,10 @@ static void emitCatchDispatchBlock(CodeGenFunction &CGF,
     assert(handler.Type.Flags == 0 &&
            "landingpads do not support catch handler flags");
     assert(typeValue && "fell into catch-all case!");
-    typeValue = CGF.Builder.CreateBitCast(typeValue, CGF.Int8PtrTy);
+    llvm::Type *argTy = llvm_eh_typeid_for->getArg(0)->getType();
+    // With opaque ptrs, only the address space can be a mismatch.
+    if (typeValue->getType() != argTy)
+      typeValue = CGF.Builder.CreateAddrSpaceCast(typeValue, argTy);
 
     // Figure out the next block.
     bool nextIsEnd;
diff --git a/clang/test/CodeGenCXX/try-catch-with-address-space.cpp b/clang/test/CodeGenCXX/try-catch-with-address-space.cpp
new file mode 100644
index 000000000000000..279d29f50fd4101
--- /dev/null
+++ b/clang/test/CodeGenCXX/try-catch-with-address-space.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 %s -triple=amdgcn-amd-amdhsa -emit-llvm -o - -fcxx-exceptions -fexceptions | FileCheck %s
+
+struct X { };
+
+const X g();
+
+void f() {
+  try {
+    throw g();
+    // CHECK: ptr addrspace(1) @_ZTI1X
+  } catch (const X x) {
+    // CHECK: catch ptr addrspace(1) @_ZTI1X
+    // CHECK: call i32 @llvm.eh.typeid.for(ptr addrspacecast (ptr addrspace(1) @_ZTI1X to ptr))
+  }
+}
+
+void h() {
+  try {
+    throw "ABC";
+    // CHECK: ptr addrspace(1) @_ZTIPKc
+  } catch (char const(&)[4]) {
+    // CHECK: catch ptr addrspace(1) @_ZTIA4_c
+    // CHECK: call i32 @llvm.eh.typeid.for(ptr addrspacecast (ptr addrspace(1) @_ZTIA4_c to ptr))
+  }
+}

@AlexVlx AlexVlx requested a review from yxsamliu September 19, 2023 13:11
Copy link
Collaborator

@yxsamliu yxsamliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks

@AlexVlx AlexVlx merged commit de018f5 into llvm:main Sep 20, 2023
@AlexVlx AlexVlx deleted the typeid_for_addrspace branch September 20, 2023 16:12
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants