[llvm-commits] Patch: LegalizeType for promoting some operands

[Duncan Sands]

Hi Mon Ping,

> -  return DAG.getNode(ISD::SELECT, NewVT, Odd, Hi, Lo);
> +  SDValue Res = DAG.getNode(ISD::SELECT, NewVT, Odd, Hi, Lo);
> +  MVT NVT = TLI.getTypeToTransformTo(OldVT);
> +  if (Res.getValueType() != NVT) {
> +    assert(Res.getValueType().getSizeInBits() < NVT.getSizeInBits() &&
> +           "Unexpected Result Type");
> +    Res = DAG.getNode(ISD::ANY_EXTEND, NVT, Res);
> +  }

you might as well not bother testing Res.getValueType() != NVT, and
always do the extension.  If the types are equal then SelectionDAG
will fold the node and simply return Res.  The assertion is also
useless: this is checked by the getNode call.

> +  MVT OldEVT = Val.getValueType();

Please add an assertion that this is the same as the vector element
type.

> +  // BitConvert a vector of twice the length out of the expanded elements,

-> Bitconvert to a vector of twice the length with elements of the expanded type,

> +  SDValue Idx = N->getOperand(2);
> +  if (ConstantSDNode *CIdx = dyn_cast<ConstantSDNode>(Idx)) {
> +    unsigned IdxVal = CIdx->getZExtValue()*2;
> +    NewVec = DAG.getNode(ISD::INSERT_VECTOR_ELT, NewVecVT, NewVec, Lo,
> +                         DAG.getIntPtrConstant(IdxVal));
> +    NewVec =  DAG.getNode(ISD::INSERT_VECTOR_ELT, NewVecVT, NewVec, Hi,
> +                          DAG.getIntPtrConstant(IdxVal + 1));
> +  } else {
> +    Idx = DAG.getNode(ISD::SHL, Idx.getValueType(), Idx,
> +                      DAG.getIntPtrConstant(1));
> +    NewVec = DAG.getNode(ISD::INSERT_VECTOR_ELT, NewVecVT, NewVec, Lo,
> Idx); +    Idx = DAG.getNode(ISD::ADD, Idx.getValueType(), Idx,
> +                      DAG.getIntPtrConstant(1));
> +    NewVec =  DAG.getNode(ISD::INSERT_VECTOR_ELT, NewVecVT, NewVec, Hi,
> Idx); +  }

I think you should drop the special version for constants, and keep
the second version.  Also, in the second version it seems better to
calculate 2*Idx by adding Idx to itself, rather than shifting left
by 1.

> +  for (unsigned i=1; i < NumElts; ++i)

Missing spaces around the equals sign.

Thanks for doing this.

Best wishes,

Duncan.