[PATCH] D105721: [amdgpu] Add scope metadata support for noalias kernel arguments.

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

Needs some IR tests showing the metadata.

Is this the state of the art for scoped AA? I thought scoped restrict was going to involve new intrinsics (which we could also insert up front if we were consistently using byref to begin with)



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerKernelArguments.cpp:148
+
+      if (const LoadInst *LI = dyn_cast<LoadInst>(I))
+        PtrArgs.push_back(LI->getPointerOperand());
----------------
Do you really need to go through all the instruction types? I think there's a utility function for this somewhere


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerKernelArguments.cpp:159
+      else if (const auto *Call = dyn_cast<CallBase>(I)) {
+        // If we know that the call does not access memory, then we'll still
+        // know that about the inlined clone of this call site, and we don't
----------------
I think this can be a bit smarter with intrinsics (ideally the helper would handle this too) . We already have to report which of the arguments are pointers


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerKernelArguments.cpp:162
+        // need to add metadata.
+        if (Call->doesNotAccessMemory())
+          continue;
----------------
Should check the callsite in case it's marked readnone, not the call itself


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105721