[PATCH] D74165: [x86] [DAGCombine] Prefer shifts of constant widths.

Tue Feb 18 13:31:05 PST 2020

jlebar marked an inline comment as done.
jlebar added inline comments.

================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:3420-3424
+  virtual bool
+  shiftOrRotateIsFasterWithConstantShiftAmount(const SDNode *N,
+                                               CombineLevel Level) const {
+    return false;
+  }
----------------
arsenm wrote:
> jlebar wrote:
> > arsenm wrote:
> > > I think we generally have too many, overly specific queries like this. Is there any real reason to NOT do this on any target?
> > I am also not thrilled about adding another query like this that pretends to be general but really means "should I do this particular peephole optimization?"
> > 
> > I gated it conditionally on the architecture because ISTM that this transformation could be a pessimation on platforms where shift-by-register is as fast as shift-by-immediate.  We're replacing 1 x shift-by-register + 1 x select with 2 x shift-by-immediate + 1 x select.  AFAICT e.g. on Coretex-A75 this would not be an improvement.
> GlobalISel avoids this problem by having every combine be an explicit opt in if I can encourage you to also do this there
Happy to consider it, but I'm not sure what the suggestion is exactly.  (Or at least, this seems like we're making the combine be an explicit opt-in, so I don't see how it's different from your suggestion.)  Can you point me to the prior art as used in non-globalisel?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74165