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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
