[PATCH] D99071: [ASAN][AMDGPU] Add support for accesses to global and constant addrspaces

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka accepted this revision.
vitalybuka added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:176-177
 
+const char kAMDGPUAddressSharedName[] = "llvm.amdgcn.is.shared";
+const char kAMDGPUAddressPrivateName[] = "llvm.amdgcn.is.private";
+
----------------
rksharma wrote:
> Is it fine that I've created variables for them or shall I directly use function names as string inside getOrInsertFunction? 
variable lgtm


================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1326-1332
+static bool isUnsupportedAMDGPUAddrspace(Value *Addr) {
+  Type *PtrTy = cast<PointerType>(Addr->getType()->getScalarType());
+  unsigned int AddrSpace = PtrTy->getPointerAddressSpace();
+  if (AddrSpace == 3 || AddrSpace == 5)
+    return true;
+  return false;
+}
----------------



================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1695-1697
+  if (isUnsupportedAMDGPUAddrspace(Addr))
+    return nullptr;
+  Type *PtrTy = cast<PointerType>(Addr->getType()->getScalarType());
----------------



================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1896
+  if (G->getAddressSpace() &&
+      !(TargetTriple.isAMDGPU() && !isUnsupportedAMDGPUAddrspace(G)))
+    return false;
----------------



================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1684
+    uint32_t TypeSize, bool IsWrite, Value *SizeArgument) {
+  // Do not instrument unsupported addrspaces
+  if(isUnsupportedAMDGPUAddrspace(Addr)) return nullptr;
----------------
vitalybuka wrote:
> And the rest of the patch.
"." after comments is not done?


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

https://reviews.llvm.org/D99071