[PATCH] D102333: [AArch64] Combine shift instructions in SelectionDAG

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Thanks. I'm glad this way worked.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14561
 
+static SDValue performShiftCombine(SDNode *N, const AArch64TargetLowering &TLI,
+                                   TargetLowering::DAGCombinerInfo &DCI) {
----------------
Perhaps performVectorShiftCombine


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14564
+  unsigned Opc = N->getOpcode();
+  if (Opc != AArch64ISD::VASHR && Opc != AArch64ISD::VLSHR)
+    return SDValue();
----------------
This probably isn't needed, or could be an assert.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14574
+  APInt ShiftedOutBits =
+      APInt::getLowBitsSet((unsigned)OpScalarSize, (unsigned)ShiftImm);
+  APInt DemandedMask = ~ShiftedOutBits;
----------------
I'm not sure these need casts, or to be uint64_t. They should both be fairly small.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14577
+
+  TLI.SimplifyDemandedBits(Op, DemandedMask, DCI);
+  return SDValue();
----------------
Most other uses of this function that I see seem to use:
if (TLI.SimplifyDemandedBits(..))
  return SDValue(N, 0);
It may not alter much, but will be closer to what DAGCombiner::combine expects the return value to be for something that changed.


================
Comment at: llvm/test/CodeGen/AArch64/aarch64-bswap-ext.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -O2 -mtriple=aarch64-unknown-linux-gnu < %s | FileCheck %s
+
----------------
This probably doesn't need -O2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102333