[PATCH] D137936: [AArch64] Optimize cmp chain before legalization

[chenglin.bi via Phabricator via llvm-commits]

bcl5980 added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:8614
     // Exit early by inverting the condition, which help reduce indentations.
-    SDValue TVal = DAG.getConstant(1, DL, VT);
-    SDValue FVal = DAG.getConstant(0, DL, VT);
-    AArch64CC::CondCode CC = changeIntCCToAArch64CC(Cond);
-    AArch64CC::CondCode InvCC = AArch64CC::getInvertedCondCode(CC);
-    return DAG.getNode(AArch64ISD::CSEL, DL, VT, FVal, TVal,
-                       DAG.getConstant(InvCC, DL, MVT::i32), CCmp);
+    return DAG.getSetCC(DL, VT, Cmp, DAG.getConstant(0, DL, VT), Cond);
   }
----------------
bcl5980 wrote:
> Allen wrote:
> > bcl5980 wrote:
> > > It looks this can continue to be simplified to 
> > > 
> > > ```
> > >     unsigned LogicOp = (Cond == ISD::SETEQ) ? ISD::AND : ISD::OR;
> > >     SDValue Cmp = DAG.getSetCC(DL, VT, XOR0, XOR1, Cond);
> > >     for (unsigned I = 1; I < WorkList.size(); I++) {
> > >       std::tie(XOR0, XOR1) = WorkList[I];
> > >       SDValue CmpChain = DAG.getSetCC(DL, VT, XOR0, XOR1, Cond);
> > >       Cmp = DAG.getNode(LogicOp, DL, VT, Cmp, CmpChain);
> > >     }
> > > 
> > >     return Cmp;
> > > ```
> > > Looks more cases can get benefit from it.
> > Yes, this change fix the case @PR58675, while regression on case combine_setcc_glue, so I'll need more work on it.
> > ```
> > +++ b/llvm/test/CodeGen/AArch64/dag-combine-setcc.ll
> > @@ -191,9 +191,11 @@ define i32 @combine_setcc_glue(i128 noundef %x, i128 noundef %y) {
> >  ; CHECK-LABEL: combine_setcc_glue:
> >  ; CHECK:       // %bb.0: // %entry
> >  ; CHECK-NEXT:    cmp x1, x3
> > -; CHECK-NEXT:    ccmp x0, x2, #0, eq
> > -; CHECK-NEXT:    ccmp x0, x2, #4, ne
> > -; CHECK-NEXT:    cset w0, eq
> > +; CHECK-NEXT:    cset w8, eq
> > +; CHECK-NEXT:    cmp x0, x2
> > +; CHECK-NEXT:    cset w9, eq
> > +; CHECK-NEXT:    and w8, w9, w8
> > +; CHECK-NEXT:    orr w0, w8, w9
> >  ; CHECK-NEXT:    ret
> > ```
> Don't worry about the combine_setcc_glue regression. D138401 already fixed that.
Sorry, the change fix combine_setcc_glue is D138398


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137936/new/

https://reviews.llvm.org/D137936