[llvm] r181926 - Fix legalization of SETCC with promoted integer intrinsics

[Hal Finkel]

----- Original Message -----
> Hi Hal,
> 
> On 15/05/13 23:37, Hal Finkel wrote:
> > Author: hfinkel
> > Date: Wed May 15 16:37:27 2013
> > New Revision: 181926
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=181926&view=rev
> > Log:
> > Fix legalization of SETCC with promoted integer intrinsics
> >
> > If the input operands to SETCC are promoted, we need to make sure
> > that we
> > either use the promoted form of both operands (or neither); a
> > mixture is not
> > allowed. This can happen, for example, if a target has a custom
> > promoted
> > i1-returning intrinsic (where i1 is not a legal type). In this
> > case, we need to
> > use the promoted form of both operands.
> >
> > This change only augments the behavior of the existing logic in the
> > case where
> > the input types (which may or may not have already been legalized)
> > disagree,
> 
> how can they disagree?  If you are seeing different operand types
> then this
> sounds like a bug somewhere else to me.

First, as a slight correction to the original commit message, this is covered only by test/CodeGen/PowerPC/bdzlr.ll.

Here's a more-complete explanation:

For target-specific intrinsics that return an illegal type (i1 in this case), they need to be promoted by DAGTypeLegalizer::CustomLowerNode. The results from the custom lowering procedure are directly used to replace the original node (ReplaceValueWith is called). This differs from the normal integer result promotion procedure, which use SetPromotedInteger and don't directly substitute the result into the DAG. In the normal procedure, this substitution does not actually take place unless GetPromotedInteger is called on the unpromoted node.

Also, PromoteIntRes_SETCC does not handle its operands as all of the other PromoteIntRes_* functions do. All of the other functions call GetPromotedInteger on their operands. If PromoteIntRes_SETCC also did this, then the above behavioural difference would not be significant. However, PromoteIntRes_SETCC seems happy to leave its operands in an illegal state (a special case which is used by several backends, including PowerPC). Unfortunately, if one of those operands was a custom promoted intrinsic, then the above logic forces the issue, and the promoted forms must be used.

So this could be fixed either by changing the custom promotion logic to use SetPromotedInteger, or by making PromoteIntRes_SETCC more robust. The latter seemed like the more localized change. I'd be happy to implement this the other way if you think that is better.

Thanks again,
Hal

> 
> Ciao, Duncan.
> 
> > and should not affect existing target code because this case would
> > otherwise
> > cause an assert in the SETCC operand promotion code.
> >
> > This will be covered by (essentially all of the) tests for the new
> > PPCCTRLoops
> > infrastructure.
> >
> > Modified:
> >      llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
> >
> > Modified:
> > llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp?rev=181926&r1=181925&r2=181926&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
> > (original)
> > +++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
> > Wed May 15 16:37:27 2013
> > @@ -521,9 +521,20 @@ SDValue DAGTypeLegalizer::PromoteIntRes_
> >     assert(SVT.isVector() ==
> >     N->getOperand(0).getValueType().isVector() &&
> >            "Vector compare must return a vector result!");
> >
> > +  SDValue LHS = N->getOperand(0);
> > +  SDValue RHS = N->getOperand(1);
> > +  if (LHS.getValueType() != RHS.getValueType()) {
> > +    if (getTypeAction(LHS.getValueType()) ==
> > TargetLowering::TypePromoteInteger &&
> > +        !LHS.getValueType().isVector())
> > +      LHS = GetPromotedInteger(LHS);
> > +    if (getTypeAction(RHS.getValueType()) ==
> > TargetLowering::TypePromoteInteger &&
> > +        !RHS.getValueType().isVector())
> > +      RHS = GetPromotedInteger(RHS);
> > +  }
> > +
> >     // Get the SETCC result using the canonical SETCC type.
> > -  SDValue SetCC = DAG.getNode(N->getOpcode(), dl, SVT,
> > N->getOperand(0),
> > -                              N->getOperand(1), N->getOperand(2));
> > +  SDValue SetCC = DAG.getNode(N->getOpcode(), dl, SVT, LHS, RHS,
> > +                              N->getOperand(2));
> >
> >     assert(NVT.bitsLE(SVT) && "Integer type overpromoted?");
> >     // Convert to the expected type.
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>