[PATCH] D123531: [GlobalsModRef][FIX] Ensure we honor synchronizing effects of intrinsics

[Artem Belevich via Phabricator via llvm-commits]

tra added a comment.

> I'm not 100% sure if the issue underlying https://reviews.llvm.org/D115302 would be solved by this or not.

You've included the `functions_without_nosync.ll` test from D115302 <https://reviews.llvm.org/D115302> which verifies that this patch fixes that issue, too. AFAICT, both patches are trying to address the same issue, but due to my lack of expertise, I was (overly?) cautious and trying to change as little as possible, and probably was doing it in a wrong place, too.

We need to document how `nosync` (or lack of it) interacts with other attributes describing side effects. E.g. interaction between `IntrReadMem`, `IntrArgMemOnly ` and (lack of) `nosync`.

One concern raised on D115302 <https://reviews.llvm.org/D115302> was the impact on back-ends that have not been upgraded to use `DefaultIntrinsic`. For them this patch would prevent some optimizations. This could potentially be an issue for users who may use such intrinsic in the hot path and they will have no easy way to work around that, as intrinsic properties are not under user control. Should we consider having a temporary hidden escape hatch option to revert to the old behavior?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123531