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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:11425
+static SDValue foldSelectOfCTTZ(SDNode *N, SelectionDAG &DAG) {
+  if (N->getNumOperands() != 3)
+    return SDValue();
----------------
This is unnecessary. If it is a ISD::SELECT it has exactly 3 operands.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:11439
+
+  if (!CTTZ.getNumOperands())
+    return SDValue();
----------------
This is unnecesary


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:11452
+
+  assert((CTTZ.getValueType() == MVT::i32 || CTTZ.getValueType() == MVT::i64) &&
+         "Illegal type in CTTZ folding");
----------------
Not sure anything above gurantees this. ISD::CTTZ and ISD::CTTZ_ZERO_UNDEF are valid for any type.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:11455
+
+  if (Op1.getOpcode() != ISD::SETCC || Op1->getNumOperands() != 3)
+    return SDValue();
----------------
the getNumOperands check is unnecessary. ISD::SETCC can only have exactly 3 operands.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:11459
+  ISD::CondCode CCVal = cast<CondCodeSDNode>(Op1->getOperand(2))->get();
+  if (!ISD::isIntEqualitySetCC(CCVal)) {
+    return SDValue();
----------------
Drop curly braces


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:11482
+
+  if (AndNode.getValueType() == llvm::MVT::i64 ||
+      N->getValueType(0) == llvm::MVT::i32) {
----------------
You can use getZExtOrTrunc here to simplify this.


================
Comment at: llvm/test/CodeGen/RISCV/cttz_zero_return_test.ll:3
+
+ at global_x = dso_local local_unnamed_addr global i32 0, align 4
+
----------------
drop dso_local and local_unnamed_addr throughout the test.


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

https://reviews.llvm.org/D151449