[llvm] r259387 - AArch64: Implement missed conditional compare sequences.

Balaram Makam via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 8 14:10:48 PDT 2016


Hi Quentin,

 

I don’t see an easy way to achieve the same combine in PerformDAGCombine because it depends on the utilities emitConditionalComparison and isConjunctionDisjunctionTree that will now fail to match the AArch64 specific DAG nodes. Is there any other way we could do the custom lowering with just the isLegal part of the check?

 

Regards,

Balaram

 

From: Quentin Colombet [mailto:qcolombet at apple.com] 
Sent: Monday, June 06, 2016 7:34 PM
To: Balaram Makam <bmakam at codeaurora.org>
Cc: llvm-commits at lists.llvm.org
Subject: Re: [llvm] r259387 - AArch64: Implement missed conditional compare sequences.

 

Hi Balaram,

 

On Jun 6, 2016, at 2:43 PM, Balaram Makam <bmakam at codeaurora.org <mailto:bmakam at codeaurora.org> > wrote:

 

Hi Quentin,

 

I will take another look at this. I believe the problem is seen only on the internal branch. Do you have a test case to reproduce the issue?

 

Unfortunately, no, I do not have a test case for you.

Basically, this patch generates illegal code after legalization, and the problem is only the fact that we use Custom here to check legality.

 

You should be able to achieve the same combine in the AArch64 backend with just the isLegal part of the check.

There are a bunch of hooks during the lowering of SDAG and I am pretty sure you could find another way to do your custom lowering (I know I’ve already been in that situation :)). The "perform dag combine” that I mentioned should be one of them, but you have others.

 

Cheers,

-Quentin





 

Thanks,

Balaram

 

From: Quentin Colombet [mailto:qcolombet at apple.com] 
Sent: Monday, June 06, 2016 5:30 PM
To: Balaram Makam <bmakam at codeaurora.org <mailto:bmakam at codeaurora.org> >
Cc: llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org> 
Subject: Re: [llvm] r259387 - AArch64: Implement missed conditional compare sequences.

 

Hi Balaram,

 

Sorry for the late reply, but I am seeing this commit is causing problem internally.

 

On Feb 1, 2016, at 11:13 AM, Balaram Makam via llvm-commits < <mailto:llvm-commits at lists.llvm.org> llvm-commits at lists.llvm.org> wrote:

 

Author: bmakam
Date: Mon Feb  1 13:13:07 2016
New Revision: 259387

URL:  <http://llvm.org/viewvc/llvm-project?rev=259387&view=rev> http://llvm.org/viewvc/llvm-project?rev=259387&view=rev
Log:
AArch64: Implement missed conditional compare sequences.

Summary:
This is an extension to the existing implementation of r242436 which
restricts to only select inputs. This version fixes missed opportunities
in pr26084 by attempting to lower conditional compare sequences of
and/or trees with setcc leafs. This will additionaly handle the case
when a tree with select input is not a conjunction-disjunction tree
but some of the sub trees are conjunction-disjunction trees.

Reviewers: jmolloy, t.p.northover, mcrosier, MatzeB

Subscribers: mcrosier, llvm-commits, junbuml, haicheng, mssimpso, gberry

Differential Revision:  <http://reviews.llvm.org/D16291> http://reviews.llvm.org/D16291

Modified:
   llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
   llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp
   llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h
   llvm/trunk/test/CodeGen/AArch64/arm64-ccmp.ll

Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
URL:  <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=259387&r1=259386&r2=259387&view=diff> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=259387&r1=259386&r2=259387&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Mon Feb  1 13:13:07 2016
@@ -1721,7 +1721,7 @@ SDValue DAGCombiner::visitADD(SDNode *N)
    return SDValue(N, 0);

  // fold (a+b) -> (a|b) iff a and b share no bits.
