[PATCH] D69498: IR: Invert convergent attribute handling

Wed Oct 30 07:47:15 PDT 2019

jdoerfert added a comment.

In D69498#1726610 <https://reviews.llvm.org/D69498#1726610>, @mehdi_amini wrote:

> > The short version is the fact that most developers aren't aware of and don't understand the subtleties of convergence, and making sure the front-end does something in all contexts requires a lot of diligence. It's very easy to introduce these difficult to debug bugs when calls are broken by default.
>
> The fact that most developers aren't aware of convergence means a lot of potential bugs in LLVM because passes and transformations won't honor the convergent semantics, but the default for the attribute won't change that.

While you are right, a default non-convergent behavior will not make all bug sources disappear, it will make *a lot of bug sources disappear*. Let me quote @arsenm here because this is so important: "Basically no frontend has gotten this right, including clang and non-clang frontends." For me, that statement alone is reason to change the status quo.

> As of the frontend, it seems to me that this is just about structuring the code-path for function emission to define the right set of default attribute. It isn't clear to me why a refactoring of the frontend isn't a better course of actions here.

Whatever we do, there will be consequences for current and future front-ends. At the end of the day, the underlying question we need to answer here is: Do we want to have "optimization with opt-in soundness" or "soundness with opt-in optimizations", and I would choose the latter any time.

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

https://reviews.llvm.org/D69498