[llvm-dev] Help understanding and lowering LLVM IDS conditional codes correctly

vivek pandya via llvm-dev llvm-dev at lists.llvm.org
Sun Mar 26 22:01:50 PDT 2017


Hello LLVM Developers,

Could you please provide review for following code snippet ? I would like
to understand if this is correct way to generate unordered comparison when
architecture natively does not support such comparison or not. Also I am
facing some assertion failure but not for all cases and not for all
optimization level that makes it herder to detect. I am facing :
clang-3.9:
/home/vpandya/llvm_old/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp:356:
void llvm::ScheduleDAGSDNodes::BuildSchedUnits(): Assertion `N->getNodeId()
== -1 && "Node already inserted!"' failed.
I have explained the code which caused this assertion but I am not able to
understand why it happens.

SDValue XXXTargetLowering::LowerSELECT_CC(SDValue Op,
                                             SelectionDAG &DAG) const {
  SDValue LHS    = Op.getOperand(0);
  SDValue RHS    = Op.getOperand(1);
  SDValue TrueV  = Op.getOperand(2);
  SDValue FalseV = Op.getOperand(3);
  ISD::CondCode CC = cast<CondCodeSDNode>(Op.getOperand(4))->get();
  SDLoc dl   (Op);

  SDValue TargetCC;
  MVT SVT = LHS.getSimpleValueType();
  SDValue Flag;
  const XXXSubtarget &STI = static_cast<const XXXSubtarget&>
                                                (DAG.getSubtarget());
  if (SVT == MVT::f32 && STI.hasFPU() ) {
    XXXCC::CondCodes TCC;
    bool isUnordered = false;
    switch (CC) {
      case ISD::SETUEQ:
        isUnordered = true;
        CC = ISD::SETEQ;
        break;
      case ISD::SETUNE:
        isUnordered = true;
        CC = ISD::SETNE;
        break;
      case ISD::SETUGT:
        isUnordered = true;
        CC = ISD::SETGT;
        break;
      case ISD::SETUGE:
        isUnordered = true;
        CC = ISD::SETGE;
        break;
      case ISD::SETULT:
        isUnordered = true;
        CC = ISD::SETLT;
        break;
      case ISD::SETULE:
        isUnordered = true;
        CC = ISD::SETLE;
        break;
    }
    if (CC == ISD::SETO)
      TCC = XXXCC::COND_UN;
    else
      getFPCCtoMBCC(CC,TCC); // just converts CC to Target specific code

      TargetCC = DAG.getConstant(TCC, dl, MVT::i32);
      // Here if I keeo MVT::Other I am facing above mentioned asserion
      // the reason for keeping this is I want to use this node as Chain
node
      // in getCopyFromReg() but when I remove this still it works
      // Is this correct use of Glue and Chain type?
      SDVTList VTList = DAG.getVTList(MVT::Glue,/* MVT::Other*/);
      Flag = DAG.getNode(XXXISD::FCMP, dl, VTList, LHS, RHS,
                         TargetCC);
      //TODO: if setCondCodeAction(unorderedCom , Exapnd) works properly
then we
      // do not have to do this manually
      if (isUnordered) {
      // read result of first FCMP from R18
      SDValue Reg1 = DAG.getCopyFromReg(Flag, dl, XXX::R18, MVT::i32,
                                        Flag);
      TCC = XXXCC::COND_UN;
      TargetCC = DAG.getConstant(TCC, dl, MVT::i32);
      SDValue UnComp = DAG.getNode(XXXISD::FCMP, dl,VTList, LHS, RHS,
                                   TargetCC);
      // read result of second FCMP from R18
      SDValue Reg2 = DAG.getCopyFromReg(UnComp, dl, XXX::R18, MVT::i32,
                                        UnComp);
      SDVTList VTList1 = DAG.getVTList(MVT::i32, MVT::Other);
      // OR results of both comparion if result is 1 then jump to
destination
      SDValue ORV = DAG.getNode(ISD::OR, dl, MVT::i32, Reg1, Reg2);
      SDVTList VTs = DAG.getVTList(MVT::Glue,MVT::Other);
      SDValue Ops[] = {ORV, DAG.getRegister(XXX::R18,
                                            ORV.getValueType()), ORV, ORV};

      Flag = DAG.getNode(ISD::CopyToReg, dl, VTs,
                    makeArrayRef(Ops, ORV.getNode() ? 4 : 3));
      }
      // fcmp instruction results 0 or 1 so next instruction will be bne{i}
      // that should branch to destination
      if (CC == ISD::SETO)
        TargetCC = DAG.getConstant(XXXCC::COND_E, dl, MVT::i32);
      else
        TargetCC = DAG.getConstant(XXXCC::COND_NE, dl, MVT::i32);

  }
  else {
  Flag = EmitCMP(LHS, RHS, TargetCC, CC, dl, DAG);
  }
  SDVTList VTs = DAG.getVTList(Op.getValueType(), MVT::Glue);
  SDValue Ops[] = {TrueV, FalseV, TargetCC, Flag};

  return DAG.getNode(XXXISD::SELECT_CC, dl, VTs, Ops);
}


Thanks,
Vivek

On Wed, Mar 15, 2017 at 12:13 AM, Hal Finkel <hfinkel at anl.gov> wrote:

>
> On 03/14/2017 01:16 PM, vivek pandya wrote:
>
>
>
> On Tue, Mar 14, 2017 at 9:59 PM, vivek pandya <vivekvpandya at gmail.com>
> wrote:
>
>>
>>
>> On Tue, Mar 14, 2017 at 7:19 PM, Hal Finkel <hfinkel at anl.gov> wrote:
>>
>>>
>>> On 03/14/2017 07:16 AM, vivek pandya wrote:
>>>
>>> Hello Hal,
>>> setCondCodeAction(expand) for un ordered comparison generates
>>> semantically wrong code for me for example SETUNE gets converted to SETOE
>>> that causes infinite loops.
>>>
>>>
>>> Can you please explain what is happening? It sounds like a bug we should
>>> fix.
>>>
>>> I don't think it is LLVM bug but I am missing some thing or I have not
>> implemented something related properly.
>> But I will experiment it with and let you my findings.
>>
>>>
>>> What is ideal place where I can convert unordered comparison to un
>>> comparison + OR + ordered comparison ?
>>> Can I do it by adding required SDNodes ?
>>> for example I am trying to do it in LowerBR_CC as shown below:
>>>       getFPCCtoMBCC(CC,TCC);
>>>       TargetCC = DAG.getConstant(TCC, dl, MVT::i8);
>>>       Flag = DAG.getNode(XXXISD::FCMP, dl, MVT::Glue, LHS, RHS,
>>>                          TargetCC);
>>>       if (isUnordered) {
>>>       TCC = XXX::COND_UN;
>>>       TargetCC = DAG.getConstant(TCC, dl, MVT::i8);
>>>       SDValue UnComp = DAG.getNode(XXX::FCMP, dl, MVT::Glue, LHS, RHS,
>>>                                    TargetCC);
>>>       Flag = DAG.getNode(ISD::OR, dl, MVT::Glue, Flag, UnComp);
>>>       }
>>> but here I can't OR 2 MVT::Glue value.
>>> How can I compare results of two fcmp SDValue objs?
>>>
>>>
>>> If your FCMP node sets some register, you'd need to read it
>>> (DAG.getCopyFromReg).
>>>
>> Hey Hal,
> I have few questions here,
> Do you here mean FCMP sets any physical register ?
>
>
> Yes.
>
> Because as per my understanding getCopyFromReg() requires a reg operand to
> copy from.
> What if it set some virtual register? Then how to use getCopyFromReg()
> method?
>
>
> You wouldn't. If your FCMP node sets a virtual register, then it should be
> one of the return values of the node (i.e. there should be a some value
> type (MVT::i8 or whatever) in the value-type list for the FCMP node).
>
>  -Hal
>
>
>
> getCopyFromReg() requires a Chain operand so I have to make FCMP both
> Chain and Glue (by using SDVTList
> <http://llvm.org/docs/doxygen/html/structllvm_1_1SDVTList.html> VTs =
> getVTList
> <http://llvm.org/docs/doxygen/html/classllvm_1_1SelectionDAG.html#a196c23d6cb4d768d037970f1f35bbf66>
> (MVT::Other
> <http://llvm.org/docs/doxygen/html/classllvm_1_1MVT.html#afd69b4f2dff97a2d7c0192cc769ef50ca62a222acce6360abd2726719fabc2797>,
> MVT::Glue
> <http://llvm.org/docs/doxygen/html/classllvm_1_1MVT.html#afd69b4f2dff97a2d7c0192cc769ef50ca59a1908cf136662bcfdc11ed49515ca9>
> )) right ?
>
> Sincerely,
> Vivek
>
>> Ok I will see some examples for getCopyFromReg().
>>
>> Thanks,
>> Vivek
>>
>>>
>>>
>>>  -Hal
>>>
>>>
>>>
>>> Please provide some guidance.
>>>
>>> Sincerely,
>>> Vivek
>>>
>>> On Thu, Mar 9, 2017 at 10:29 PM, vivek pandya <vivekvpandya at gmail.com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Thu, Mar 9, 2017 at 9:35 PM, Hal Finkel <hfinkel at anl.gov> wrote:
>>>>
>>>>>
>>>>> On 02/25/2017 03:06 AM, vivek pandya via llvm-dev wrote:
>>>>>
>>>>> Note: Question is written after describing what I have coded.
>>>>>
>>>>> Hello LLVMDevs,
>>>>>
>>>>> I am trying to impliment floating point comparsion for an architecture
>>>>> which
>>>>> supports following type of floating point comparision if FPU is
>>>>> available:
>>>>> fcmp.un --> true if one of the operand is NaN
>>>>> fcmp.lt --> ordered less than, if any input NaN then return false
>>>>> fcmp.eq --> ordered equal, if any input NaN then return false
>>>>> fcmp.le --> ordered less equal, if any input NaN then return false
>>>>> fcmp.gt --> ordered grater than, if any input NaN then return false
>>>>> fcmp.ne --> ordered not equal, if any input NaN then return true
>>>>> fcmp.ge --> ordered grater equal, if any input NaN then return false
>>>>>
>>>>> When FPU is not present I need to generate a library call,
>>>>>
>>>>> so I have added following code in LowerBR_CC function in
>>>>> XXXISelLowering.cpp
>>>>>
>>>>> const XXXSubtarget &STI = static_cast<const XXXSubtarget&>
>>>>>                                              (DAG.getSubtarget());
>>>>>  XXXCC::CondCodes TCC;
>>>>>  getFPCCtoXXCC(CC,TCC);
>>>>>  TargetCC = DAG.getConstant(TCC, dl, MVT::i8);
>>>>>  if (STI.useHardFloat()) {
>>>>>     // if fcmp instruction is available use it
>>>>>    SDValue Flag = DAG.getNode(XXXISD::FCMP, dl, MVT::Glue, LHS, RHS,
>>>>>                       TargetCC);
>>>>>    return DAG.getNode(XXXISD::BR_CC, dl, Op.getValueType(),
>>>>>                   Chain, Dest, TargetCC, Flag);
>>>>>  }
>>>>>  else {
>>>>>     // else generate library call
>>>>>    DAG.getTargetLoweringInfo().softenSetCCOperands(DAG, MVT::f32,
>>>>> LHS, RHS,
>>>>>                                                    CC, dl);
>>>>>
>>>>>    SDValue Flag = DAG.getNode(XXXISD::CMP, dl, MVT::Glue, LHS, RHS);
>>>>>
>>>>>    if (!RHS.getNode()) {
>>>>>      RHS = DAG.getConstant(0, dl, LHS.getValueType());
>>>>>      TargetCC = DAG.getConstant(XXXCC::COND_NE, dl, MVT::i8);
>>>>>    }
>>>>>    return DAG.getNode(XXXISD::BR_CC, dl, MVT::Other,
>>>>>                   Chain, Dest, TargetCC, Flag);
>>>>>  }
>>>>>
>>>>>  and code for getFPCCtoXXCC() is as following:
>>>>>
>>>>>  static void getFPCCtoXXCC(ISD::CondCode CC, XXXCC::CondCodes
>>>>> &CondCode) {
>>>>>   switch (CC) {
>>>>>     default:
>>>>>       llvm_unreachable("Unknown FP condition!");
>>>>>     case ISD::SETEQ:
>>>>>     case ISD::SETOEQ:
>>>>>       CondCode = XXXCC::COND_E;
>>>>>       break;
>>>>>     case ISD::SETGT:
>>>>>     case ISD::SETOGT:
>>>>>       CondCode = XXXCC::COND_GT;
>>>>>       break;
>>>>>     case ISD::SETGE:
>>>>>     case ISD::SETOGE:
>>>>>       CondCode = XXXCC::COND_GE;
>>>>>       break;
>>>>>     case ISD::SETOLT:
>>>>>     case ISD::SETLT:
>>>>>       CondCode = XXXCC::COND_LT;
>>>>>       break;
>>>>>     case ISD::SETOLE:
>>>>>     case ISD::SETLE:
>>>>>       CondCode = XXXCC::COND_LE;
>>>>>       break;
>>>>>     case ISD::SETONE:
>>>>>     case ISD::SETNE:
>>>>>       CondCode = XXXCC::COND_NE;
>>>>>       break;
>>>>>     case ISD::SETUO:
>>>>>       CondCode = XXXCC::COND_UN;
>>>>>       break;
>>>>>     case ISD::SETO:
>>>>>     case ISD::SETUEQ:
>>>>>     case ISD::SETUGT:
>>>>>     case ISD::SETUGE:
>>>>>     case ISD::SETULT:
>>>>>     case ISD::SETULE:
>>>>>     case ISD::SETUNE:
>>>>>       CC = getSetCCInverse(CC,false);
>>>>>       getFPCCtoMBCC(CC,CondCode);
>>>>>       break;
>>>>>   }
>>>>> }
>>>>>
>>>>>  I am generating wrong code when using floating point library call for
>>>>> comparions. For the following simple case:
>>>>> float branchTest(float a, float b) {
>>>>> float retVal;
>>>>> if (a == b) {
>>>>> retVal = a / b + 22.34;
>>>>> }
>>>>> return retVal;
>>>>> }
>>>>> I am getting:
>>>>> brlid r15,__nesf2
>>>>> nop
>>>>> beqi r3,.LBB0_2 ; r3 is return regsiter
>>>>>
>>>>> Now I want to understand difference between three different version of
>>>>> Condition
>>>>> Codes for same operation and how according to my target I should
>>>>> handle them.
>>>>> For example let's consider SETNE, SETONE and SETUNE so for my
>>>>> architecture
>>>>> I think for floating point all three are same
>>>>>
>>>>>
>>>>> No, they're not the same. Please see:
>>>>>
>>>>>   http://llvm.org/docs/LangRef.html#id290
>>>>>
>>>>> which explains the difference between SETONE (one) and SETUNE (une).
>>>>> Regarding how SETNE is interpreted for FP, see the comment in the
>>>>> definition of CondCode in include/llvm/CodeGen/ISDOpcodes.h which
>>>>> explains, "// Don't care operations: undefined if the input is a nan.".
>>>>>
>>>>> To support the unordered comparisons, if your FPU has only ordered
>>>>> comparisons, then you might need to do the underlying comparison, and a NaN
>>>>> check, and then OR the results. I think that using setCondCodeAction will
>>>>> cause the expansions for the hardware-unsupported variants to happen for
>>>>> you.
>>>>>
>>>>>  -Hal
>>>>>
>>>>
>>>> Thanks Hal for the guidance !
>>>>
>>>> -Vivek
>>>>
>>>>>
>>>>> so do I need to use
>>>>> getSetCCInverse() ? Also when I look at the code of
>>>>> TargetLowering::softenSetCCOperands I see that for some condition
>>>>> code it uses
>>>>> getSetCCInverse() and also I am not able to understand the way it
>>>>> groups
>>>>> condition code in switch case for example :
>>>>>   case ISD::SETEQ:
>>>>>   case ISD::SETOEQ:
>>>>>     LC1 = (VT == MVT::f32) ? RTLIB::OEQ_F32 :
>>>>>           (VT == MVT::f64) ? RTLIB::OEQ_F64 : RTLIB::OEQ_F128;
>>>>>     break;
>>>>>   case ISD::SETNE:
>>>>>   case ISD::SETUNE:
>>>>>     LC1 = (VT == MVT::f32) ? RTLIB::UNE_F32 :
>>>>>           (VT == MVT::f64) ? RTLIB::UNE_F64 : RTLIB::UNE_F128;
>>>>>     break;
>>>>> here why SETNE and SETUNE is considered same, why SETONE is considered
>>>>> differently. Is there any guideline to lower conditional code properly?
>>>>>
>>>>> Sincerely,
>>>>> Vivek
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> LLVM Developers mailing listllvm-dev at lists.llvm.orghttp://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>>
>>>>> --
>>>>> Hal Finkel
>>>>> Lead, Compiler Technology and Programming Languages
>>>>> Leadership Computing Facility
>>>>> Argonne National Laboratory
>>>>>
>>>>> --
>>> Hal Finkel
>>> Lead, Compiler Technology and Programming Languages
>>> Leadership Computing Facility
>>> Argonne National Laboratory
>>>
>>> --
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170327/e107b1d7/attachment.html>


More information about the llvm-dev mailing list