[PATCH] D151449: [RISCV] Add DAG combine for CTTZ in the case of input 0

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro marked 6 inline comments as done.
djtodoro added inline comments.
Herald added a subscriber: wangpc.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:11430
+
+  if (Op3.getOpcode() == ISD::TRUNCATE || Op3.getOpcode() == ISD::ZERO_EXTEND) {
+    CTTZ = Op3.getOperand(0);
----------------
mgudim wrote:
> craig.topper wrote:
> > ```
> > SDValue CTTZ = N->getOperand(2);
> > if (CTTZ.getOpcode() == ISD::TRUNCATE || CTTZ.getOpcode() == ISD::ZERO_EXTEND)
> >   CTTZ = CTTZ.getOperand(0);
> > ```
> can you please give the example IR when we see `ZERO_EXTEND` or `TRUNCATE`?
Please take a look into the functions: `ctz_dereferencing_pointer_zext` and `ctz4` from the test.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:11442
+
+  // The `true` branch should be constant `0`.
+  if (!isNullConstant(Op2))
----------------
mgudim wrote:
> what if select is reversed?
Done. Covered that as well.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:11450
+  ISD::CondCode CCVal = cast<CondCodeSDNode>(Op1->getOperand(2))->get();
+  if (!ISD::isIntEqualitySetCC(CCVal))
+    return SDValue();
----------------
craig.topper wrote:
> If we're requiring Op2 to be the null constant and Op3 to be the cttz then the only valid CC here is SETEQ. IsIntEquality checks for SETEQ or SETNE.
> 
> We should handle `X != 0 ? CTTZ : 0` as well
Yeah, I've handled the case that `LLVM opt` produces as a result of pattern matching, but yeah, definitely we should handle that case as well.


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

https://reviews.llvm.org/D151449