[llvm-commits] [PATCH] Type-legalizer - Add flag to enable new legalization kind

[Duncan Sands]

Hi Nadav,

> Index: ../llvm/test/CodeGen/Generic/promote-integers.ll
> ===================================================================
> --- ../llvm/test/CodeGen/Generic/promote-integers.ll	(revision 0)
> +++ ../llvm/test/CodeGen/Generic/promote-integers.ll	(revision 0)
> @@ -0,0 +1,15 @@
> +; Test that vectors are scalarized/lowered correctly.
> +; RUN: llc -march=x86 -promote-elements < %s
> +
> +; This test is the poster-child for integer-element-promotion.
> +; Until this feature is complete, we mark this test as expected to fail.
> +; XFAIL: *
> +; CHECK: vector_code
> +; CHECK: ret

since you are not running FileCheck, these CHECK lines are useless.

> --- ../llvm/include/llvm/Target/TargetLowering.h	(revision 132345)
> +++ ../llvm/include/llvm/Target/TargetLowering.h	(working copy)
> @@ -35,6 +35,10 @@
>  #include <map>
>  #include <vector>
>
> +
> +
> +
> +

What's with the blank lines?

> +      // Try to promote the integer element types

Missing full stop.  Also, rather than just saying "try" please explain
when this works and what is done when it fails.

> +      while (1) {
> +        // Promote the element type
Missing full stop.  Please explain how it is promoted: round the number
of bits up to the next power of 2, or to 8 bits, whichever is larger.

> +        EltVT = EVT::getIntegerVT(Context, 1 + EltVT.getSizeInBits()
> +                                 ).getRoundIntegerType(Context);
> +
> +        // Stop trying when getting an illegal element type
> +        if (!EltVT.isSimple()) break;

Maybe you should just loop over the simple integer types...

> +        //

Empty comment.

> +        MVT NVT = MVT::getVectorVT(EltVT.getSimpleVT(), NumElts);
> +        if (ValueTypeActions.getTypeAction(NVT) == TypeLegal) {
> +          // Found a legal widended vector type

Missing full stop.  It's a promoted vector type, not a widened one.

> +          return LegalizeKind(TypePromoteInteger,
> +                              EVT::getVectorVT(Context, EltVT, NumElts));
> +        }

There was no need for these curly brackets.

> +      }
> +    }
> +
>      // Try to widen the vector until a legal type is found.
>      // If there is no wider legal type, split the vector.
>      while (1) {

> --- ../llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp	(revision 132345)
> +++ ../llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp	(working copy)
> @@ -814,6 +823,24 @@
>        bool IsLegalWiderType = false;
>        for (unsigned nVT = i+1; nVT <= MVT::LAST_VECTOR_VALUETYPE; ++nVT) {
>          EVT SVT = (MVT::SimpleValueType)nVT;
> +
> +        // If we allow the promotion of vector elements using a flag,
> +        // then return TypePromoteInteger on vector elements.
> +        if (mayPromoteElements) {
> +          // Promote vectors of integers to vectors with the same number
> +          // of elements, with a wider element type.
> +          if (SVT.getVectorElementType().getSizeInBits() > EltVT.getSizeInBits()
> +              && SVT.getVectorNumElements() == NElts &&
> +              isTypeLegal(SVT) && SVT.getScalarType().isInteger()) {
> +            TransformToType[i] = SVT;
> +            RegisterTypeForVT[i] = SVT;
> +            NumRegistersForVT[i] = 1;
> +            ValueTypeActions.setTypeAction(VT, TypePromoteInteger);
> +            IsLegalWiderType = true;
> +            break;
> +          }
> +        }
> +
>          if (SVT.getVectorElementType() == EltVT &&
>              SVT.getVectorNumElements() > NElts &&
>              isTypeLegal(SVT)) {

It would be great if all type computation logic could be unified...

> @@ -3290,3 +3317,6 @@
>               DAG.getConstant(magics.s-1, getShiftAmountTy(NPQ.getValueType())));
>    }
>  }
> +
> +
> +

What's with the blank lines?

Ciao, Duncan.