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

Mon Jul 26 08:01:59 PDT 2021

jdoerfert added a comment.

In D106749#2902911 <https://reviews.llvm.org/D106749#2902911>, @nikic wrote:

> 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 --

correct yes, confusing still, anyway.

> If we do want to rename it, do you have suggestions on what the name should be?

good questions, doesn't matter now so let's not try to name things, that's hard anyway.

> 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 [...]".

write/readonly/none use it and it is not clear to me if they should now not be applicable to functions with endless loops (I hope they are still as it composes with willreturn).
mustprogress uses it and we can probably rephrase that as it talks about terminating loops without side-effects.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106749