[PATCH] D115302: GlobalsModRef should treat functions w/o nosync conservatively.

Artem Belevich via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 9 14:08:02 PST 2021


tra added a comment.

In D115302#3183783 <https://reviews.llvm.org/D115302#3183783>, @jdoerfert wrote:

> The properties we associate (somewhere) with intrinsics are captured by `nosync` and `nocallback`. Neither is inherent to intrinsics but we pretend they are and that causes the bug. Only by making these explicit we can fix the bug without just hiding it, and treat functions that have the same properties the same way.

This may be the place which causes different behavior of intrinsics vs functions.
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Analysis/CallGraph.cpp#L96

That said, we seem to have multiple moving parts here, that may be worth separating:

- lack of `nosync` should force GlobalModRef to treat functions conservatively.

  Problem: not all intrinsics have been converted to use default attributes, so it will be a performance regression for them.

- Some intrinsics *may* call back and that's currently covered only by an incomplete`isLeaf`.

  Problem: `nocallback` is not here yet and we have the same issue with the default attributes as above.

- Both issues above will not have a quick fix, but we do need to deal with the miscompiles we have now.

  Q: What can we do short-term to mitigate the issue?

Perhaps as a stop-gap workaround, we can explicitly enumerate NVPTX intrinsics that must be treated as a synchronization barrier. 
AFAICT, the only other platform that may potentially be affected by this is AMDGPU, so the special case list should be manageable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115302



More information about the llvm-commits mailing list