[PATCH] D92394: [amdgpu] Teach one more case for assumed global pointers.

[Michael Liao via Phabricator via llvm-commits]

hliao abandoned this revision.
hliao added a comment.

the case is no longer valid considering concurrent kernel execution.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp:553
+  const Argument *Arg = dyn_cast_or_null<Argument>(getUnderlyingObject(Ptr));
+  if (Arg && Arg->onlyReadsMemory() && Arg->hasNoAliasAttr() &&
+      Arg->getParent()->getCallingConv() == CallingConv::AMDGPU_KERNEL)
----------------
arsenm wrote:
> hliao wrote:
> > arsenm wrote:
> > > Readonly is incorrect here, as it only indicates the function does not write to the memory and does not indicate the memory may be changed by another function/thread. I also don't think noalias is really the correct check for the owned object
> > That `readonly` atttribute is marked in function attribute deduction pass by checking every use of a pointer argument, including if it's passed into another sub-func. It's only marked as `readonly` when it's safe. It's safe to assume there's no write though a pointer argument marked with `readonly`.
> > `noalias` here is to ensure that there's no other path within the kernel function execution to modify that memory pointed by that pointer argument.
> > By checking both, we could ensure that memory is only prepared by the host.
> Yes, it infers that *this* function did not modify the memory. It's a step further to conclude that no function exists that's modifying the memory. A different kernel could have access to the same memory which it is writing to
yeah, by allowing concurrent kernel execution, the assumption is no longer held.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92394