[PATCH] D136319: [GISel] Rework trunc/shl combine in a generic trunc/shift combine

[Pierre van Houtryve via Phabricator via llvm-commits]

Pierre-vh added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:2297
+    // Try and reduce the shift when it's profitable according to the target.
+    LLT NewShiftTy = getTargetLowering().getPreferredShiftAmountTy(SrcTy);
+    const unsigned NewShiftSize = NewShiftTy.getScalarSizeInBits();
----------------
arsenm wrote:
> This usage of getPreferredShiftAmountTy doesn't make sense, but you need something to pick an intermediate type here. Without adding anything, I guess you could bisect types until you found a legal shift? Creating the minimum required shift also "should" work out, but may create more legalizer and optimizer work. It would be nice if we could directly inspect the list of legal shift types, but the API doesn't have that.
> 
> Does just reusing the truncating type work as well?
Reusing the truncating type doesn't work as well, and trying to guess a legal one doesn't either for AArch64 because 16 bits shifts seem legal there, but are suboptimal compared to 64 (word size) shifts.

My first version actually did a simplified bisecting to find a good type, but AArch64 suffered and that's why I didn't do it.

I think a new TLI hook would be best so targets can just choose whatever they prefer. I'll post a version with that and we can discuss there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136319