[PATCH] D99071: [ASAN][AMDGPU] Add support for accesses to global and constant addrspaces
Vitaly Buka via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 26 18:11:53 PDT 2021
vitalybuka added reviewers: kda, eugenis.
vitalybuka added subscribers: kda, eugenis.
vitalybuka added a comment.
LGTM with few minor comments
@eugenis and @kda mostly FYI
================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1703-1711
+ if (TargetTriple.isAMDGPU()) {
+ Type *PtrTy = cast<PointerType>(Addr->getType()->getScalarType());
+ unsigned int AddrSpace = PtrTy->getPointerAddressSpace();
+ if (AddrSpace == 0)
+ InsertBefore = instrumentAMDGPUGenericAddress(
+ OrigIns, InsertBefore, Addr, TypeSize, IsWrite, SizeArgument);
+ if (AddrSpace == 3 || AddrSpace == 5)
----------------
can you please move more AMD specific code into instrumentAMDGPUGenericAddress?
================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1882
+ !(TargetTriple.isAMDGPU() &&
+ (G->getAddressSpace() == 1 || G->getAddressSpace() == 4)))
+ return false;
----------------
Can you please extract all these AddressSpace checks into some functions with self explaining names?
maybe isUnsupportedAmdAddressSpace()
same for 3,5
================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:2406-2408
+ GlobalVariable *NewGlobal = new GlobalVariable(
+ M, NewTy, G->isConstant(), Linkage, NewInitializer, "", G,
+ G->getThreadLocalMode(), G->getAddressSpace());
----------------
can you sent this as a separate NFC patch?
================
Comment at: llvm/lib/Transforms/Utils/ModuleUtils.cpp:92
for (auto *V : Values) {
- Constant *C = ConstantExpr::getBitCast(V, Int8PtrTy);
+ Constant *C = ConstantExpr::getPointerBitCastOrAddrSpaceCast(V, Int8PtrTy);
if (InitAsSet.insert(C).second)
----------------
can you sent this as a separate NFC patch?
does it affect any existing tests?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99071/new/
https://reviews.llvm.org/D99071
More information about the llvm-commits
mailing list