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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:3420-3424
+  virtual bool
+  shiftOrRotateIsFasterWithConstantShiftAmount(const SDNode *N,
+                                               CombineLevel Level) const {
+    return false;
+  }
----------------
lebedev.ri wrote:
> arsenm wrote:
> > jlebar wrote:
> > > 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?
> > I mean SelectionDAG is bad and am just generally disappointed there isn't more work going on for globalisel optimizations, and everyone seems to still just be working on SelectionDAG
> From personal expirience - i only look at seldag because that is what is on the current (codegen) path.
> Doing stuff for GISel would currently result in no benefit to codegen on X86, which kinda misses the point.
> Is there an actual plan for X86 target to migrate to GISel?
<crickets chirping...>

Speaking only for myself here: migrating to GISel is a daunting task for x86 because there's so much code that has to be adapted, and I have not seen a list or estimate of the potential benefits to incentivize the move.

There are over 100 tests in llvm/test/CodeGen/X86/GlobalISel, so some work has been done. But whether that is planned to continue or not, I have no idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74165