[PATCH] D121449: [AArch64] Combine ISD::SETCC into AArch64ISD::ANDS

[chenglin.bi via Phabricator via llvm-commits]

bcl5980 added a comment.

In D121449#3379479 <https://reviews.llvm.org/D121449#3379479>, @paulwalker-arm wrote:

> Not sure I understand.  The `combining: ....` text just means it's visiting a node.  There's no following text to suggest it has created anything new like there is when the `AND` is created. My guess is that DAGCombine continues to visit the operands of the original node despite it being replace.
>
> I just tried
>
>   // setcc (srl x, imm), 0, ne ==> setcc (and x, (-1 << imm)), 0, ne
>   if (Cond == ISD::SETNE && isNullConstant(RHS) &&
>       LHS->getOpcode() == ISD::SRL && isa<ConstantSDNode>(LHS->getOperand(1)) &&
>       LHS->hasOneUse()) {
>     EVT TstVT = LHS->getValueType(0);
>     if (TstVT == MVT::i32 || TstVT == MVT::i64) {
>       uint64_t TstImm = -1ULL << LHS->getConstantOperandVal(1);
>       SDValue TST = DAG.getNode(ISD::AND, DL, TstVT, LHS->getOperand(0),
>                                 DAG.getConstant(TstImm, DL, TstVT));
>       return DAG.getNode(ISD::SETCC, DL, VT, TST, RHS, N->getOperand(2));
>     }
>   }
>
> using origin/main from a few minutes ago and I get the same new output for `llvm/test/CodeGen/AArch64/arm64-xaluo.ll ` as you do.  Are you seeing something different?

Yeah, it works. Test failed because this code generate tst w8, 0xff00(not 0xffffff00). That is also correct as knownbits is 16 bit for i8 mul.
So the question is which way do you think is better?

performSUBSCombine:  subs (srl x, imm), 0 ==> ands x, (-1 << imm) 
or
performSETCCCombine: setcc (srl x, imm), 0, ne ==> setcc (and x, (-1 << imm)), 0, ne

Both of them keep ISD before legalize.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121449