[PATCH] D69498: IR: Invert convergent attribute handling

Tue Oct 29 12:54:32 PDT 2019

arsenm added a comment.

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

> Maybe we can start by looking into the motivation for this patch:
>
> > There is a burden on frontends in environments that care about convergent operations to add the attribute just in case it is needed. This has proven to be somewhat problematic, and different frontend projects have consistently run into problems related to this.
>
> Can you clarify what kind of problems? Do you have concrete examples?

Basically no frontend has gotten this right, including clang and non-clang frontends. There's a gradual discovery process that it's necessary in the first place, and then a long tail of contexts where it's discovered it's missing. As I mentioned before, SYCL and OpenMP are both still broken in this regard. For example in clang, convergent wasn't initially added to all functions, and then it was discovered that transforms on indirect callers violated it. Then later it was discovered this was broken for inline asm. Then some specific intrinsic call sites were a problem. Then there are special cases that don't necessarily go through the standard user function codegen paths which don't get the default attributes. Some odd places where IR is compiled into the user library that didn't have convergent added. The discovery of convergent violations is painful; more painful than discovering that control flow didn't optimize the way you expected. It requires eternal vigilance and all developers to be aware of it.

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 introduce bugs when calls are broken by default.

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

https://reviews.llvm.org/D69498