[PATCH] D143394: [RISCV] Add performMULcombine to perform strength-reduction

[Philipp Tomsich via Phabricator via llvm-commits]

philipp.tomsich added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:8574
+                                 const RISCVSubtarget &Subtarget) {
+  SDLoc DL(N);
+  MVT XLenVT = Subtarget.getXLenVT();
----------------
kito-cheng wrote:
> Early exit if no `Subtarget.hasVendorXTHeadBa()`?
> 
The final optimization (turning this into slli + slli + add is applicable to RV64I).
So that block (the one "C has 2 bits set") will eventually move out of the if in a later commit.

Refer to the remark from the commit message:
> Even though the slli+slli+add sequence would we supported without
> XTheadBa, this currently is gated to avoid having to update a large
> number of test cases (i.e., anything that has a multiplication with a
> constant where only 2 bits are set) in this commit.

For that reason, I'd like to keep the logic as is.




================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:8613
+
+  if (Subtarget.hasVendorXTHeadBa()) {
+    // We try to simplify using shift-and-add instructions into up to
----------------
kito-cheng wrote:
> Does it applicable on zba?
Yes, it is applicable.
I'd like to keep this for a later commit (once we had a chance to run a full QA run with this enabled for Zba).

Ok to defer?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:8645
+                 ? Value
+                 : DAG.getNode(ISD::ANY_EXTEND, DL, XLenVT, Value);
+    };
----------------
craig.topper wrote:
> craig.topper wrote:
> > It's harmless to create a nop ANY_EXTEND. getNode detects it.
> Though I'm not sure why the VT would be different. The code doesn't look through any extends or truncates so the types should be changing right?
th.addsl is defined for XLen only (unlike mul, which has a W-form).
When operating on a MVT::i32, this any-extends and then truncate the result (which will be merged into a W-form add or shift on the final instruction).

If we don't any-extend, the compile on RV64 for the function

> unsigned func32(unsigned a)
> {
>   return a*200;
> }
> 

will keep the first operation separated out as a slliw + add:

> func32:
> 	slliw	a1, a0, 2
> 	add	a0, a0, a1
> 	th.addsl	a0, a0, a0, 2
> 	slliw	a0, a0, 3
> 	ret
> 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143394