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

[Ben Shi via Phabricator via llvm-commits]

benshi001 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:537
+    SDNode *NM =
+        CurDAG->getMachineNode(RISCV::MUL, DL, VT, SDValue(NA, 0),
+                               N0->getOperand(1));
----------------
benshi001 wrote:
> benshi001 wrote:
> > craig.topper wrote:
> > > benshi001 wrote:
> > > > craig.topper wrote:
> > > > > What about all the optimizations we have for mul by constant using SHXADD? Won't this miss out on those?
> > > > I do not think it will affect the optimization with SHxADD.
> > > > 
> > > > For the ones such as
> > > > (mul 11/13/25/41/73/37/21), (mul (3/5/9)*power_of_2), (mul power_of_2 + (2/4/8)), those are pure mul without add involved, so they won't be affected.
> > > > 
> > > > For the rules such as
> > > > (x + y * 4) -> (SH2ADD y, x)
> > > > (x + y * 20) -> (SH2ADD (SH2ADD x, x), y)
> > > > .......
> > > > 
> > > > My patch still generate better code.
> > > > 
> > > > c1 + x * 72 (c1 is a non-simm12 constant)
> > > > 
> > > > before current patch
> > > > ```
> > > > lui Ry, higher-bits of c1
> > > > addi Ry, Ry, lower-12-bits of c1
> > > > sh3add Rz, x, x
> > > > sh3add Rz, Rz, Ry
> > > > ```
> > > > 
> > > > after my patch
> > > > ```
> > > > addi Ry, x, c1/72
> > > > sh3add Ry, Ry, Ry
> > > > sll Ry, Ry, 3
> > > > ```
> > > > 
> > > > I will add those cases to the test file.
> > > Isn’t it possible for the MUL you create here to have 11/13/25/41/73/37/21 as a constant that should use SHXADD?
> > Yes, it is possible. And for mul with 11/13/25/41/73/37/21 which use shxadd, my change still generates better asm. I will add in the tests.
> Sorry, your concern is right. I can not handle mul with 11/13/25/41/73/37/21. 
I have skip those constants in the zba extension. And will figure out a better way for them in the future.

Thanks for your help.


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

https://reviews.llvm.org/D107711