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

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n marked 2 inline comments as done.
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:
> 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?


================
Comment at: llvm/test/CodeGen/X86/omit-urem-of-power-of-two-or-zero-when-comparing-with-zero.ll:323
+; SSE4-NEXT:    paddd %xmm2, %xmm1
+; SSE4-NEXT:    pslld $2, %xmm2
+; SSE4-NEXT:    paddd %xmm1, %xmm2
----------------
pengfei wrote:
> IIRC, the intention is to generate `pslld` for new targets. Why this is affected given it's a general turning?
> 
> The same for others. I didn't check all, but in most tests we just enable features without specifying a turning target.
> IIRC, the intention is to generate `pslld` for new targets. Why this is affected given it's a general turning?
> 
> The same for others. I didn't check all, but in most tests we just enable features without specifying a turning target.

That was the 2x `pslld` case but changed to cover the 1x `pslld` + 2x `padd` case as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150324