[PATCH] D153092: [Clang][CodeGen]`vtable`, `typeinfo` et al. are globals

[Alex Voicu via Phabricator via cfe-commits]

AlexVlx added inline comments.


================
Comment at: clang/lib/CodeGen/CGVTables.cpp:836
+        fnPtr =
+            llvm::ConstantExpr::getAddrSpaceCast(fnPtr, CGM.GlobalsInt8PtrTy);
+      return builder.add(fnPtr);
----------------
efriedma wrote:
> If I follow correctly, the fnPtr is guaranteed to be in the global address-space, but GetAddrOfFunction returns a generic pointer.  So the vtable entries are in the global address-space for efficiency? Seems reasonable.
> 
> There isn't really much point to explicitly checking `FnAS != GVAS` before the call to getAddrSpaceCast; getAddrSpaceCast does the same check internally.
Right, we know that these are going to be in global memory, and there is overhead when dealing with flat/generic. I would've liked to remove the check, but I think that's infeasible with how getAddrSpaceCast is currently implemented, because it `assert`s on `castIsValid`; addrspacecasts from the same AS to the same AS are invalid, so the `assert` flares on targets where FnAS == GVAS (e.g. x86). This is not super ergonomic IMHO, as it should be valid & a NOP just returning the source, but that's a change that would be required to allow deleting the silly check, I believe.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153092