[PATCH] D82669: [ARM] Generate URHADD from (b - (~a)) >> 1

[Dave Green via Phabricator via llvm-commits]

dmgreen added reviewers: SjoerdMeijer, efriedma, sdesmalen.
dmgreen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:8810
+
+  if (!VT.isVector())
+    return Op;
----------------
It might be worth adding a check that this isn't a scalable vector.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:8818
+  // Is the right shift using an immediate value of 1?
+  ConstantSDNode *ShiftAmount = dyn_cast<ConstantSDNode>(Shift.getOperand(1));
+  if (!ShiftAmount)
----------------
getConstantOperandVal


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:8861-8864
+  LLVM_DEBUG(dbgs() << "aarch64-lower: transformed: \n");
+  LLVM_DEBUG(Op->dump(&DAG));
+  LLVM_DEBUG(dbgs() << "into: \n");
+  LLVM_DEBUG(ResultURHADD->dump(&DAG));
----------------
This debug isn't usually added, the combiner will print this kind of info already.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:11122
+//  (v16i8(urhadd(v16i8 OpA, v16i8 OpB)))
+  if (N->getNumOperands() == 2 &&
+      N0->getOpcode() == AArch64ISD::URHADD &&
----------------
I'm a little surprised that there is no code to do this already. I guess it doesn't usually come up. Please run clang-format on the patch.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:11134-11137
+      SDValue N00Source = N00->getOperand(0);
+      SDValue N01Source = N01->getOperand(0);
+      SDValue N10Source = N10->getOperand(0);
+      SDValue N11Source = N11->getOperand(0);
----------------
Make sure you check the 0 and the 8 from the extract_subvector.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.h:168
+  // Vector usigned rounding halving addition
+  URHADD,
+
----------------
Is it possible to add srhadd at the same time? I guess there is also uhadd and shadd?


================
Comment at: llvm/test/CodeGen/AArch64/arm64-vhadd.ll:332
 
+define void @testLowerToURHADD16b(<16 x i8> %src1, <16 x i8> %src2, <16 x i8>* %dest) nounwind {
+; CHECK-LABEL: testLowerToURHADD16b:
----------------
It is worth having tests for half width too - <8 x i8>.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82669