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

[Dan Gohman]

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?

>>
>>
>>
>> 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