[PATCH] D150324: [X86] Make constant `mul` -> `shl` + `add`/`sub` work for vector types

Mon May 29 22:40:06 PDT 2023

goldstein.w.n added inline comments.

================
Comment at: llvm/lib/Target/X86/X86.td:562-564
+def TuningFastImmVectorShift : SubtargetFeature<"tuning-fast-imm-vector-shift",
+                                   "FastImmVectorShift", "true",
+                                   "Vector shifts are fast (2/cycle) as opposed to slow (1/cycle)">;
----------------
pengfei wrote:
> goldstein.w.n wrote:
> > pengfei wrote:
> > > Can we get such info from SchedModel rather than put another tuning?
> > > Can we get such info from SchedModel rather than put another tuning?
> > 
> > I'm not sure. The transform proposed in this patch works on SDNode which aren't in MachineInst form yet. AFAIK the schedmodel works on MachineInst, not SDNode, but if there is a way to get sched info for an SDNode I agree it would be better to use that.
> > Is there a way?
> That's a good point. My suggestion was based on an assumption that if a target has fast shifts, all shift instructions, no matter SSE/AVX/AVX512 MI should have the same port info. So we can choose any one of them to check here.
> However, I found it is not true, see https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/X86/X86SchedSkylakeServer.td#L416-L419
> Adding a tuning flag has the same problem.
> One alternative way is to check `WriteVecShiftImm/X/Y/Z` based on type and predicates. It's not always precise and I think it's a big hammer for this.
Agreed, although this does highlight that it would be nice to have some scalable way to estimate SDNode cost

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150324