[PATCH] D118661: [AMDGPU] Check atomics aliasing in the clobbering annotation

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec marked an inline comment as done.
rampitec added a comment.

In D118661#3287271 <https://reviews.llvm.org/D118661#3287271>, @foad wrote:

>> MemorySSA considers any atomic a def to any operation it dominates
>
> I guess this is because of the problem described by the TODO in MemorySSA::createNewAccess:
>
>   // The isOrdered check is used to ensure that volatiles end up as defs
>   // (atomics end up as ModRef right now anyway).  Until we separate the
>   // ordering chain from the memory chain, this enables people to see at least
>   // some relative ordering to volatiles.  Note that getClobberingMemoryAccess
>   // will still give an answer that bypasses other volatile loads.  TODO:
>   // Separate memory aliasing and ordering into two different chains so that
>   // we can precisely represent both "what memory will this read/write/is
>   // clobbered by" and "what instructions can I move this past".
>
> ?
>
> Maybe a cleaner way to handle this would be to implement a new MemorySSAWalker subclass where getClobberingMemoryAccess(I) ignores ordering, and only returns a def that mayAlias I. But I suppose that's more work.

Yes, that's the issue. This is how ordering is modeled.


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

https://reviews.llvm.org/D118661