[PATCH] D69498: IR: Invert convergent attribute handling

Matt Arsenault via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 30 11:02:23 PDT 2019


arsenm added a comment.

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

> In D69498#1727080 <https://reviews.llvm.org/D69498#1727080>, @jdoerfert wrote:
>
> > 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.
>
>
> I am not convinced by this: this seems overly simplistic. The fact that convergent wasn't rolled out properly in frontend can also been as an artifact of the past. There is a logical gap between this and concluding that the only solution is to change the default. For instance someone mentioned a pass that could be inserted as the very beginning of the pipeline by any GPU compiler and add the convergent attribute everywhere.


Avoiding human error is fundamental to good design. If you have to have an additional IR pass, then there's already a phase where the IR is broken and something could legally break the IR.

> 
> 
>>> 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.
> 
> This view seems appropriate to me only if you consider a GPU (or SIMT) compiler alone. Another view (which seems to be what @rjmccall has) is that SIMT/GPU is a marginal use-case and "soundness" is already existing for most LLVM use-cases.

I think this is a concerning argument. Declaring that GPUs are a "marginal" use case and LLVM only follows its design principles in cases where it benefits C++ x86/ARM users is an issue. In that case LLVM is no longer trying to be the compiler infrastructure for everyone that it tries to be. Soundness for most doesn't sound like a good design. I've proposed attributes in the past that were shot down for not following a correct by-default design and to me it seems like a problem if this principle isn't going to be universally followed. It's additionally concerning since most GPUs uses LLVM in some fashion if they're going to be declared a second class use case.


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

https://reviews.llvm.org/D69498





More information about the cfe-commits mailing list