-  if ((!LegalOperations || TLI.isOperationLegal(ISD::OR, VT)) &&
+  if ((!LegalOperations || TLI.isOperationLegalOrCustom(ISD::OR, VT)) &&

 

This is wrong. Custom does not imply that the operation is Legal!

 

You said in the review that without custom, one of the lit test case was failing. I believe you can fix that by making the Custom lowering as part of the combine process (PerformDAGCombine.)

 

Anyhow, this line is generally not correct.

 

Cheers,

-Quentin

 






      VT.isInteger() && !VT.isVector() && DAG.haveNoCommonBitsSet(N0, N1))
    return DAG.getNode(ISD::OR, SDLoc(N), VT, N0, N1);

@@ -6363,7 +6363,7 @@ SDValue DAGCombiner::visitZERO_EXTEND(SD
      isa<LoadSDNode>(N0.getOperand(0)) &&
      N0.getOperand(1).getOpcode() == ISD::Constant &&
      TLI.isLoadExtLegal(ISD::ZEXTLOAD, VT, N0.getValueType()) &&
-      (!LegalOperations && TLI.isOperationLegal(N0.getOpcode(), VT))) {
+      (!LegalOperations && TLI.isOperationLegalOrCustom(N0.getOpcode(), VT))) {
    LoadSDNode *LN0 = cast<LoadSDNode>(N0.getOperand(0));
    if (LN0->getExtensionType() != ISD::SEXTLOAD && LN0->isUnindexed()) {
      bool DoXform = true;

Modified: llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp
URL:  <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp?rev=259387&r1=259386&r2=259387&view=diff> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp?rev=259387&r1=259386&r2=259387&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp Mon Feb  1 13:13:07 2016
@@ -144,6 +144,16 @@ AArch64TargetLowering::AArch64TargetLowe
  setOperationAction(ISD::XOR, MVT::i32, Custom);
  setOperationAction(ISD::XOR, MVT::i64, Custom);

+  // Custom lowering hooks are needed for OR
+  // to fold it into CCMP.
+  setOperationAction(ISD::OR, MVT::i32, Custom);
+  setOperationAction(ISD::OR, MVT::i64, Custom);
+
+  // Custom lowering hooks are needed for AND
+  // to fold it into CCMP.
+  setOperationAction(ISD::AND, MVT::i32, Custom);
+  setOperationAction(ISD::AND, MVT::i64, Custom);
+
  // Virtually no operation on f128 is legal, but LLVM can't expand them when
  // there's a valid register class, so we need custom operations in most cases.
  setOperationAction(ISD::FABS, MVT::f128, Expand);
@@ -1597,6 +1607,27 @@ static SDValue getAArch64Cmp(SDValue LHS
  return Cmp;
}

+// Attempt to form conditional compare sequences for and/or trees
+// with setcc leafs.
+static SDValue tryLowerToAArch64Cmp(SDValue Op, SelectionDAG &DAG) {
+  SDValue LHS = Op.getOperand(0);
+  SDValue RHS = Op.getOperand(1);
+  if ((LHS.getOpcode() != ISD::SETCC) || (RHS.getOpcode() != ISD::SETCC))
+    return Op;
+
+  bool CanNegate;
+  if (!isConjunctionDisjunctionTree(Op, CanNegate))
+    return SDValue();
+
+  EVT VT = Op.getValueType();
+  SDLoc DL(Op);
+  SDValue TVal = DAG.getConstant(1, DL, VT);
+  SDValue FVal = DAG.getConstant(0, DL, VT);
+  SDValue CCVal;
+  SDValue Cmp = getAArch64Cmp(Op, FVal, ISD::SETEQ, CCVal, DAG, DL);
+  return DAG.getNode(AArch64ISD::CSEL, DL, VT, FVal, TVal, CCVal, Cmp);
+}
+
static std::pair<SDValue, SDValue>
getAArch64XALUOOp(AArch64CC::CondCode &CC, SDValue Op, SelectionDAG &DAG) {
  assert((Op.getValueType() == MVT::i32 || Op.getValueType() == MVT::i64) &&
@@ -1718,6 +1749,18 @@ SDValue AArch64TargetLowering::LowerF128
  return makeLibCall(DAG, Call, MVT::f128, Ops, false, SDLoc(Op)).first;
}

+SDValue AArch64TargetLowering::LowerAND(SDValue Op, SelectionDAG &DAG) const {
+  if (Op.getValueType().isVector())
+    return LowerVectorAND(Op, DAG);
+  return tryLowerToAArch64Cmp(Op, DAG);
+}
+
+SDValue AArch64TargetLowering::LowerOR(SDValue Op, SelectionDAG &DAG) const {
+  if (Op.getValueType().isVector())
+    return LowerVectorOR(Op, DAG);
+  return tryLowerToAArch64Cmp(Op, DAG);
+}
+
static SDValue LowerXOR(SDValue Op, SelectionDAG &DAG) {
  SDValue Sel = Op.getOperand(0);
  SDValue Other = Op.getOperand(1);
@@ -2372,9 +2415,9 @@ SDValue AArch64TargetLowering::LowerOper
  case ISD::FCOPYSIGN:
    return LowerFCOPYSIGN(Op, DAG);
  case ISD::AND:
-    return LowerVectorAND(Op, DAG);
+    return LowerAND(Op, DAG);
  case ISD::OR:
-    return LowerVectorOR(Op, DAG);
+    return LowerOR(Op, DAG);
  case ISD::XOR:
    return LowerXOR(Op, DAG);
  case ISD::PREFETCH:

Modified: llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h
URL:  <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h?rev=259387&r1=259386&r2=259387&view=diff> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h?rev=259387&r1=259386&r2=259387&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h Mon Feb  1 13:13:07 2016
@@ -488,6 +488,8 @@ private:
  SDValue LowerCTPOP(SDValue Op, SelectionDAG &DAG) const;
  SDValue LowerF128Call(SDValue Op, SelectionDAG &DAG,
                        RTLIB::Libcall Call) const;
+  SDValue LowerAND(SDValue Op, SelectionDAG &DAG) const;
+  SDValue LowerOR(SDValue Op, SelectionDAG &DAG) const;
  SDValue LowerFCOPYSIGN(SDValue Op, SelectionDAG &DAG) const;
  SDValue LowerFP_EXTEND(SDValue Op, SelectionDAG &DAG) const;
  SDValue LowerFP_ROUND(SDValue Op, SelectionDAG &DAG) const;

Modified: llvm/trunk/test/CodeGen/AArch64/arm64-ccmp.ll
URL:  <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/arm64-ccmp.ll?rev=259387&r1=259386&r2=259387&view=diff> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/arm64-ccmp.ll?rev=259387&r1=259386&r2=259387&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/arm64-ccmp.ll (original)
+++ llvm/trunk/test/CodeGen/AArch64/arm64-ccmp.ll Mon Feb  1 13:13:07 2016
@@ -371,21 +371,76 @@ define i32 @select_andor(i32 %v1, i32 %v
  ret i32 %sel
}

-; CHECK-LABEL: select_noccmp1
-define i64 @select_noccmp1(i64 %v1, i64 %v2, i64 %v3, i64 %r) {
-; CHECK: cmp x0, #0
-; CHECK-NEXT: cset [[REG0:w[0-9]+]], lt
-; CHECK-NEXT: cmp x0, #13
-; CHECK-NOT: ccmp
+; CHECK-LABEL: single_noselect
+define i32 @single_noselect(i32 %A, i32 %B) #0 {
+; CHECK: cmp w1, #1
+; CHECK-NEXT: ccmp w0, #1, #8, ge
+; CHECK-NEXT: cset w0, lt
+; CHECK-NEXT: ret
+  %notlhs = icmp slt i32 %A, 1
+  %notrhs = icmp slt i32 %B, 1
+  %lnot = or i1 %notlhs, %notrhs
+  %conv = zext i1 %lnot to i32
+  ret i32 %conv
+}
+
+; CHECK-LABEL: single_and_ext
+define i32 @single_and_ext(i32 %A, i32 %B, i32 %C) #0 {
+; CHECK: cmp w1, #2
+; CHECK-NEXT: ccmp w0, #4, #0, lt
+; CHECK-NEXT: cinc w0, w2, lt
+; CHECK-NEXT: ret
+  %cmp = icmp slt i32 %A, 4
+  %cmp1 = icmp slt i32 %B, 2
+  %and1 = and i1 %cmp, %cmp1
+  %conv = zext i1 %and1 to i32
+  %add = add nsw i32 %conv, %C
+  ret i32 %add
+}
+
+; CHECK-LABEL: single_noselect_phi
+define i32 @single_noselect_phi(i32 %A, i32 %B, i32 %C) #0 {
+; CHECK: cmp w1, #0
+; CHECK-NEXT: ccmp w0, #0, #4, gt
; CHECK-NEXT: cset [[REG1:w[0-9]+]], gt
-; CHECK-NEXT: cmp x2, #2
+; CHECK-NEXT: cmp w1, #2
+; CHECK-NEXT: ccmp w0, #4, #8, ge
; CHECK-NEXT: cset [[REG2:w[0-9]+]], lt
+; CHECK-NEXT: cmp w2, #0
+; CHECK-NEXT: csel w0, [[REG1]], [[REG2]], eq
+; CHECK-NEXT: ret
+entry:
+  %tobool = icmp eq i32 %C, 0
+  br i1 %tobool, label %if.else, label %if.then
+
+if.then:                                          ; preds = %entry
+  %cmp = icmp slt i32 %A, 4
+  %cmp1 = icmp slt i32 %B, 2
+  %0 = or i1 %cmp, %cmp1
+  br label %if.end
+
+if.else:                                          ; preds = %entry
+  %cmp2 = icmp sgt i32 %A, 0
+  %cmp3 = icmp sgt i32 %B, 0
+  %1 = and i1 %cmp2, %cmp3
+  br label %if.end
+
+if.end:                                           ; preds = %if.else, %if.then
+  %b.0.in = phi i1 [ %0, %if.then ], [ %1, %if.else ]
+  %conv = zext i1 %b.0.in to i32
+  ret i32 %conv
+}
+
+; CHECK-LABEL: select_noccmp1
+define i64 @select_noccmp1(i64 %v1, i64 %v2, i64 %v3, i64 %r) {
+; CHECK: cmp x0, #13
+; CHECK-NEXT: ccmp x0, #0, #0, gt
+; CHECK-NEXT: cset [[REG1:w[0-9]+]], lt
; CHECK-NEXT: cmp x2, #4
-; CHECK-NEXT: cset [[REG3:w[0-9]+]], gt
-; CHECK-NEXT: and [[REG4:w[0-9]+]], [[REG0]], [[REG1]]
-; CHECK-NEXT: and [[REG5:w[0-9]+]], [[REG2]], [[REG3]]
-; CHECK-NEXT: orr [[REG6:w[0-9]+]], [[REG4]], [[REG5]]
-; CHECK-NEXT: cmp [[REG6]], #0
+; CHECK-NEXT: ccmp x2, #2, #0, gt
+; CHECK-NEXT: cset [[REG2:w[0-9]+]], lt
+; CHECK-NEXT: orr [[REG3:w[0-9]+]], [[REG1]], [[REG2]]
+; CHECK-NEXT: cmp [[REG3]], #0
; CHECK-NEXT: csel x0, xzr, x3, ne
; CHECK-NEXT: ret
  %c0 = icmp slt i64 %v1, 0


_______________________________________________
llvm-commits mailing list
 <mailto:llvm-commits at lists.llvm.org> llvm-commits at lists.llvm.org
 <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160608/2bdb155e/attachment.html>


More information about the llvm-commits mailing list