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

[AlexVlx]

Summary

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.

[yaxunl]

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.

[AlexVlx]

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 asserts we just do the bitcast, which happens to work on our target).

[arsenm]

clang/lib/CodeGen/CGCall.cpp

5237–5238

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

[rjmccall]

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.

[AlexVlx]

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

[AlexVlx]

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 suggested, 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.

[AlexVlx]

clang/lib/CodeGen/CGCall.cpp

5237–5238

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 asserts flare. I'm not sure if that is vestigial or just overly cautious behaviour.

[AlexVlx]

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

[AlexVlx]

Gentle ping.

[yaxunl]

LGTM. Thanks.

clang/lib/CodeGen/CGCall.cpp

         // If the argument doesn't match, perform a bitcast to coerce it.  This
         // can happen due to trivial type mismatches.
         if (FirstIRArg < IRFuncTy->getNumParams() &&
-            V->getType() != IRFuncTy->getParamType(FirstIRArg))
+            V->getType() != IRFuncTy->getParamType(FirstIRArg)) {
+          auto VTy = V->getType();
+          auto IRTy = IRFuncTy->getParamType(FirstIRArg);
+
+          if (VTy->isPointerTy() &&
+              VTy->getPointerAddressSpace() != IRTy->getPointerAddressSpace()) {
+            // In the case of targets that use a non-default address space for
+            // globals it is possible to get a mismatch between the AST address
+            // space of type_info formals and the IR type of type_info actuals.
+            auto Ty = info_it->type.getTypePtr()->getPointeeType();
+            if (auto CD = Ty->getAsCXXRecordDecl()) {
+              if (CD->isInStdNamespace() && CD->getName() == "type_info")
+                V = Builder.CreateAddrSpaceCast(V, IRTy);
+              else
+                llvm_unreachable("Unexpected address space mismatch.");
+            }
+          } else {
+            // If the argument doesn't match, perform a bitcast to coerce it.
+            // This can happen due to trivial type mismatches.
             V = Builder.CreateBitCast(V, IRFuncTy->getParamType(FirstIRArg));
+          }
+        }
 
         if (ArgHasMaybeUndefAttr)
           V = Builder.CreateFreeze(V);

