[PATCH] D108607: [RISCV] Optimize (add (mul x, c0), c1)

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:5851
+  EVT VT = N->getValueType(0);
+  if (!VT.isInteger() || VT.isVector() ||
+      VT.getSizeInBits() > Subtarget.getXLen())
----------------
VT is guaranteed integer since you're starting from an integer add instruction.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:5859
+  // Check if c0 and c1 match above conditions.
+  auto *NC0 = dyn_cast<ConstantSDNode>(N0->getOperand(1));
+  auto *NC1 = dyn_cast<ConstantSDNode>(N->getOperand(1));
----------------
I believe N0C and N1C are more consistent with other DAGCombiner code.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:5887
+  // if c1%c0 is zero, and c1/c0 is simm12 while c1 is not.
+  SDValue N0 = transformAddImmMulImm(N, DAG, Subtarget);
+  if (N0)
----------------
I don't like the name N0 here. SelectionDAG code very often uses N0 to mean N->getOperand(0).

I'd suggest

```
if (SDValue V = transformAddImmMulImm(N, DAG, Subtarget))
  return V;
```

V is more generic name and putting in the declare in the `if` limits its scope.


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

https://reviews.llvm.org/D108607