[PATCH] D58950: [PowerPC] Strength reduction of multiply by a constant by shift and add/sub in place

[Jinsong Ji via Phabricator via llvm-commits]

jsji added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:14591
+
+      // The cycles of related operation are showed as a table above.
+      // Only do (mul x, -(2^N + 1)) => -(add (shl x, N), x) for vector type.
----------------
wuzish wrote:
> jsji wrote:
> > These are for P9 right? I believe P8 is different no?
> Hmm, well. Actually I only have cycles information table about P9, so we can only do for P9 and leave P8 as TODO, and also change comments into `// TODO: enhance the condition for subtarget before pwr9`
OK for me.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:14593
+      // Only do (mul x, -(2^N + 1)) => -(add (shl x, N), x) for vector type.
+      // Because vector type mul costs more cycles than scalar type,
+      // it's worthy to do such opt.
----------------
wuzish wrote:
> jsji wrote:
> > ? I am confused: the combination here is doing strength reduction from `mul` to `shl`. Why we are comparing the cycles between vector  type and scalar type? Should we compare the cycles for `mul` seq and `shl` seq for the SAME type?
> Because this transformation replacement are 3 instrs. So it's not profitable for scalar mul operation because scalar mul takes 5 cycles, but it's profitable for vector mul operation because vector mul takes 7 cycles.
yes, but my point is your comment here is misleading.

How about something like:
``` mul is 5(scalar)/7(vector) cycle, add/sub/shl are all 2 cycle. For 2 instrs patterns, add/sub + shl are 4 cycles, it is always profitable; but for 3 instrs patterns (mul x, -(2^N + 1)) => -(add (shl x, N), x) , sub+add+shl are 6 cycles, so we should only do it for vector type. ```


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

https://reviews.llvm.org/D58950