clang/lib/CodeGen/CGExprCXX.cpp

 }
 
 llvm::Value *CodeGenFunction::EmitCXXTypeidExpr(const CXXTypeidExpr *E) {
-  llvm::Type *PtrTy = llvm::PointerType::getUnqual(getLLVMContext());
+  llvm::Type *PtrTy = CGM.GlobalsInt8PtrTy;
 
   if (E->isTypeOperand()) {
     llvm::Constant *TypeInfo =

clang/lib/CodeGen/ItaniumCXXABI.cpp

                                        llvm::Type *StdTypeInfoPtrTy) {
   auto *ClassDecl =
       cast<CXXRecordDecl>(SrcRecordTy->castAs<RecordType>()->getDecl());
-  llvm::Value *Value = CGF.GetVTablePtr(
-      ThisPtr, llvm::PointerType::getUnqual(CGF.getLLVMContext()), ClassDecl);
+  llvm::Value *Value = CGF.GetVTablePtr(ThisPtr, CGM.GlobalsInt8PtrTy,
+                                        ClassDecl);
 
   if (CGM.getItaniumVTableContext().isRelativeLayout()) {
     // Load the type info.

clang/test/CodeGenCXX/typeid-cxx11-with-address-space.cpp

+// RUN: %clang_cc1 -I%S %s -triple amdgcn-amd-amdhsa -emit-llvm -std=c++11 -o - | FileCheck %s
+#include <typeinfo>
+
+namespace Test1 {
+
+struct Item {
+  const std::type_info &ti;
+  const char *name;
+  void *(*make)();
+};
+
+template<typename T> void *make_impl() { return new T; }
+template<typename T> constexpr Item item(const char *name) {
+  return { typeid(T), name, make_impl<T> };
+}
+
+struct A { virtual ~A(); };
+struct B : virtual A {};
+struct C { int n; };
+
+// CHECK: @_ZN5Test15itemsE ={{.*}} constant [4 x {{.*}}] [{{.*}} ptr addrspacecast (ptr addrspace(1) @_ZTIN5Test11AE to ptr), {{.*}} @_ZN5Test19make_implINS_1AEEEPvv {{.*}} ptr addrspacecast (ptr addrspace(1) @_ZTIN5Test11BE to ptr), {{.*}} @_ZN5Test19make_implINS_1BEEEPvv {{.*}} ptr addrspacecast (ptr addrspace(1) @_ZTIN5Test11CE to ptr), {{.*}} @_ZN5Test19make_implINS_1CEEEPvv {{.*}} ptr addrspacecast (ptr addrspace(1) @_ZTIi to ptr), {{.*}} @_ZN5Test19make_implIiEEPvv }]
+extern constexpr Item items[] = {
+  item<A>("A"), item<B>("B"), item<C>("C"), item<int>("int")
+};
+
+// CHECK: @_ZN5Test11xE ={{.*}} constant ptr addrspacecast (ptr addrspace(1) @_ZTIN5Test11AE to ptr), align 8
+constexpr auto &x = items[0].ti;
+
+// CHECK: @_ZN5Test11yE ={{.*}} constant ptr addrspacecast (ptr addrspace(1) @_ZTIN5Test11BE to ptr), align 8
+constexpr auto &y = typeid(B{});
+
+}

clang/test/CodeGenCXX/typeid-with-address-space.cpp

+// RUN: %clang_cc1 -I%S %s -triple amdgcn-amd-amdhsa -emit-llvm -fcxx-exceptions -fexceptions -o - | FileCheck %s
+#include <typeinfo>
+
+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 &int_ti = typeid(int);
+
+// CHECK: @_ZN5Test14A_tiE ={{.*}} constant ptr addrspacecast (ptr addrspace(1) @_ZTIN5Test11AE to ptr), align 8
+const std::type_info &A_ti = typeid(const volatile A &);
+
+volatile char c;
+
+// CHECK: @_ZN5Test14c_tiE ={{.*}} constant ptr addrspacecast (ptr addrspace(1) @_ZTIc to ptr), align 8
+const std::type_info &c_ti = typeid(c);
+
+extern const double &d;
+
+// CHECK: @_ZN5Test14d_tiE ={{.*}} constant ptr addrspacecast (ptr addrspace(1) @_ZTId to ptr), align 8
+const std::type_info &d_ti = typeid(d);
+
+extern A &a;
+
+// CHECK: @_ZN5Test14a_tiE ={{.*}} global
+const std::type_info &a_ti = typeid(a);
+
+// CHECK: @_ZN5Test18A10_c_tiE ={{.*}} constant ptr addrspacecast (ptr addrspace(1) @_ZTIA10_c to ptr), align 8
+const std::type_info &A10_c_ti = typeid(char const[10]);
+
+// CHECK-LABEL: define{{.*}} ptr @_ZN5Test11fEv
+// CHECK-SAME:  personality ptr @__gxx_personality_v0
+const char *f() {
+  try {
+    // CHECK: br i1
+    // CHECK: invoke void @__cxa_bad_typeid() [[NR:#[0-9]+]]
+    return typeid(*static_cast<A *>(0)).name();
+  } catch (...) {
+    // CHECK:      landingpad { ptr, i32 }
+    // CHECK-NEXT:   catch ptr null
+  }
+
+  return 0;
+}
+
+}
+
+// CHECK: attributes [[NR]] = { noreturn }

clang/test/CodeGenCXX/typeinfo

     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<unsigned long long>(__name);
+    }
   protected:
     const char *__name;
   };

clang/test/CodeGenCXX/typeinfo-with-address-space.cpp

+// 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 <typeinfo>
+
+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: %call15 = call noundef i64 @_ZNKSt9type_info9hash_codeEv(ptr {{.*}} %14)
+    return typeid(b).hash_code();
+}