[llvm-commits] Buggy SelectionDAG binop vector widening

[Dale Johannesen]

On Jun 8, 2010, at 2:26 AM, Visa Putkinen wrote:
> On Thu, Jun 03, 2010 at 11:12:59AM -0700, Dale Johannesen wrote:
>> On Jun 3, 2010, at 6:27 AMPDT, Visa Putkinen wrote:
>>> The function responsible for widening vector binary operation  
>>> nodes in
>>> SelectionDAGs (DAGTypeLegalizer::WidenVecRes_Binary in
>>> lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp, r105388) returns
>>> incorrect results for many vector lengths and even triggers  
>>> asserts on
>>> lengths 9 and 13. I didn't find a bug report about this bug.
>>>
>>>
>>> +        NextVT = EVT::getVectorVT(*DAG.getContext(), WidenEltVT,
>>> NextSize);
>>> +      } while (!TLI.isTypeLegal(NextVT));
>>
>> This should be isTypeSynthesizable, not isTypeLegal.  There are  
>> probably
>> other places in here where that's also true.
>
> 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?

> Any other thoughts about the patch? The issue is still relevant in
> r105601: my patch passes http://vpu.me/v-binop-widen.ll and
> http://vpu.me/v-binop-widen2.ll , which fail without the patch. The
> patch doesn't seem to break any other DejaGNU tests.
>
> -- 
> Visa Putkinen // visa.putkinen at iki.fi
> <vector-binop-widen.patch>