[PATCH] D157452: [RFC][Clang][Codegen] `std::type_info` needs special care with explicit address spaces

2023-08-28 Thread Alex Voicu via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9c760ca8ecfd: [Clang][CodeGen] `typeid` needs special care 
when `type_info` is not in the… (authored by AlexVlx).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157452/new/

https://reviews.llvm.org/D157452

Files:
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/typeid-cxx11-with-address-space.cpp
  clang/test/CodeGenCXX/typeid-with-address-space.cpp
  clang/test/CodeGenCXX/typeinfo
  clang/test/CodeGenCXX/typeinfo-with-address-space.cpp

Index: clang/test/CodeGenCXX/typeinfo-with-address-space.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/typeinfo-with-address-space.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -I%S %s -triple amdgcn-amd-amdhsa -emit-llvm -o - | FileCheck %s -check-prefix=AS
+// RUN: %clang_cc1 -I%S %s -triple x86_64-linux-gnu -emit-llvm -o - | FileCheck %s -check-prefix=NO-AS
+#include 
+
+class A {
+virtual void f() = 0;
+};
+
+class B : A {
+void f() override;
+};
+
+// AS: @_ZTISt9type_info = external addrspace(1) constant ptr addrspace(1)
+// NO-AS: @_ZTISt9type_info = external constant ptr
+// AS: @_ZTIi = external addrspace(1) constant ptr addrspace(1)
+// NO-AS: @_ZTIi = external constant ptr
+// AS: @_ZTVN10__cxxabiv117__class_type_infoE = external addrspace(1) global ptr addrspace(1)
+// NO-AS: @_ZTVN10__cxxabiv117__class_type_infoE = external global ptr
+// AS: @_ZTS1A = linkonce_odr addrspace(1) constant [3 x i8] c"1A\00", comdat, align 1
+// NO-AS: @_ZTS1A = linkonce_odr constant [3 x i8] c"1A\00", comdat, align 1
+// AS: @_ZTI1A = linkonce_odr addrspace(1) constant { ptr addrspace(1), ptr addrspace(1) } { ptr addrspace(1) getelementptr inbounds (ptr addrspace(1), ptr addrspace(1) @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), ptr addrspace(1) @_ZTS1A }, comdat, align 8
+// NO-AS: @_ZTI1A = linkonce_odr constant { ptr, ptr } { ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), ptr @_ZTS1A }, comdat, align 8
+// AS: @_ZTIf = external addrspace(1) constant ptr addrspace(1)
+// NO-AS: @_ZTIf = external constant ptr
+
+unsigned long Fn(B& b) {
+// AS: %call = call noundef zeroext i1 @_ZNKSt9type_infoeqERKS_(ptr {{.*}} addrspacecast (ptr addrspace(1) @_ZTISt9type_info to ptr), ptr {{.*}} %2)
+// NO-AS: %call = call noundef zeroext i1 @_ZNKSt9type_infoeqERKS_(ptr {{.*}} @_ZTISt9type_info, ptr {{.*}} %2)
+if (typeid(std::type_info) == typeid(b))
+return 42;
+// AS: %call2 = call noundef zeroext i1 @_ZNKSt9type_infoneERKS_(ptr {{.*}} addrspacecast (ptr addrspace(1) @_ZTIi to ptr), ptr {{.*}} %5)
+// NO-AS: %call2 = call noundef zeroext i1 @_ZNKSt9type_infoneERKS_(ptr {{.*}} @_ZTIi, ptr {{.*}} %5)
+if (typeid(int) != typeid(b))
+return 1712;
+// AS: %call5 = call noundef ptr @_ZNKSt9type_info4nameEv(ptr {{.*}} addrspacecast (ptr addrspace(1) @_ZTI1A to ptr))
+// NO-AS: %call5 = call noundef ptr @_ZNKSt9type_info4nameEv(ptr {{.*}} @_ZTI1A)
+// AS: %call7 = call noundef ptr @_ZNKSt9type_info4nameEv(ptr {{.*}} %8)
+// NO-AS: %call7 = call noundef ptr @_ZNKSt9type_info4nameEv(ptr {{.*}} %8)
+if (typeid(A).name() == typeid(b).name())
+return 0;
+// AS: %call11 = call noundef zeroext i1 @_ZNKSt9type_info6beforeERKS_(ptr {{.*}} %11, ptr {{.*}} addrspacecast (ptr addrspace(1) @_ZTIf to ptr))
+// NO-AS:   %call11 = call noundef zeroext i1 @_ZNKSt9type_info6beforeERKS_(ptr {{.*}} %11, ptr {{.*}} @_ZTIf)
+if (typeid(b).before(typeid(float)))
+return 1;
+// AS: %call15 = call noundef i64 @_ZNKSt9type_info9hash_codeEv(ptr {{.*}} %14)
+// NO-AS: %call15 = call noundef i64 @_ZNKSt9type_info9hash_codeEv(ptr {{.*}} %14)
+return typeid(b).hash_code();
+}
Index: clang/test/CodeGenCXX/typeinfo
===
--- clang/test/CodeGenCXX/typeinfo
+++ clang/test/CodeGenCXX/typeinfo
@@ -10,6 +10,14 @@
 bool operator!=(const type_info& __arg) const {
   return !operator==(__arg);
 }
