[PATCH] D143283: [AArch64][SVE]: custom lower AVGFloor/AVGCeil.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

Thanks for all the changes @hassnaa-arm, I've just left some final minor comments.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13375
 
+SDValue AArch64TargetLowering::LowerAVG(SDValue Op, SelectionDAG &DAG,
+                                        unsigned NewOp) const {
----------------
Can you add a comment explaining what this does, e.g. something like

  When x and y are extended, lower:
    avgfloor(x, y) -> (x + y) >> 1
    avgceil(x, y)  -> (x + y + 1) >> 1

  Otherwise, lower to:
    avgfloor(x, y) -> (x >> 1) + (y >> 1) + (x && y && 1)
    avgceil(x, y)  -> (x >> 1) + (y >> 1) + ((x || y) && 1)


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13404-13405
+      (IsSigned && IsSignExtended(OpA) && IsSignExtended(OpB))) {
+    // if the Ops are already extended, then shift their addition directly,
+    // it's pointless to shift each on on its own.
+    SDValue Add = DAG.getNode(ISD::ADD, dl, VT, OpA, OpB);
----------------
nit: this comment would be redundant if you address my other comment (to add a comment for the function itself)


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13412-13414
+  SDValue ShiftOpA, ShiftOpB;
+  ShiftOpA = DAG.getNode(ShiftOpc, dl, VT, OpA, ConstantOne);
+  ShiftOpB = DAG.getNode(ShiftOpc, dl, VT, OpB, ConstantOne);
----------------
nit: 

  SDValue ShiftOpA = DAG.getNode(ShiftOpc, dl, VT, OpA, ConstantOne);
  SDValue ShiftOpB = DAG.getNode(ShiftOpc, dl, VT, OpB, ConstantOne);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143283