[PATCH] Add new phase to legalization to handle vector operations

Per subject, this patch adding an additional pass to handle vector
operations; the idea is that this allows removing the code from
LegalizeDAG that handles illegal types, which should be a significant
simplification. There are still some issues with this patch, but does
the approach look sane?

-Eli

legalizevecops.txt (10.7 KB)

New version with a few minor changes and fixed-up comments; the
previous version had some comments with inaccurate statements, which
might be confusing for reviewing.

-Eli

legalizevecops.txt (10.5 KB)

Can you explain why you chose the approach of using a new pass?
I pictured removing LegalizeDAG's type legalization code would
mostly consist of finding all the places that use TLI.getTypeAction
and just deleting code for handling its Expand and Promote. Are you
anticipating something more complicated?

Dan

Consider the case of an illegal operation (like division) on a <2 x
i64> on x86-32. If we unroll it, the result has to go through type
legalization. Therefore, if we want to remove the type legalization
code from LegalizeDAG, we have to unroll vector operations first, then
run a round of type legalization. (There are potential alternatives,
like making LegalizeDAG call back into type legalization, but it gets
messy.)

Actually, I went ahead and tried writing the patch to remove type
legalization from LegalizeDAG; I actually ran into another issue,
which is the legalization of i32 UINT_TO_FP. We want to legalize it
into an i64 UINT_TO_FP, but that requires type legalization. I'm not
quite sure what to do about that; if I can't come up with a better
solution, I'll force the x86 backend to custom-lower that case.

-Eli

Attached is a more complete version, including the simplification of
LegalizeDAG and the fixes for UINT_TO_FP and FP_TO_UINT on x86. It
passes regression tests except for one failure due to a missing
implementation of legalization for EXTRACT_SUBVECTOR where both types
are legal.

-Eli

x.txt (207 KB)

Can you explain why you chose the approach of using a new pass?

I pictured removing LegalizeDAG's type legalization code would

mostly consist of finding all the places that use TLI.getTypeAction

and just deleting code for handling its Expand and Promote. Are you

anticipating something more complicated?

Consider the case of an illegal operation (like division) on a <2 x

i64> on x86-32. If we unroll it, the result has to go through type

legalization. Therefore, if we want to remove the type legalization

code from LegalizeDAG, we have to unroll vector operations first, then

run a round of type legalization. (There are potential alternatives,

like making LegalizeDAG call back into type legalization, but it gets

messy.)

Ah, ok. Thanks for explaining that. Could you put this in a comment?

Actually, I went ahead and tried writing the patch to remove type

legalization from LegalizeDAG; I actually ran into another issue,

which is the legalization of i32 UINT_TO_FP. We want to legalize it

into an i64 UINT_TO_FP, but that requires type legalization. I'm not

quite sure what to do about that; if I can't come up with a better

solution, I'll force the x86 backend to custom-lower that case.

Attached is a more complete version, including the simplification of
LegalizeDAG and the fixes for UINT_TO_FP and FP_TO_UINT on x86. It
passes regression tests except for one failure due to a missing
implementation of legalization for EXTRACT_SUBVECTOR where both types
are legal.

Should it be named something other than LegalizeVectors then,
since it handles more than just vectors? Perhaps something
like LegalizeOpsNeedingNewTypes?

I haven't looked into details, but I think the approach
in the patch looks reasonable. Adding yet another pass is
somewhat undesirable, though it's better than having two
type legalizers, and better than having a complicated
scheme to call back into LegalizeTypes. And we can always
revisit this in the future.

Dan

Can you explain why you chose the approach of using a new pass?

I pictured removing LegalizeDAG's type legalization code would

mostly consist of finding all the places that use TLI.getTypeAction

and just deleting code for handling its Expand and Promote. Are you

anticipating something more complicated?

Consider the case of an illegal operation (like division) on a <2 x

i64> on x86-32. If we unroll it, the result has to go through type

legalization. Therefore, if we want to remove the type legalization

code from LegalizeDAG, we have to unroll vector operations first,
then

run a round of type legalization. (There are potential alternatives,

like making LegalizeDAG call back into type legalization, but it gets

messy.)

Ah, ok. Thanks for explaining that. Could you put this in a comment?

Okay, will do.

Actually, I went ahead and tried writing the patch to remove type

legalization from LegalizeDAG; I actually ran into another issue,

which is the legalization of i32 UINT_TO_FP. We want to legalize it

into an i64 UINT_TO_FP, but that requires type legalization. I'm not

quite sure what to do about that; if I can't come up with a better

solution, I'll force the x86 backend to custom-lower that case.

Attached is a more complete version, including the simplification of
LegalizeDAG and the fixes for UINT_TO_FP and FP_TO_UINT on x86. It
passes regression tests except for one failure due to a missing
implementation of legalization for EXTRACT_SUBVECTOR where both types
are legal.

Should it be named something other than LegalizeVectors then,
since it handles more than just vectors? Perhaps something
like LegalizeOpsNeedingNewTypes?

No, it only handles vectors; I worked around the UINT_TO_FP it by
custom-lowering the construct in the x86 backend. I think it's the
only case of something like that; x86 is weird in having a legal
SINT_TO_FP operation that takes an illegal type.

I haven't looked into details, but I think the approach
in the patch looks reasonable. Adding yet another pass is
somewhat undesirable, though it's better than having two
type legalizers, and better than having a complicated
scheme to call back into LegalizeTypes. And we can always
revisit this in the future.

Okay, cool.

-Eli