[llvm-commits] Patch: generalize vector shuffle

[Mon Ping Wang]

Hi Duncan,

On Nov 5, 2008, at 12:42 PM, Duncan Sands wrote:

> Hi Mon Ping,
>
>>>> +//===---- LegalizeTypesUtils.cpp - Utilities for Legalization of
>>>> types
>>>
>>> Why not only do it in LegalizeTypes?
>>
>> Are you asking why these utilities are not part of LegalizeTypes
>> instead of placing them in their own class?  If so, LegalizeDAG would
>> need access to the utility...
>
> why does LegalizeDAG need access to this at all?  At some point I'm
> going to start removing all type legalization code from LegalizeDAG.
> Not quite yet, I admit!  However is there really much point in adding
> new type legalization code to LegalizeDAG?

My understanding is that LegalizeType is optional so I wanted to be  
able to turn on and off LegalizeType and get the same code coming out  
as some people might turn off LegalizeType.  I do see your point that  
if LegalizeType is going to be required soon, it seems silly to add  
functionality to LegalizeDAG and then rip it out again.  What I'll do  
is wait to checkin the LegalizeDAG changes until I see a real need for  
them for what we are doing and just checkin the code for LegalizeTypes.

>> The changes in LegalizesTypes are needed to take advantage of
>> generalized the vector shuffle.   I should have copied my original
>> email for the patch to this last patch as an introduction.   With
>> these changes, we try to keep as much as the original vector program
>> (after optimization) structure all the way to the backend.  The
>> incoming program uses generalize vector shuffles, in SelectionDAG
>> building, we normalize, and then in Legalization, we convert any of
>> the illegal normalized shuffles to concat, extract sub vectors, etc.
>> Before, legalization would break it down to insert/extracts which we
>> had a hard time rebuilding.   For examples like transposing a vector,
>> this allows us to generate much better code.  Sorry about the  
>> confusion!
>
> if I understand right, the changes to LegalizeTypes/LegalizeDAG are
> logically independent of the generalization of vector shuffle (and  
> must
> be, because this code never sees generalized vector shuffles!): they
> would already make sense without the vector shuffle generalization.
> Maybe you can break them out of the patch and submit them separately.
> For example, the EXTRACT_SUBVECTOR part could go in at once (with the
> small cleanups I mentioned), and likewise for the vector shuffle
> legalization, after a bit more cleaning/simplifying/reviewing.

I don't have a problem of breaking it into two patches if they will be  
easier for people to digest.  The two patches will follow relatively  
quickly one after the other though (after another review for the  
shuffle one of course :-).

> Just a thought,
>
> Duncan.

-- Mon Ping