[PATCH] D106237: [ISel] Port AArch64 HADD and RHADD to ISel

Simon Pilgrim via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 7 00:11:59 PST 2022


RKSimon accepted this revision.
RKSimon added a comment.
This revision is now accepted and ready to land.

LGTM with a few minors



================
Comment at: llvm/include/llvm/CodeGen/ISDOpcodes.h:619
+  /// type i[N+1], halving the result by shifting it one bit right.
+  /// shr(add(ext(X), ext(Y)), 1)
+  AVGFLOORS,
----------------
Really pedantic but none of these description refer to these being averaging adds, yet a lot of the comments in other places just refers to them as averages.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12610
+static SDValue performAvgCombine(SDNode *N, SelectionDAG &DAG) {
+  EVT VT = N->getValueType(0);
+
----------------
assert(N->getOpcode() == ISD::TRUNCATE && "TRUNCATE node expected");


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12670
+  EVT OpAVT = OpA.getValueType();
+  assert(ExtendOpA.getValueType() == ExtendOpB.getValueType());
+  if (VT != OpAVT || OpAVT != OpB.getValueType())
----------------
Is this assert necessary?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14353
+  // extracted subvectors from the same original vectors. Combine these into a
+  // single [us]rhadd or [us]hadd that operates on the two original vectors.
+  // avgceil is the target independant name for rhadd, avgfloor is a hadd.
----------------
update these as well


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

https://reviews.llvm.org/D106237



More information about the llvm-commits mailing list