[llvm-commits] Buggy SelectionDAG binop vector widening

[Visa Putkinen]

Hi!

Sorry for the long delay.

On Wed, Jun 09, 2010 at 11:25:20PM -0700, Mon Ping Wang wrote:
> All the occurrences in widening should use isTypeLegal should be updated to use isTypeSynthesizable but I can do that update if you prefer after this patch is in.

I replaced all isTypeLegals with isTypeSynthesizable in
LegalizeVectorTypes.cpp to spare you the trouble.

> Looking at the changes, it overall looks good.  The code now separates the code that breaks the vector into synthesizable types with where it recombines it.  I think we can simplify the code during the recombine step
>     while (true) {
>       bool wrongTypeInConcatOps = false;
>       for (Idx = ConcatEnd - 1; Idx >= 0; Idx--) {
>         if (ConcatOps[Idx].getValueType() != MaxVT) {
>           wrongTypeInConcatOps = true;
>           break;
>         }
>       }
>       if(!wrongTypeInConcatOps)
>         break;
>       ...
>    }
> 
> When we split the vector into synthesizable types, we go form largest to smallest.  If the last piece is the list of operations we want to concatenate is MaxVT, I believe every other piece must be MaxVT so we don't need the loop. I think we can get rid of the for loop and we can move the condition into the while.  

A very good observation! I didn't like the clumsy for loop either, so
good riddance. I implemented your suggestion and it does work: with the
patch (against r105937) llc produces correct code from 
'fdiv <n x float> %a, %b' for x86-64 for vector lengths 1-32. The
updated patch is attached.

Two DejaGNU test cases are also included in the patch.

-- 
Visa Putkinen // vputkine at cs.hut.fi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vector-binop-widen.patch
Type: text/x-diff
Size: 10529 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100614/c415f97e/attachment.patch>