[PATCH] D106749: [IR] Consider non-willreturn as side effect

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D106749#2902631 <https://reviews.llvm.org/D106749#2902631>, @jdoerfert wrote:

> I like us to check `willreturn` and consider it an effect. I think we should rename the function though, two reasons:
>
> 1. it changes the behavior and people will be confused.
> 2. lang ref talks about "side-effects" and we would need to make it consistent (e.g., for `mustprogress`)
>
> That said, 2) should be addressed anyway separately anyway. I would not oppose to look for side-effect everywhere and define it
> to include termination properly. Does this make sense?

I don't think a rename is necessary here, because the change is a conservatively correct one -- if a use of mayHaveSideEffects() is "incorrect", then at worst that is a performance regression, but it will never cause a miscompile. If we do want to rename it, do you have suggestions on what the name should be? The current "side effect" terminology seems pretty good (you can't add, remove or reorder side effects, which is also true for non-willreturn calls).

I looked at uses of "side effect"/"side-effect" in LangRef, and it seems like the term is currently being used colloquially rather than normatively. We should probably replace uses of this term with a more precise description depending on context. For example "If a readonly function writes memory visible to the program, or has other side-effects [...]" should probably be something like "If a readonly function writes memory visible to the program, or synchronizes [...]".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106749