[PATCH] D21534: GlobalISel: first outline of legalization interface.

[Quentin Colombet via llvm-commits]

Hi Tim,

> On Jun 22, 2016, at 9:16 AM, Tim Northover <t.p.northover at gmail.com> wrote:
> 
> Hi Quentin,
> 
> On 21 June 2016 at 11:58, Quentin Colombet <qcolombet at apple.com> wrote:
>> I believe this is work in progress so I did not go into great details to comment on each part of the patch. For instance, for a detailed review, I would have expected the patch to be split into smaller one.
> 
> Sure, I'll try to look for smaller independently testable bits.
> 
>> - I would split the file with the legalization APIs in two:
>>  - one for the machine pass
>>  - one for the helper
>> 
>> The rationale is to be able to include them independently.
>> 
>> - I would split the LegalizeHelper in two:
>>  - One with all the methods you added (narrow, widen, etc), but legalize
>>  - One with the legalize API and the set action stuff
> 
> Yep, sounds compeltely reasonable.
> 
>> - Replace the Type/EVT by something lower level (Cf. my email)
>> 
>> Basically, we do not need to know what are the types, we only need to know their size and there number of lanes, i.e., I would go with a different types representation for the MI level.
> 
> I think we do need to know the the full types (at least MVT
> information) to decide on the correct action. <4 x float> could have
> very different requirements from <4 x i32>. I'm afraid I'm not sure
> what e-mail you're talking about either, do you have a link handy?

I was talking of the offline discussion in "GISel - reality check"

That’s just a remark in that email, the basic idea is what I said, I.e., I think we only need the size and number of lanes.
Let me explain a bit why I think it is sufficient.
<4 x float> and <4 x i32> are <4 x 32-bit> for most operations (load, store, and, shuffle, etc.). The only cases where that make a difference AFAICT are arithmetic operations. However, for those, the type is actually encoded in the opcode, e.g., fadd vs. add (we do not have G_FADD yet but I think we should :)). In other word, when type matters, the opcode should give us the information otherwise we should be able to get away of that business.

Cheers,
-Quentin

> 
>> - Add unit test cases for the legalize helper.
> 
> Good idea. That'll certainly help split the patch into smaller slices.
> 
> Cheers.
> 
> Tim.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160622/6a958776/attachment.html>