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

Wed Apr 13 11:59:55 PDT 2022

tra added a comment.

In D123531#3447211 <https://reviews.llvm.org/D123531#3447211>, @jdoerfert wrote:

> In D123531#3446219 <https://reviews.llvm.org/D123531#3446219>, @tra wrote:
>
>> 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`.
>
>
>
> In D123531#3447021 <https://reviews.llvm.org/D123531#3447021>, @tra wrote:
>
>> @jdoerfert:  A friendly nag to update intrinsic attribute docs. Looks like they didn't make it to the landed change.
>
> You mean the above?

Yes.

> That ain't something I can actually "just do". My opinion and the opinion of others are known to be different.

You do have the power to change the way LLVM works. I believe you do have the power to document how things work **now** and save other folks who just want to create an intrinsic the hassle of finding out about the quirks the hard way, when something breaks.

> What we need (in my opinion) is a major overhaul of our idea of side effects. We did a first step with termination but now we need to make "sync" and "write" different things. Even writing that up will take some time.

IMO, that's independent of documenting the current state of affairs. For what it's worth, I do agree with you on the nature of the problem we have describing intrinsics' side effects.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123531