[llvm-commits] [llvm] r68996 - in /llvm/trunk: include/llvm/CodeGen/SelectionDAGNodes.h lib/CodeGen/SelectionDAG/DAGCombiner.cpp lib/CodeGen/SelectionDAG/LegalizeDAG.cpp lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp lib/CodeGen/SelectionDAG/SelectionDAG.cpp

[Bob Wilson]

On Apr 18, 2009, at 5:27 AM, Duncan Sands wrote:

> Hi Bob, thanks for doing this.
>
>>     /// BUILD_VECTOR(ELT0, ELT1, ELT2, ELT3,...) - Return a vector
>>     /// with the specified, possibly variable, elements.  The  
>> number of elements
>> -    /// is required to be a power of two.
>> +    /// is required to be a power of two.  The types of the  
>> operands must
>> +    /// all be the same.  They must match the vector element type,  
>> except if an
>> +    /// integer element type is not legal for the target, the  
>> operands may
>> +    /// be promoted to a legal type, in which case the operands  
>> are implicitly
>> +    /// truncated to the vector element types.
>
> I think it's best not to mention legal types here, and simply say  
> that if the
> component is of integer type then the elements are allowed to be of  
> a bigger
> integer type.

OK.

>
>> +  for (unsigned i = 0; i < NumElts; ++i) {
>> +    NewOps.push_back(GetPromotedInteger(N->getOperand(i)));
>>   }
>
> No need for braces {} around a one line loop body.

OK.

>
>> @@ -768,7 +768,8 @@
>>     // following checks at least makes it possible to legalize most  
>> of the time.
>> //    MVT EltVT = N->getValueType(0).getVectorElementType();
>> //    for (SDNode::op_iterator I = N->op_begin(), E = N->op_end();  
>> I != E; ++I)
>> -//      assert(I->getValueType() == EltVT &&
>> +//      assert((I->getValueType() == EltVT ||
>> +//              I->getValueType() ==  
>> TLI.getTypeToTransformTo(EltVT)) &&
>> //             "Wrong operand type!");
>
> Please uncomment the check, and change it to check the bitwidths in  
> the
> case of integer types, removing the getTypeToTransformTo check.

Are you sure that won't break any of the vector shuffles?  I'd be  
inclined to change it as you suggest but leave it commented out until  
the shuffles are changed to stop using BUILD_VECTORs for the mask  
operands.

>
>
>> +        assert(Elt.getValueType() == TLI.getTypeToTransformTo(VT) &&
>> +               "Bad type for BUILD_VECTOR operand");
>
> Please check bitwidths rather than getTypeToTransformTo.

OK, I'll fix those up soon.  Thanks for reviewing this.