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

Tim Northover via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 22 09:16:52 PDT 2016


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?

> - Add unit test cases for the legalize helper.

Good idea. That'll certainly help split the patch into smaller slices.

Cheers.

Tim.


More information about the llvm-commits mailing list