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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:987-988
+    NVT = VT.changeVectorElementType(NVT);
+  if (!TLI.isOperationLegalOrCustom(AVGOpc, NVT))
+    return SDValue();
+
----------------
Can we get to here where the operations aren't legal yet?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:908
+  // add(add(ext, 1), ext)
+  // add(ext, add(ext, 1))
+  SDValue Add = Op.getOperand(0);
----------------
dmgreen wrote:
> lebedev.ri wrote:
> > There's also the case where one of the operands is a constant, so you don't have an explicit +1
> For ARM/Arch64 that would be an hadd (avgfloor), IIUC. For X86 where avgceil is legal but avgfloor is not, we can probably do something about that if needed, or make it target specific. It's probably best not to try and address ever issue in this single patch though.
Sure, this can be done in a follow-up.

What i'm saying is that if you want to catch more cases,
you might want to try to deal with `add(ext, C)` as with `add(ext, add(C-1, 1))` (or vice versa).
Probably this might be best done by adding a wrapper function,
and adding a parameter to this function, `bool LookingForCeil`.


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

https://reviews.llvm.org/D106237