+
+bool before(const type_info& __arg) const {
+  return __name < __arg.__name;
+}
+
+unsigned long hash_code() const {
+  return reinterpret_cast(__name);
+}
   protected:
 const char *__name;
   };
Index: clang/test/CodeGenCXX/typeid-with-address-space.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/typeid-with-address-space.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -I%S %s -triple amdgcn-amd-amdhsa -emit-llvm -fcxx-exceptions -fexceptions -o - | FileCheck %s
+#include 
+
+namespace Test1 {
+
+// PR7400
+struct A { virtual void f(); };
+
+// CHECK: @_ZN5Test16int_tiE ={{.*}} constant ptr addrspacecast (ptr addrspace(1) @_ZTIi to ptr), align 8
+const std::type_info _ti = typeid(int);

[PATCH] D157452: [RFC][Clang][Codegen] `std::type_info` needs special care with explicit address spaces

2023-08-23 Thread Yaxun Liu via Phabricator via cfe-commits 
yaxunl accepted this revision.
yaxunl added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157452/new/

https://reviews.llvm.org/D157452

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157452: [RFC][Clang][Codegen] `std::type_info` needs special care with explicit address spaces

2023-08-23 Thread Alex Voicu via Phabricator via cfe-commits 
AlexVlx added a comment.

Gentle ping.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157452/new/

https://reviews.llvm.org/D157452

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157452: [RFC][Clang][Codegen] `std::type_info` needs special care with explicit address spaces

2023-08-15 Thread Alex Voicu via Phabricator via cfe-commits 
AlexVlx updated this revision to Diff 550516.
AlexVlx added a comment.

Remove unneeded cast, the dynamic case already emitted a generic pointer to 
`typeinfo`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157452/new/

https://reviews.llvm.org/D157452

Files:
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/typeid-cxx11-with-address-space.cpp
  clang/test/CodeGenCXX/typeid-with-address-space.cpp
  clang/test/CodeGenCXX/typeinfo
  clang/test/CodeGenCXX/typeinfo-with-address-space.cpp

Index: clang/test/CodeGenCXX/typeinfo-with-address-space.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/typeinfo-with-address-space.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -I%S %s -triple amdgcn-amd-amdhsa -emit-llvm -o - | FileCheck %s -check-prefix=AS
+// RUN: %clang_cc1 -I%S %s -triple x86_64-linux-gnu -emit-llvm -o - | FileCheck %s -check-prefix=NO-AS
+#include 
+
+class A {
+virtual void f() = 0;
+};
+
+class B : A {
+void f() override;
+};
+
+// AS: @_ZTISt9type_info = external addrspace(1) constant ptr addrspace(1)
+// NO-AS: @_ZTISt9type_info = external constant ptr
+// AS: @_ZTIi = external addrspace(1) constant ptr addrspace(1)
+// NO-AS: @_ZTIi = external constant ptr
+// AS: @_ZTVN10__cxxabiv117__class_type_infoE = external addrspace(1) global ptr addrspace(1)
+// NO-AS: @_ZTVN10__cxxabiv117__class_type_infoE = external global ptr
+// AS: @_ZTS1A = linkonce_odr addrspace(1) constant [3 x i8] c"1A\00", comdat, align 1
+// NO-AS: @_ZTS1A = linkonce_odr constant [3 x i8] c"1A\00", comdat, align 1
+// AS: @_ZTI1A = linkonce_odr addrspace(1) constant { ptr addrspace(1), ptr addrspace(1) } { ptr addrspace(1) getelementptr inbounds (ptr addrspace(1), ptr addrspace(1) @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), ptr addrspace(1) @_ZTS1A }, comdat, align 8
+// NO-AS: @_ZTI1A = linkonce_odr constant { ptr, ptr } { ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), ptr @_ZTS1A }, comdat, align 8
+// AS: @_ZTIf = external addrspace(1) constant ptr addrspace(1)
+// NO-AS: @_ZTIf = external constant ptr
+
+unsigned long Fn(B& b) {
+// AS: %call = call noundef zeroext i1 @_ZNKSt9type_infoeqERKS_(ptr {{.*}} addrspacecast (ptr addrspace(1) @_ZTISt9type_info to ptr), ptr {{.*}} %2)
+// NO-AS: %call = call noundef zeroext i1 @_ZNKSt9type_infoeqERKS_(ptr {{.*}} @_ZTISt9type_info, ptr {{.*}} %2)
+if (typeid(std::type_info) == typeid(b))
+return 42;
+// AS: %call2 = call noundef zeroext i1 @_ZNKSt9type_infoneERKS_(ptr {{.*}} addrspacecast (ptr addrspace(1) @_ZTIi to ptr), ptr {{.*}} %5)
+// NO-AS: %call2 = call noundef zeroext i1 @_ZNKSt9type_infoneERKS_(ptr {{.*}} @_ZTIi, ptr {{.*}} %5)
+if (typeid(int) != typeid(b))
+return 1712;
+// AS: %call5 = call noundef ptr @_ZNKSt9type_info4nameEv(ptr {{.*}} addrspacecast (ptr addrspace(1) @_ZTI1A to ptr))
+// NO-AS: %call5 = call noundef ptr @_ZNKSt9type_info4nameEv(ptr {{.*}} @_ZTI1A)
+// AS: %call7 = call noundef ptr @_ZNKSt9type_info4nameEv(ptr {{.*}} %8)
+// NO-AS: %call7 = call noundef ptr @_ZNKSt9type_info4nameEv(ptr {{.*}} %8)
+if (typeid(A).name() == typeid(b).name())
+return 0;
+// AS: %call11 = call noundef zeroext i1 @_ZNKSt9type_info6beforeERKS_(ptr {{.*}} %11, ptr {{.*}} addrspacecast (ptr addrspace(1) @_ZTIf to ptr))
+// NO-AS:   %call11 = call noundef zeroext i1 @_ZNKSt9type_info6beforeERKS_(ptr {{.*}} %11, ptr {{.*}} @_ZTIf)
+if (typeid(b).before(typeid(float)))
+return 1;
+// AS: %call15 = call noundef i64 @_ZNKSt9type_info9hash_codeEv(ptr {{.*}} %14)
+// NO-AS: %call15 = call noundef i64 @_ZNKSt9type_info9hash_codeEv(ptr {{.*}} %14)
+return typeid(b).hash_code();
+}
Index: clang/test/CodeGenCXX/typeinfo
===
--- clang/test/CodeGenCXX/typeinfo
+++ clang/test/CodeGenCXX/typeinfo
@@ -10,6 +10,14 @@
 bool operator!=(const type_info& __arg) const {
   return !operator==(__arg);
 }
+
+bool before(const type_info& __arg) const {
+  return __name < __arg.__name;
+}
+
+unsigned long hash_code() const {
+  return reinterpret_cast(__name);
+}
   protected:
 const char *__name;
   };
