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

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp:1036-1064
+  // If the instruction is G_FSHR, has a 64-bit G_CONSTANT for shift amount
+  // in the range of 0 <-> BitWidth, it is legal
+  if (ShiftTy.getSizeInBits() == 64 &&
+      MI.getOpcode() == TargetOpcode::G_FSHR && 
+      Amount.ult(BitWidth) &&
+      Amount.uge(0)) {
+    return true;
----------------
arsenm wrote:
> Could this just be an optimization combine instead? You're ultimately just falling back to the default legalization
Can you explain what you mean? My understanding was that returning true meant "this has been legalized", so the G_FSHR with a constant shift between 0 and BitWidth is legal. (So it ends up reusing the tablegen patterns for fshr). Non-constant shifts are illegal (expanded) and fshl are transformed to a fshr.


================
Comment at: llvm/test/CodeGen/AArch64/funnel-shift.ll:474
+; CHECK-GI:       // %bb.0:
+; CHECK-GI-NEXT:    extr w0, w0, w1, #0
+; CHECK-GI-NEXT:    ret
----------------
This one may be incorrect I think. It should be returning the other operand from the test below (ideally without a extr).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155484