[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:5864
+  int64_t C1 = NC1->getSExtValue();
+  if (APInt(64, C1).isSignedIntN(12) || !APInt(64, C1 % C0).isSignedIntN(12) ||
+      !APInt(64, C1 / C0).isSignedIntN(12))
----------------
You just use isInt<12> from MathUtils.h and not use APInt.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:5864
+  int64_t C1 = NC1->getSExtValue();
+  if (APInt(64, C1).isSignedIntN(12) || !APInt(64, C1 % C0).isSignedIntN(12) ||
+      !APInt(64, C1 / C0).isSignedIntN(12))
----------------
craig.topper wrote:
> You just use isInt<12> from MathUtils.h and not use APInt.
You need to guard against C0 being 0.

I'm also concerned about the possibility of signed overflow if C1=INT_MIN C0 = -1.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:5875
+    return NewOp1;
+  else
+    return DAG.getNode(ISD::ADD, DL, VT, NewOp1,
----------------
No need for else after a return.


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

https://reviews.llvm.org/D108607