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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D115302#3183741 <https://reviews.llvm.org/D115302#3183741>, @nikic wrote:

> Please let's not bring `nocallback` into this. It's an as-yet completely unused attribute, and we haven't even converted most target intrinsics to use default attributes yet. Changing `Intrinsic::isLeaf()` to use an attribute instead of a hard-coded list seems completely orthogonal to this issue.

It is not orthogonal. The problem doesn't happen for functions because we think intrinsics have a magic property, or in fact multiple, that cannot cause an effect in certain situations. This is plain wrong. From there things will always go sideways.

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.

All that said, we obviously need a lang ref patch for `nocallback`.

Finally, not having converted target intrinsics is even more reason to do this. Pretending intrinsics are special in any way you just need to when you write an analysis/optimization is always going to cause bugs. If we do not specify properties explicitly we are fighting a loosing battle. If we make them explicit however, people might take a look at their target directives and port them, as we ported the nvptx ones recently ;).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115302