[PATCH] D118107: [AArch64] Reassociate integer extending reductions to pairwise addition.

[Sjoerd Meijer via Phabricator via llvm-commits]

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

Looks like a good change to me.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13131
+// transform UADDV(add(y, add(zext(extract_lo(x)), zext(extract_hi(x))))).
+static SDValue performUADDVCombine(SDNode *N, SelectionDAG &DAG) {
+  auto DetectAddExtract = [&](SDValue A) {
----------------
"naming the hardest problem in computer science"?

Just a nit about the function names. Perhaps this should be `performSUADDVCombine` for completeness/clarity?
Then, we wouldn't need to rename `performUADDVCombine` to `performAddUADDVCombine` although the new name is probably better otherwise a 1 character letter difference would be difficult to read.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13134
+    // Look for add(zext(extract_lo(x)), zext(extract_hi(x))), returning
+    // UADDLP(x) if found.
+    if (A.getOpcode() != ISD::ADD)
----------------
Or a `SADDLP` if the zexts are sexts in the example above, right?


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

https://reviews.llvm.org/D118107