[PATCH] D32737: [Constants][SVE] Represent the runtime length of a scalable vector

Chandler Carruth via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun May 21 13:26:24 PDT 2017


chandlerc added a comment.

In https://reviews.llvm.org/D32737#760472, @chandlerc wrote:

> There is a lot of discussion here that I really don't think should be on a patch review. It should be an an llvm-dev thread. See below.
>
> In https://reviews.llvm.org/D32737#760433, @rengolin wrote:
>
> > In https://reviews.llvm.org/D32737#760430, @lattner wrote:
> >
> > > > From my point of view, making scalable vectors a native and core type of IR is the only way forward, because the semantics needs to be ingrained in the language to make sense. AFAICS, at the IR level, the differences between vectors and scalable ones is not that big a deal, certainly not bigger than vectors versus scalar.
> > >
> > > This is much more up for debate.  Details matter.  Compilers are a set of engineering tradeoffs.  You need to explain and defend your position carefully, not just state it as though it were "obviously true".
> >
> >
> > I though that was clear from my previous response to you: "I don't think anyone disagrees with that point."
> >
> > I'm certainly not asking people to "trust me, I'm an engineer". We had previous discussions in the list, other people chimed in. This is not even my patch and I have nothing to do with their work... "I just work here".
> >
> > I'm also certainly not asserting that I know all the answers and that this change is worry free, by any means, and that's precisely why I pinged more people to give their opinions, because I was uncomfortable with the low amount of reviews. But it was certainly not "just me".
> >
> > > Do you have a discussion somewhere of the overall design of the extensions you're proposing?  This is presumably only a small piece of it.
> >
> > There were discussions on the list, a "whole-patch approach" in Github and some previous patches. I can't find any of it (my mail client - gmail - is a mess). I'll let Graham cover that part.
>
>
> Yes, but in that discussion, I specifically asked for a new, fresh RFC thread that reflected substantial changes made to the design presented in the first RFC email through the discussion. That thread, AFAICT, never happened. If I missed, it I'm happy to get a pointer to it. If not, the author of this patch should start it. Either way, that is where this discussion should take place. I think there remain a lot of important technical issues here. I would like to dig into them, but I *don't* want to do it here where I don't even have the complete design.
>
> I would encourage everyone (Hal, Sanjoy, etc) to hold off on debating "how does X work" here and redirect that to the (either existing or eventual) llvm-dev thread with an updated overall design.


Put differently, this patch makes sense *once* we clearly have consensus on llvm-dev. So far, the only thread I can find did not reach any meaningful consensus. Notably, none of the code generator people were heavily contributing to that thread, and there remain large unmentioned technical concerns (stack spills, alloca size, etc)


https://reviews.llvm.org/D32737





More information about the llvm-commits mailing list