[llvm-commits] [llvm] r132689 - in /llvm/trunk: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp lib/CodeGen/SelectionDAG/LegalizeTypes.h lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp test/CodeGen/Generic/basic-promote-integers.ll
Duncan Sands
baldrick at free.fr
Thu Jun 9 05:01:38 PDT 2011
Hi Nadav,
> Add methods to support the integer-promotion of vector types. Methods to
> legalize SDNodes such as BUILD_VECTOR, EXTRACT_VECTOR_ELT, etc.
> --- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp Mon Jun 6 15:55:56 2011
> @@ -180,6 +191,10 @@
> if (NOutVT.bitsEq(NInVT))
> // The input promotes to the same size. Convert the promoted value.
> return DAG.getNode(ISD::BITCAST, dl, NOutVT, GetPromotedInteger(InOp));
> + if (NInVT.isVector())
> + // Promote vector element via memory load/store.
> + return DAG.getNode(ISD::ANY_EXTEND, dl, NOutVT,
> + CreateStackStoreLoad(InOp, OutVT));
What is this for? The break on the next line results in exactly this logic
being executed (see the line after the end of the switch).
> break;
> +SDValue DAGTypeLegalizer::PromoteIntRes_EXTRACT_SUBVECTOR(SDNode *N) {
> + SDValue InOp0 = N->getOperand(0);
> + EVT InVT = InOp0.getValueType();
> + EVT NInVT = TLI.getTypeToTransformTo(*DAG.getContext(), InVT);
> +
> + EVT OutVT = N->getValueType(0);
> + EVT NOutVT = TLI.getTypeToTransformTo(*DAG.getContext(), OutVT);
> + assert(NOutVT.isVector()&& "This type must be promoted to a vector type");
> + unsigned OutNumElems = N->getValueType(0).getVectorNumElements();
N->getValueType(0) -> OutVT
> + EVT NOutVTElem = NOutVT.getVectorElementType();
> +
> + DebugLoc dl = N->getDebugLoc();
> + SDValue BaseIdx = N->getOperand(1);
> +
> + SmallVector<SDValue, 8> Ops;
You know how many elements are going to be added, so you could reserve that much
space here.
> + for (unsigned i = 0; i != OutNumElems; ++i) {
> +
> + // Extract the element from the original vector.
> + SDValue Index = DAG.getNode(ISD::ADD, dl, BaseIdx.getValueType(),
> + BaseIdx, DAG.getIntPtrConstant(i));
If BaseIdx.getValueType() is not the same as IntPtr type than this will
crash. You should get "i" as a constant of type BaseIdx.getValueType().
> + SDValue Ext = DAG.getNode(ISD::EXTRACT_VECTOR_ELT, dl,
> + InVT.getVectorElementType(), N->getOperand(0), Index);
> +
> + SDValue Op = DAG.getNode(ISD::ANY_EXTEND, dl, NOutVTElem, Ext);
> + // Insert the converted element to the new vector.
> + Ops.push_back(Op);
> + }
> +
> + return DAG.getNode(ISD::BUILD_VECTOR, dl, NOutVT,&Ops[0], Ops.size());
> +}
Another possible approach is to enhance EXTRACT_SUBVECTOR to allow the extracted
subvector to have a wider element type than the original vector. I think this
is useful because the processor may support that. For example I think on x86
you can do <4 x i32> -> <2 x i64> (grab first two elements) as one processor
instruction.
> +SDValue DAGTypeLegalizer::PromoteIntRes_VECTOR_SHUFFLE(SDNode *N) {
> +
Pointless blank line.
> + ShuffleVectorSDNode *SV = cast<ShuffleVectorSDNode>(N);
> + EVT VT = N->getValueType(0);
> + DebugLoc dl = N->getDebugLoc();
> +
> + unsigned NumElts = VT.getVectorNumElements();
> + SmallVector<int, 8> NewMask;
> + for (unsigned i = 0; i != NumElts; ++i) {
> + NewMask.push_back(SV->getMaskElt(i));
> + }
The mask doesn't change, so is there any point to this? Can't you just reuse
the existing mask? If you can't access it (I don't recall how this works) then:
(1) you can reserve space in NewMask because you know how many elements are
going to be added; (2) no need for curly brackets in one-line "if".
> +
> + SDValue V0 = GetPromotedInteger(N->getOperand(0));
> + SDValue V1 = GetPromotedInteger(N->getOperand(1));
> + EVT OutVT = TLI.getTypeToTransformTo(*DAG.getContext(), VT);
There is no need to compute OutVT, since it is necessarily the type of V0 and
V1, so you can just use V0.getValueType().
> + return DAG.getVectorShuffle(OutVT, dl, V0,V1,&NewMask[0]);
Missing space after comma.
> +SDValue DAGTypeLegalizer::PromoteIntRes_BUILD_VECTOR(SDNode *N) {
> +
Pointless blank line.
> + SDValue InOp0 = N->getOperand(0);
> + EVT InVT = InOp0.getValueType();
> + EVT NInVT = TLI.getTypeToTransformTo(*DAG.getContext(), InVT);
> +
> + EVT OutVT = N->getValueType(0);
> + EVT NOutVT = TLI.getTypeToTransformTo(*DAG.getContext(), OutVT);
> + assert(NOutVT.isVector()&& "This type must be promoted to a vector type");
> + unsigned NumElems = N->getNumOperands();
> + EVT NOutVTElem = NOutVT.getVectorElementType();
> +
> + DebugLoc dl = N->getDebugLoc();
> +
> + SmallVector<SDValue, 8> Ops;
You can reserve space here.
> + for (unsigned i = 0; i != NumElems; ++i) {
> + SDValue Op = DAG.getNode(ISD::ANY_EXTEND, dl, NOutVTElem, N->getOperand(i));
> + Ops.push_back(Op);
> + }
> +
> + return DAG.getNode(ISD::BUILD_VECTOR, dl, NOutVT,&Ops[0], Ops.size());
Missing space after comma.
> +}
I'm wondering if this will always work correctly. Consider the case where the
new vector type is (say) <2 x i128>, on a machine where i128 is not legal. Can
this happen (maybe with AVX on x86?)? If so, probably BUILD_VECTOR needs to be
generalized so that the input type can be smaller than the output element type.
That said, I think your code is good enough for now.
> +SDValue DAGTypeLegalizer::PromoteIntRes_SCALAR_TO_VECTOR(SDNode *N) {
> +
Pointless blank line.
> + DebugLoc dl = N->getDebugLoc();
> +
> + SDValue InOp0 = N->getOperand(0);
> + EVT InVT = InOp0.getValueType();
> + EVT NInVT = TLI.getTypeToTransformTo(*DAG.getContext(), InVT);
> + assert(!InVT.isVector()&& "Input must not be a scalar");
must not be a scalar -> must not be a vector
> +
> + EVT OutVT = N->getValueType(0);
> + EVT NOutVT = TLI.getTypeToTransformTo(*DAG.getContext(), OutVT);
> + assert(NOutVT.isVector()&& "This type must be promoted to a vector type");
> + EVT NOutVTElem = NOutVT.getVectorElementType();
> +
> + SDValue Op = DAG.getNode(ISD::ANY_EXTEND, dl, NOutVTElem, N->getOperand(0));
> +
> + return DAG.getNode(ISD::SCALAR_TO_VECTOR, dl, NOutVT, Op);
> +}
This has the same potential problem as BUILD_VECTOR.
> +SDValue DAGTypeLegalizer::PromoteIntRes_INSERT_VECTOR_ELT(SDNode *N) {
> +
Pointless blank line.
> + SDValue InOp0 = N->getOperand(0);
> + EVT InVT = InOp0.getValueType();
> + EVT InElVT = InVT.getVectorElementType();
> + EVT NInVT = TLI.getTypeToTransformTo(*DAG.getContext(), InVT);
> +
> + EVT OutVT = N->getValueType(0);
> + EVT NOutVT = TLI.getTypeToTransformTo(*DAG.getContext(), OutVT);
> + assert(NOutVT.isVector()&& "This type must be promoted to a vector type");
> +
> + EVT NOutVTElem = NOutVT.getVectorElementType();
> +
> + DebugLoc dl = N->getDebugLoc();
> +
> + SDValue ConvertedVector = DAG.getNode(ISD::ANY_EXTEND, dl, NOutVT, InOp0);
Since the input vector is necessarily going to be promoted too, you should use
GetPromotedInteger here.
> + SDValue ConvElem = DAG.getNode(ISD::ANY_EXTEND, dl,
> + NOutVTElem, N->getOperand(1));
> + return DAG.getNode(ISD::INSERT_VECTOR_ELT, dl,NOutVT,
Missing space after comma.
> + ConvertedVector, ConvElem, N->getOperand(2));
> +}
This may have a similar problem to BUILD_VECTOR (vector element type bigger than
largest legal integer type).
> +SDValue DAGTypeLegalizer::PromoteIntOp_EXTRACT_VECTOR_ELT(SDNode *N) {
> + DebugLoc dl = N->getDebugLoc();
> + SDValue V0 = GetPromotedInteger(N->getOperand(0));
> + SDValue V1 = N->getOperand(1);
> + SDValue Ext = DAG.getNode(ISD::EXTRACT_VECTOR_ELT, dl,
> + V0->getValueType(0).getScalarType(), V0, V1);
> +
> + return DAG.getNode(ISD::TRUNCATE, dl, N->getValueType(0), Ext);
In theory, the return type could be larger than the element type of the new
vector (see the definition of EXTRACT_VECTOR_ELT), in which case this truncate
would assert.
> +
> +}
> +
> +SDValue DAGTypeLegalizer::PromoteIntOp_CONCAT_VECTORS(SDNode *N) {
> +
Pointless blank line.
> + DebugLoc dl = N->getDebugLoc();
> +
> + EVT RetSclrTy = N->getValueType(0).getVectorElementType();
> +
> + SmallVector<SDValue, 8> NewOps;
You could reserve space here.
> +
> + // For each incoming vector
> + for (unsigned VecIdx = 0, E = N->getNumOperands(); VecIdx!= E; ++VecIdx) {
Missing space after VecIdx.
> + SDValue Incoming = GetPromotedInteger(N->getOperand(VecIdx));
> + EVT SclrTy = Incoming->getValueType(0).getVectorElementType();
> + unsigned NumElem = Incoming->getValueType(0).getVectorNumElements();
> +
> + for (unsigned i=0; i<NumElem; ++i) {
> + // Extract element from incoming vector
> + SDValue Ex = DAG.getNode(ISD::EXTRACT_VECTOR_ELT, dl, SclrTy,
> + Incoming, DAG.getIntPtrConstant(i));
> + SDValue Tr = DAG.getNode(ISD::TRUNCATE, dl, RetSclrTy, Ex);
> + NewOps.push_back(Tr);
> + }
> + }
> +
> + return DAG.getNode(ISD::BUILD_VECTOR, dl, N->getValueType(0),
> +&NewOps[0], NewOps.size());
> + }
Rather than scalarizing, the definition of CONCAT_VECTORS could be enhanced to
allow the input vectors to have wider elements than the output vectors. This
would probably result in better code for, eg, concat <2 x i32>, <2 x i32> on
x86, since I think you can get pretty nice vector code on x86 that combines
and truncates two <2 x i64> into one <4 x i32>.
> --- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp Mon Jun 6 15:55:56 2011
> @@ -783,6 +783,17 @@
> DAG.getIntPtrConstant(InNVT.getVectorNumElements()));
> break;
> }
> + case TargetLowering::TypePromoteInteger: {
> + SDValue InOp = GetPromotedInteger(N->getOperand(0));
> + EVT InNVT = EVT::getVectorVT(*DAG.getContext(),
> + InOp.getValueType().getVectorElementType(),
> + LoVT.getVectorNumElements());
> + Lo = DAG.getNode(ISD::EXTRACT_SUBVECTOR, dl, InNVT, InOp,
> + DAG.getIntPtrConstant(0));
> + Hi = DAG.getNode(ISD::EXTRACT_SUBVECTOR, dl, InNVT, InOp,
> + DAG.getIntPtrConstant(InNVT.getVectorNumElements()));
> + break;
> + }
> case TargetLowering::TypeSplitVector:
> GetSplitVector(N->getOperand(0), Lo, Hi);
> break;
The entire switch here is kind of pointless. The code for the TypeLegal case
should always work (the legalizer will then need to legalize the created
EXTRACT_SUBVECTOR, but that's OK), so this is at best a small compile time
speedup, which I don't think it is worth all the extra code.
Ciao, Duncan.
More information about the llvm-commits
mailing list