[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

[Duncan Sands]

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.

> +  // Check if the BUILD_VECTOR operands were promoted to legalize their types.
> +  if (OpVT != EltVT) {
> +    // Now that the DAG combiner and target-specific lowering have had a
> +    // chance to optimize/recognize the BUILD_VECTOR with promoted operands,
> +    // transform it so the operand types match the vector.  Build a vector of
> +    // half the length out of elements of twice the bitwidth.
> +    // For example <4 x i16> -> <2 x i32>.
> +    MVT NewVT = MVT::getIntegerVT(2 * EltVT.getSizeInBits());

What if NewVT is not a legal type?  In theory there could be a platform on
which 4 x i16 is legal, i16 and i32 are illegal and i64 is legal (so operands
would have type i64).  Here NewVT would be i32, which is illegal.  It is safe
to assume that the bitsize of OpVT is a power-of-two multiple of the EltVT
bitsize, so a possible fix is to do the "half the length, double the bitwidth"
transform in a loop until NewVT reaches OpVT.  This is what would happen in
LegalizeTypes by the way, though it is not explicit (the generic infrastructure
keeps on calling the legalization routines until all types are legal, generating
an implicit loop here).

> +    // Recurse
> +    NewVec = ExpandBUILD_VECTOR(NewVec.getNode());

OK, maybe you handle this already :)

> +  for (unsigned i = 0; i < NumElts; ++i) {
> +    NewOps.push_back(GetPromotedInteger(N->getOperand(i)));
>    }

No need for braces {} around a one line loop body.

> @@ -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.

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

Please check bitwidths rather than getTypeToTransformTo.

Ciao,

Duncan.