Index: clang/test/CodeGenCXX/typeid-with-address-space.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/typeid-with-address-space.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -I%S %s -triple amdgcn-amd-amdhsa -emit-llvm -fcxx-exceptions -fexceptions -o - | FileCheck %s
+#include 
+
+namespace Test1 {
+
+// PR7400
+struct A { virtual void f(); };
+
+// CHECK: @_ZN5Test16int_tiE ={{.*}} constant ptr addrspacecast (ptr addrspace(1) @_ZTIi to ptr), align 8
+const std::type_info _ti = typeid(int);
+
+// CHECK: @_ZN5Test14A_tiE ={{.*}} constant ptr addrspacecast (ptr addrspace(1) @_ZTIN5Test11AE to ptr), align 8
+const std::type_info _ti

[PATCH] D157452: [RFC][Clang][Codegen] `std::type_info` needs special care with explicit address spaces

2023-08-15 Thread Alex Voicu via Phabricator via cfe-commits 
AlexVlx added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:5237-5238
+
+  if (VTy->isPointerTy() &&
+  VTy->getPointerAddressSpace() != IRTy->getPointerAddressSpace()) 
{
+// In the case of targets that use a non-default address space for

arsenm wrote:
> you can also just unconditionally call CreateAddrSpaceCast and let it no-op 
> if the types match
I would've if I could've:) Sadly, CastIsValid returns false for address space 
casts between the same AS: 
https://github.com/llvm/llvm-project/blob/ca68a7f956f24aa3882134c5d8e72659355292dc/llvm/lib/IR/Instructions.cpp#L3895,
 and this makes `assert`s flare. I'm not sure if that is vestigial or just 
overly cautious behaviour.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157452/new/

https://reviews.llvm.org/D157452

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157452: [RFC][Clang][Codegen] `std::type_info` needs special care with explicit address spaces

2023-08-15 Thread Alex Voicu via Phabricator via cfe-commits 
AlexVlx added a comment.

In D157452#4586554 , @rjmccall wrote:

> The path of least resistance here is that IRGen should just insert 
> conversions from the global AS to the default AS as part of evaluating 
> `typeid`.  I haven't looked at it closely, but that seems to be what this 
> patch is doing.
>
> However, `std::type_info` is an interesting special case in that we actually 
> know statically that all objects of that type will be allocated in the global 
> AS, so there's really no reason to pass around pointers in the default AS; 
> `std::type_info *` should just default to being in the global AS.  It'd be a 
> non-trivial feature in support of a somewhat uncommonly-used C++ feature, and 
> I can't tell how best to spend your time, *but*... if you're so inclined, I 
> think it would make a somewhat compelling feature to be able to declare a 
> default AS for a class type, which your target could then adopt in the 
> headers for `std::type_info`.

I've reworked things along these lines, as both you and @yaxunl, thank you; 
turns out that what I was doing was unsound / would not catch all cases, 
whereas this does. As for the feature suggestion, that actually seems as if it 
would be very useful, beyond this application; I will add it to my TODO list, 
although I cannot promise to get to investigating it right away.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157452/new/

https://reviews.llvm.org/D157452

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157452: [RFC][Clang][Codegen] `std::type_info` needs special care with explicit address spaces

2023-08-15 Thread Alex Voicu via Phabricator via cfe-commits 
AlexVlx updated this revision to Diff 550311.
AlexVlx added a comment.

Rework the patch as the proposed approach was unsound; keep `typeid` generic.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157452/new/

https://reviews.llvm.org/D157452

Files:
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/typeid-cxx11-with-address-space.cpp
  clang/test/CodeGenCXX/typeid-with-address-space.cpp
  clang/test/CodeGenCXX/typeinfo
  clang/test/CodeGenCXX/typeinfo-with-address-space.cpp

Index: clang/test/CodeGenCXX/typeinfo-with-address-space.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/typeinfo-with-address-space.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -I%S %s -triple amdgcn-amd-amdhsa -emit-llvm -o - | FileCheck %s -check-prefix=AS
+// RUN: %clang_cc1 -I%S %s -triple x86_64-linux-gnu -emit-llvm -o - | FileCheck %s -check-prefix=NO-AS
+#include 
+
+class A {
+virtual void f() = 0;
+};
+
+class B : A {
+void f() override;
+};
+
+// AS: @_ZTISt9type_info = external addrspace(1) constant ptr addrspace(1)
+// NO-AS: @_ZTISt9type_info = external constant ptr
+// AS: @_ZTIi = external addrspace(1) constant ptr addrspace(1)
+// NO-AS: @_ZTIi = external constant ptr
+// AS: @_ZTVN10__cxxabiv117__class_type_infoE = external addrspace(1) global ptr addrspace(1)
+// NO-AS: @_ZTVN10__cxxabiv117__class_type_infoE = external global ptr
+// AS: @_ZTS1A = linkonce_odr addrspace(1) constant [3 x i8] c"1A\00", comdat, align 1
+// NO-AS: @_ZTS1A = linkonce_odr constant [3 x i8] c"1A\00", comdat, align 1
+// AS: @_ZTI1A = linkonce_odr addrspace(1) constant { ptr addrspace(1), ptr addrspace(1) } { ptr addrspace(1) getelementptr inbounds (ptr addrspace(1), ptr addrspace(1) @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), ptr addrspace(1) @_ZTS1A }, comdat, align 8
+// NO-AS: @_ZTI1A = linkonce_odr constant { ptr, ptr } { ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), ptr @_ZTS1A }, comdat, align 8
+// AS: @_ZTIf = external addrspace(1) constant ptr addrspace(1)
+// NO-AS: @_ZTIf = external constant ptr
+
+unsigned long Fn(B& b) {
+// AS: %call = call noundef zeroext i1 @_ZNKSt9type_infoeqERKS_(ptr {{.*}} addrspacecast (ptr addrspace(1) @_ZTISt9type_info to ptr), ptr {{.*}} %2)
+// NO-AS: %call = call noundef zeroext i1 @_ZNKSt9type_infoeqERKS_(ptr {{.*}} @_ZTISt9type_info, ptr {{.*}} %2)
+if (typeid(std::type_info) == typeid(b))
+return 42;
+// AS: %call2 = call noundef zeroext i1 @_ZNKSt9type_infoneERKS_(ptr {{.*}} addrspacecast (ptr addrspace(1) @_ZTIi to ptr), ptr {{.*}} %5)
+// NO-AS: %call2 = call noundef zeroext i1 @_ZNKSt9type_infoneERKS_(ptr {{.*}} @_ZTIi, ptr {{.*}} %5)
+if (typeid(int) != typeid(b))
+return 1712;
+// AS: %call5 = call noundef ptr @_ZNKSt9type_info4nameEv(ptr {{.*}} addrspacecast (ptr addrspace(1) @_ZTI1A to ptr))
+// NO-AS: %call5 = call noundef ptr @_ZNKSt9type_info4nameEv(ptr {{.*}} @_ZTI1A)
+// AS: %call7 = call noundef ptr @_ZNKSt9type_info4nameEv(ptr {{.*}} %8)
+// NO-AS: %call7 = call noundef ptr @_ZNKSt9type_info4nameEv(ptr {{.*}} %8)
+if (typeid(A).name() == typeid(b).name())
+return 0;
+// AS: %call11 = call noundef zeroext i1 @_ZNKSt9type_info6beforeERKS_(ptr {{.*}} %11, ptr {{.*}} addrspacecast (ptr addrspace(1) @_ZTIf to ptr))
+// NO-AS:   %call11 = call noundef zeroext i1 @_ZNKSt9type_info6beforeERKS_(ptr {{.*}} %11, ptr {{.*}} @_ZTIf)
+if (typeid(b).before(typeid(float)))
+return 1;
+// AS: %call15 = call noundef i64 @_ZNKSt9type_info9hash_codeEv(ptr {{.*}} %14)
+// NO-AS: %call15 = call noundef i64 @_ZNKSt9type_info9hash_codeEv(ptr {{.*}} %14)
+return typeid(b).hash_code();
+}
Index: clang/test/CodeGenCXX/typeinfo
===
--- clang/test/CodeGenCXX/typeinfo
+++ clang/test/CodeGenCXX/typeinfo
@@ -10,6 +10,14 @@
 bool operator!=(const type_info& __arg) const {
   return !operator==(__arg);
 }
+
+bool before(const type_info& __arg) const {
+  return __name < __arg.__name;
+}
+
+unsigned long hash_code() const {
+  return reinterpret_cast(__name);
+}
   protected:
 const char *__name;
   };
