[PATCH] D32530: [SVE][IR] Scalable Vector IR Type

Fri Apr 5 02:36:46 PDT 2019

huntergr added a comment.

> I think this patch is a good start and I don't mind having it merged as is, I was just suggesting a way forward since it seems people are still hesitating. While there seems to be some consensus on moving forward with native types, I'm sure many of the details are still somewhat fuzzy. In my opinion having more patches reviewed would make those details clear, and it would thus make it easier to get more sign-offs on this patch as well.

Ok. I'll see if I can get some time to work on updating other core parts for review.

>>> I agree that this is a significant change and I can understand why people are a bit nervous about merging it. Would it help if we had more middle-end patches reviewed before committing, so people could have a better understanding of the impact? Off the top of my head, the rework of D35137 <https://reviews.llvm.org/D35137> would be interesting, and also constant handling.
>>> 
>>> Can you also tell us what the plan is regarding all the places in the optimizer that may need updating to handle the new vectors?
>> 
>> A patch series has been posted for review, see section 7. in
>>  http://lists.llvm.org/pipermail/llvm-dev/2018-June/123780.html
> 
> My understanding is that Graham is reworking some of those. Some were only RFC to begin with, some have been abandoned since, and in any case there has been some back-and-forth on the mailing list since then. I think now would be a good time to rebase/update all the patches that are still relevant so people can see the current version of things.

Indeed. We are considering a github repo with a more complete implementation split into sensible patches (unlike the current repo with a single megapatch that covers far more than just SVE support). It will take some time to prepare that, though.

> Also, my question regarding updating the optimizer still stands, I don't think I've seen patches related to updating the existing passes to take the 'scalable' property into account. While in some places it's likely to have no impact, I'd be very surprised if you could pass an IR file with scalable types through the optimizer and not get a soup of fixed-width and scalable vectors out the other end (or the corresponding assertions about mismatched types). I'd be happy to help deal with that churn, by the way :) that's rather why I was asking about the plan.

There aren't that many changes (imo), but I understand that you'd like to see for yourself what they are. I'll see what the opinion is on the repo vs. lots of phabricator patches at EuroLLVM.

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

https://reviews.llvm.org/D32530