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

[Eli Friedman]

On Fri, May 22, 2009 at 11:15 AM, Dan Gohman <gohman at apple.com> wrote:
>
> On May 20, 2009, at 9:14 PM, Eli Friedman wrote:
>
>
>> On Wed, May 20, 2009 at 5:26 PM, Eli Friedman
>> <eli.friedman at gmail.com> wrote:
>>
>>> On Wed, May 20, 2009 at 4:55 PM, Dan Gohman <gohman at apple.com> wrote:
>>>
>>>> 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