[llvm-commits] [llvm] r132649 - in /llvm/trunk: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp test/CodeGen/Generic/basic-promote-integers.ll

[Duncan Sands]

Hi Nadav,

> TypeLegalizer: Add support for passing of vector-promoted types in registers (copyFromParts/copyToParts).

I see several crashes on the attached testcase.

> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp Sat Jun  4 15:58:08 2011
> @@ -280,7 +280,28 @@
>       }
>
>       // Vector/Vector bitcast.
> -    return DAG.getNode(ISD::BITCAST, DL, ValueVT, Val);
> +    if (ValueVT.getSizeInBits() == PartVT.getSizeInBits())
> +      return DAG.getNode(ISD::BITCAST, DL, ValueVT, Val);
> +
> +    assert(PartVT.getVectorNumElements() == ValueVT.getVectorNumElements()&&
> +      "Cannot handle this kind of promotion");
> +    // Promoted vector extract
> +    unsigned NumElts = ValueVT.getVectorNumElements();
> +    SmallVector<SDValue, 8>  NewOps;
> +    for (unsigned i = 0; i<  NumElts; ++i) {
> +      SDValue Ext = DAG.getNode(ISD::EXTRACT_VECTOR_ELT, DL,
> +        PartVT.getScalarType(), Val ,DAG.getIntPtrConstant(i));
> +      SDValue Cast;
> +
> +      bool Smaller = ValueVT.bitsLE(PartVT);
> +
> +      Cast = DAG.getNode((Smaller ? ISD::TRUNCATE : ISD::ANY_EXTEND),
> +                         DL, ValueVT.getScalarType(), Ext);
> +
> +      NewOps.push_back(Cast);
> +    }
> +    return DAG.getNode(ISD::BUILD_VECTOR, DL, ValueVT,
> +&NewOps[0], NewOps.size());
>     }

Instead of scalarizing, and doing an element-by-element truncate, can't you just
do a vector truncate (which should result in better code)?

> @@ -452,7 +473,24 @@
>
>         //SDValue UndefElts = DAG.getUNDEF(VectorTy);
>         //Val = DAG.getNode(ISD::CONCAT_VECTORS, DL, PartVT, Val, UndefElts);
> -    } else {
> +    } else if (PartVT.isVector()&&
> +               PartVT.getVectorElementType().bitsGE(
> +                 ValueVT.getVectorElementType())&&

Missing space before &&

> +               PartVT.getVectorNumElements() == ValueVT.getVectorNumElements()) {

The code preceding this "if" is also for vectors.  How about having an
   if (PartVT.isVector()) {
     // all the vector code goes here
   }
and concentrate all the vector code inside it, set up the same way as in
getCopyFromParts.  Making the getCopyFromParts and getCopyToPartsVector code
more similar makes them easier to compare.

> +
> +      // Promoted vector extract
> +      unsigned NumElts = ValueVT.getVectorNumElements();
> +      SmallVector<SDValue, 8>  NewOps;
> +      for (unsigned i = 0; i<  NumElts; ++i) {
> +        SDValue Ext = DAG.getNode(ISD::EXTRACT_VECTOR_ELT, DL,
> +                       ValueVT.getScalarType(), Val ,DAG.getIntPtrConstant(i));
> +        SDValue Cast = DAG.getNode(ISD::ANY_EXTEND,
> +                       DL, PartVT.getScalarType(), Ext);
> +        NewOps.push_back(Cast);
> +      }
> +      Val = DAG.getNode(ISD::BUILD_VECTOR, DL, PartVT,
> +&NewOps[0], NewOps.size());
> +    } else{

This could also be done with a vector any_extend, rather than scalarizing.
Also, I don't know if a vector parameter can have the "zext" or "sext"
attribute, but if they can then you should do the appropriate kind of
extension here.

Ciao, Duncan.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: vparam.ll
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110609/6d604392/attachment.ksh>