[PATCH] D155484: [AArch64] Global Isel Funnel Shift Lowering

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

In D155484#4520130 <https://reviews.llvm.org/D155484#4520130>, @tschuett wrote:

> You lower at least s37 and s128 without any attempts to legalize them. There may be some opportunities, at least for s37.

I would expect that i128 is more important than i37. It may be a job for combining, as opposed to legalization (or better generic code for legalizing fsh's)



================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp:1005
+
+  // Keep as G_FSHR if number of shifts is a G_CONSTANT, else use generic
+  // lowering
----------------
number of shifts -> shift amount


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp:1017
+  int64_t BitWidth = ShiftTy.getSizeInBits();
+  int64_t Amount = MI.getOpcode() == TargetOpcode::G_FSHL
+                       ? BitWidth - VRegAndVal->Value.getSExtValue()
----------------
It might be worth making this an APInt, so that it doesn't need to convert again below.


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp:1023
+  // in the range of 0 <-> BitWidth, it is legal
+  if ((ShiftTy.getSizeInBits() == 64) &&
+      (MI.getOpcode() == TargetOpcode::G_FSHR) && (Amount < BitWidth) &&
----------------
These have more () brackets than they need.


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

https://reviews.llvm.org/D155484