Index: clang/test/CodeGenCXX/typeid-with-address-space.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/typeid-with-address-space.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -I%S %s -triple amdgcn-amd-amdhsa -emit-llvm -fcxx-exceptions -fexceptions -o - | FileCheck %s
+#include 
+
+namespace Test1 {
+
+// PR7400
+struct A { virtual void f(); };
+
+// CHECK: @_ZN5Test16int_tiE ={{.*}} constant ptr addrspacecast (ptr addrspace(1) @_ZTIi to ptr), align 8
+const std::type_info _ti = typeid(int);
+
+// CHECK: @_ZN5Test14A_tiE ={{.*}} constant ptr addrspacecast (ptr addrspace(1) @_ZTIN5Test11AE to ptr), align 8
+const std::type_info _ti =

[PATCH] D157452: [RFC][Clang][Codegen] `std::type_info` needs special care with explicit address spaces

2023-08-14 Thread John McCall via Phabricator via cfe-commits 
rjmccall added a comment.

The path of least resistance here is that IRGen should just insert conversions 
from the global AS to the default AS as part of evaluating `typeid`.  I haven't 
looked at it closely, but that seems to be what this patch is doing.

However, `std::type_info` is an interesting special case in that we actually 
know statically that all objects of that type will be allocated in the global 
AS, so there's really no reason to pass around pointers in the default AS; 
`std::type_info *` should just default to being in the global AS.  It'd be a 
non-trivial feature in supported of a somewhat uncommonly-used C++ feature, and 
I can't tell how best to spend your time, *but*... if you're so inclined, I 
think it would make a somewhat compelling feature to be able to declare a 
default AS for a class type, which your target could then adopt in the C++ std 
headers.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157452/new/

https://reviews.llvm.org/D157452

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157452: [RFC][Clang][Codegen] `std::type_info` needs special care with explicit address spaces

2023-08-10 Thread Matt Arsenault via Phabricator via cfe-commits 
arsenm added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:5237-5238
+
+  if (VTy->isPointerTy() &&
+  VTy->getPointerAddressSpace() != IRTy->getPointerAddressSpace()) 
{
+// In the case of targets that use a non-default address space for

you can also just unconditionally call CreateAddrSpaceCast and let it no-op if 
the types match


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157452/new/

https://reviews.llvm.org/D157452

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157452: [RFC][Clang][Codegen] `std::type_info` needs special care with explicit address spaces

2023-08-09 Thread Alex Voicu via Phabricator via cfe-commits 
AlexVlx added a comment.

In D157452#4573076 , @yaxunl wrote:

> It is a little concerning how far the global address will spread further. 
> Compared to handling user-defined global variables, we keep the global 
> address to its definition in the IR and any use of it will use the generic 
> pointer addrcasted from its definition. This simplifies things a lot since 
> the AST is not aware of the global address. Should we reconsider the handling 
> of type id and type info here with a similar approach to how the user-defined 
> global variables are handled? Or we are confident that the effect of global 
> address can be confined.

This is a really good observation / question. I'm fairly confident this should 
be the last piece of noise, until (if) we decide to do something about 
functions. My reservation about address casting uses is that it might lead to a 
proliferation of casts, and it also appeared (when I tried) that it would be 
fairly spread out. `type_info` is special and a bit obnoxious because it is 
actually defined in the standard and has a mangled interface, so "lying" about 
the signature of its member functions in IR seems risky too. FWIW, this was 
silently incorrect in some cases today as well (without `assert`s we just do 
the bitcast, which happens to work on our target).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157452/new/

https://reviews.llvm.org/D157452

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157452: [RFC][Clang][Codegen] `std::type_info` needs special care with explicit address spaces

2023-08-09 Thread Yaxun Liu via Phabricator via cfe-commits 
yaxunl added a comment.

It is a little concerning how far the global address will spread further. 
Compared to handling user-defined global variables, we keep the global address 
to its definition in the IR and any use of it will use the generic pointer 
addrcasted from its definition. This simplifies things a lot since the AST is 
not aware of the global address. Should we reconsider the handling of type id 
and type info here with a similar approach to how the user-defined global 
variables are handled? Or we are confident that the effect of global address 
can be confined.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157452/new/

https://reviews.llvm.org/D157452

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157452: [RFC][Clang][Codegen] `std::type_info` needs special care with explicit address spaces

2023-08-08 Thread Alex Voicu via Phabricator via cfe-commits 
AlexVlx created this revision.
AlexVlx added reviewers: rjmccall, efriedma, yaxunl, arsenm.
AlexVlx added a project: clang.
Herald added a project: All.
AlexVlx requested review of this revision.
Herald added subscribers: cfe-commits, wdng.

After https://reviews.llvm.org/D153092, for targets that use a non-default AS 
for globals, an "interesting" situation arises around `typeid` and its paired 
type, `type_info`:

- on the AST level, the `type_info` interface is defined with default / generic 
addresses, be it for function arguments, or for `this`;
- in IR, `type_info` values are globals, and thus pointers to `type_info` 
values are pointers to global

This leads to a mismatch between the function signature / formal type of the 
argument, and its actual type. Currently we try to handle such mismatches via 
bitcast, but that is wrong in this case, since an ascast is required. 
**However**, I'm not convinced this is the right way to address it, I've just 
not found a better / less noisy one (yet?); the other alternative would be to 
special case codegen for typeinfo itself, and adjust the IR function signatures 
there. That's less centralised and doesn't actually seem correct, since 
`type_info` has a C++(mangled) interface, and we'd be basically lying about the 
type. Hopefully I'm missing some obvious and significantly more tidy solution - 
hence the RFC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157452

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/typeid-cxx11-with-address-space.cpp
  clang/test/CodeGenCXX/typeid-with-address-space.cpp
  clang/test/CodeGenCXX/typeinfo
  clang/test/CodeGenCXX/typeinfo-with-address-space.cpp

Index: clang/test/CodeGenCXX/typeinfo-with-address-space.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/typeinfo-with-address-space.cpp
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -I%S %s -triple amdgcn-amd-amdhsa -emit-llvm -o - | FileCheck %s -check-prefix=AS
+// RUN: %clang_cc1 -I%S %s -triple x86_64-linux-gnu -emit-llvm -o - | FileCheck %s -check-prefix=NO-AS
+#include 
+
+class A {
+virtual void f() = 0;
+};
+
+class B : A {
+void f() override;
+};
+
+// AS: @_ZTISt9type_info = external addrspace(1) constant ptr addrspace(1)
+// NO-AS: @_ZTISt9type_info = external constant ptr
+// AS: @_ZTIi = external addrspace(1) constant ptr addrspace(1)
+// NO-AS: @_ZTIi = external constant ptr
+// AS: @_ZTVN10__cxxabiv117__class_type_infoE = external addrspace(1) global ptr addrspace(1)
+// NO-AS: @_ZTVN10__cxxabiv117__class_type_infoE = external global ptr
+// AS: @_ZTS1A = linkonce_odr addrspace(1) constant [3 x i8] c"1A\00", comdat, align 1
+// NO-AS: @_ZTS1A = linkonce_odr constant [3 x i8] c"1A\00", comdat, align 1
+// AS: @_ZTI1A = linkonce_odr addrspace(1) constant { ptr addrspace(1), ptr addrspace(1) } { ptr addrspace(1) getelementptr inbounds (ptr addrspace(1), ptr addrspace(1) @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), ptr addrspace(1) @_ZTS1A }, comdat, align 8
+// NO-AS: @_ZTI1A = linkonce_odr constant { ptr, ptr } { ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), ptr @_ZTS1A }, comdat, align 8
+// AS: @_ZTIf = external addrspace(1) constant ptr addrspace(1)
+// NO-AS: @_ZTIf = external constant ptr
+
+unsigned long Fn(B& b) {
+// AS: %3 = addrspacecast ptr addrspace(1) %2 to ptr
+// AS-NEXT: %call = call noundef zeroext i1 @_ZNKSt9type_infoeqERKS_(ptr {{.*}} addrspacecast (ptr addrspace(1) @_ZTISt9type_info to ptr), ptr {{.*}} %3)
+// NO-AS: %call = call noundef zeroext i1 @_ZNKSt9type_infoeqERKS_(ptr {{.*}} @_ZTISt9type_info, ptr {{.*}} %2)
+if (typeid(std::type_info) == typeid(b))
+return 42;
+// AS: %7 = addrspacecast ptr addrspace(1) %6 to ptr
+// AS-NEXT: %call2 = call noundef zeroext i1 @_ZNKSt9type_infoneERKS_(ptr {{.*}} addrspacecast (ptr addrspace(1) @_ZTIi to ptr), ptr {{.*}} %7)
+// NO-AS: %call2 = call noundef zeroext i1 @_ZNKSt9type_infoneERKS_(ptr {{.*}} @_ZTIi, ptr {{.*}} %5)
+if (typeid(int) != typeid(b))
+return 1712;
+// AS: %11 = addrspacecast ptr addrspace(1) %10 to ptr
+// AS-NEXT: %call7 = call noundef ptr @_ZNKSt9type_info4nameEv(ptr {{.*}} %11)
+// NO-AS: %call7 = call noundef ptr @_ZNKSt9type_info4nameEv(ptr {{.*}} %8)
+if (typeid(A).name() == typeid(b).name())
+return 0;
+// AS: %15 = addrspacecast ptr addrspace(1) %14 to ptr
+// AS-NEXT: %call11 = call noundef zeroext i1 @_ZNKSt9type_info6beforeERKS_(ptr {{.*}} %15, ptr {{.*}} addrspacecast (ptr addrspace(1) @_ZTIf to ptr))
+// NO-AS:   %call11 = call noundef zeroext i1 @_ZNKSt9type_info6beforeERKS_(ptr {{.*}} %11, ptr {{.*}} @_ZTIf)
+if (typeid(b).before(typeid(float)))
+return 1;
+// AS: %19 = addrspacecast ptr addrspace(1) %18 to ptr
+// AS-NEXT: %call15 = call noundef i64 @_ZNKSt9type_info9hash_codeEv(ptr {{.*}} %19)
+// NO-AS: