[llvm-commits] Buggy SelectionDAG binop vector widening

Mon Ping Wang monping at apple.com
Wed Jun 9 23:25:20 PDT 2010


Hi Visa,

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.

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.  

-- Mon Ping




On Jun 9, 2010, at 9:31 AM, Mon Ping Wang wrote:

> I'll take a look as well.
> 
>  -- Mon Ping
> 
> On Jun 8, 2010, at 11:32 PM, Duncan Sands wrote:
> 
>> Hi Dale,
>> 
>>>> Thanks for the advice. I changed all three occurances of isTypeLegal to
>>>> isTypeSynthesizable in DAGTypeLegalizer::WidenVecRes_Binary. There are
>>>> still six other isTypeLegal call sites in LegalizeVectorTypes.cpp that
>>>> probably should be updated, but I didn't touch those for now. The
>>>> updated patch is attached.
>>> 
>>> It looks OK to me but I am not expert in this code. Duncan, you wrote
>>> this didn't you? Can you look?
>> 
>> Mon Ping wrote the vector widening stuff.  I will try to take a look too.
>> 
>> Ciao,
>> 
>> Duncan.
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100609/97650195/attachment.html>


More information about the llvm-commits mailing list