[llvm-commits] [llvm] r59001 - in /llvm/trunk: include/llvm/CodeGen/SelectionDAG.h include/llvm/CodeGen/SelectionDAGNodes.h include/llvm/Intrinsics.td lib/CodeGen/SelectionDAG/LegalizeDAG.cpp lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp lib/CodeGen/SelectionDAG/LegalizeTypes.h lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp lib/CodeGen/SelectionDAG/SelectionDAG.cpp lib/CodeGen/SelectionDAG/SelectionDAGBuild.cpp lib/Target/TargetSelectionDAG.td

[Duncan Sands]

Hi,

> +    //   1) dest type (type to convert to)

are you really storing this in the node?  That's not needed because
it is the type of the node result.

> +    //   2) src type (type to convert from)

Likewise for this.  This must be the type of "0) value".

> +    //   3) rounding imm

What is this precisely?

> +    //   4) saturation imm

What is this precisely?

> +SDValue DAGTypeLegalizer::PromoteIntOp_CONVERT_RNDSAT(SDNode *N) {
> +  MVT OutVT = TLI.getTypeToTransformTo(N->getValueType(0));

Why is this OutVT?  It is the new type for the input, so if anything
it should be InVT.  More traditional is NVT.

> +  SDValue In = DAG.getConvertRndSat(OutVT,N->getOperand(0),

You said the type of the input changed (OutVT) but did change the input
(still using N->getOperand(0)).  You should use: GetPromotedInteger(N->getOperand(0))

Also, what if it's operand 3 or 4 that needs promoting?

> +SDValue DAGTypeLegalizer::ScalarizeVecRes_CONVERT_RNDSAT(SDNode *N) {

The documentation forgot to mention that CONVERT_RNDSAT can be applied to
vectors.

> +  return DAG.getConvertRndSat(NewVT, Op0, DAG.getValueType(NewVT),
> +                              DAG.getValueType(Op0.getValueType()),

^ Here you see that several arguments to getConvertRndSat are
(apparently) redundant.

> +  GetSplitDestVTs(N->getValueType(0), LoVT, HiVT);
> +  SDValue VLo, VHi;
> +  GetSplitVector(N->getOperand(0), VLo, VHi);

There's no need to call GetSplitDestVTs: LoVT is the type of
VLo, HiVT is the type of VHi.

> +  SDValue DTyOpLo =  DAG.getValueType(LoVT);
> +  SDValue DTyOpHi =  DAG.getValueType(HiVT);
> +  SDValue STyOpLo =  DAG.getValueType(VLo.getValueType());
> +  SDValue STyOpHi =  DAG.getValueType(VHi.getValueType());

^ None of these would be needed if getConvertRndSat didn't take
redundant arguments.

Ciao,

Duncan.