[llvm-commits] [llvm] r138877 - in /llvm/trunk/lib/CodeGen/SelectionDAG: LegalizeFloatTypes.cpp LegalizeIntegerTypes.cpp LegalizeTypes.cpp LegalizeTypes.h LegalizeTypesGeneric.cpp LegalizeVectorTypes.cpp

[Duncan Sands]

Hi Eli,

> +SDValue DAGTypeLegalizer::SoftenFloatRes_MERGE_VALUES(SDNode *N) {
> +  SDValue Op = DecomposeMERGE_VALUES(N);
> +  return Op.getValueType().isVector() ?
> +      BitConvertVectorToIntegerVector(Op) :
> +      BitConvertToInteger(Op);

how can you get a vector here?

> +SDValue DAGTypeLegalizer::PromoteIntRes_MERGE_VALUES(SDNode *N) {
> +  SDValue Op = DecomposeMERGE_VALUES(N);
> +  assert(Op.getValueType().isInteger()
> +&&  "Must decompose to an integer type!");

Here this might be a vector of integers, so I think this assertion is wrong.
(For the moment you need to pass the -promote-elements flag to llc to have
vectors turn up in the integer promotion code).

> +  return GetPromotedInteger(Op);
> +}
> +
>   SDValue DAGTypeLegalizer::PromoteIntRes_AssertSext(SDNode *N) {
>     // Sign-extend the new bits, and continue the assertion.
>     SDValue Op = SExtPromotedInteger(N->getOperand(0));
> @@ -1548,6 +1556,13 @@
>     // use the new one.
>     ReplaceValueWith(SDValue(N, 1), Hi.getValue(1));
>   }

Missing blank line.

> --- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp Wed Aug 31 13:36:04 2011
> @@ -946,6 +946,25 @@
>     return true;
>   }
>
> +SDValue DAGTypeLegalizer::DecomposeMERGE_VALUES(SDNode *N) {

I know the comment describing DecomposeMERGE_VALUES is in the header, but it
would be nice to have to here too.

> +  unsigned i;
> +   // A MERGE_VALUES node can produce any number of values.
> +   // We know that the first illegal type needs to be handled.

Perhaps it should be clarified that this is the case because the type legalizer
always legalizes results in order.

> +  for (i = 0; isTypeLegal(N->getValueType(i)); ++i)
> +    ReplaceValueWith(SDValue(N, i), SDValue(N->getOperand(i)));
> +
> +  // The first illegal result must be the one that needs to be handled.

This comment repeats the previous comment.

> +  SDValue BadValue = N->getOperand(i);

It's not bad, it's illegal :)  How about IllegalValue instead.
> +
> +  // Legalize the rest of the results into the input operands whether they

This isn't legalizing, so how about: Map the rest of the results to the input
operands whether they

> +  // are legal or not.
> +  unsigned e = N->getNumValues();
> +  for (++i; i != e; ++i)
> +    ReplaceValueWith(SDValue(N, i), SDValue(N->getOperand(i)));
> +
> +  return BadValue;
> +}
> +
>   /// GetSplitDestVTs - Compute the VTs needed for the low/hi parts of a type
>   /// which is split into two not necessarily identical pieces.
>   void DAGTypeLegalizer::GetSplitDestVTs(EVT InVT, EVT&LoVT, EVT&HiVT) {
>

> --- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.h (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.h Wed Aug 31 13:36:04 2011
> @@ -148,12 +148,20 @@
>     SDValue CreateStackStoreLoad(SDValue Op, EVT DestVT);
>     bool CustomLowerNode(SDNode *N, EVT VT, bool LegalizeResult);
>     bool CustomWidenLowerNode(SDNode *N, EVT VT);
> +
> +  // DecomposeMERGE_VALUES  takes a SDNode and returns the first
> +  // illegal operand that needs to be modified.

"that needs to be modified" is redundant.  There is no such thing as
an illegal operand that doesn't need to be modified.

> +  // All other nodes are legalized, whether they are legal or not.

I find this line confusing.  They aren't being legalized.

> +  // The resulting SDValue needs to be modified to make it legal.
> +  SDValue DecomposeMERGE_VALUES(SDNode *N);
> +
>     SDValue GetVectorElementPointer(SDValue VecPtr, EVT EltVT, SDValue Index);
>     SDValue JoinIntegers(SDValue Lo, SDValue Hi);
>     SDValue LibCallify(RTLIB::Libcall LC, SDNode *N, bool isSigned);
>     SDValue MakeLibCall(RTLIB::Libcall LC, EVT RetVT,
>                         const SDValue *Ops, unsigned NumOps, bool isSigned,
>                         DebugLoc dl);

I didn't check the rest.

Ciao, Duncan.