[PATCH] D150969: [AArch64] Try to convert two XTN and two SMLSL to UZP1, SMLSL and SMLSL2

JinGu Kang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 12 02:24:37 PDT 2023


jaykang10 added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:22381
+// Try to combine op with uzp1.
+static SDValue tryCombineOpWithUZP1(SDNode *N,
+                                    TargetLowering::DAGCombinerInfo &DCI,
----------------
dmgreen wrote:
> Perhaps name this tryCombineMULLWithUZP1, to show it operands on mull nodes.
Let me update the name.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:22437
+  SDValue EXTRACTHIGHSrcVec = EXTRACTHIGH.getOperand(0);
+  if (EXTRACTHIGHSrcVec->use_size() != 2)
+    return SDValue();
----------------
dmgreen wrote:
> If these or the conditions below fail, could it still create the UZP with an undef operand?
> Something like this in pseudocode.
> ```
> SDValue TRUNCLOWOP = DAG.getUndef(VT);
> if (.. find the other operand through the uses) // This is the complex bit
>   TRUNCLOWOP = Found Other Op;
> UZP = DAG.getNode(AArch64ISD::UZP1, DL, UZP1VT, TRUNCLOWOP, TRUNCHIGHOP);
> ReplaceUse(TRUNCHIGH, UZP EXTRACT_SUBVECTOR Hi).
> if (previouslyFoundOtherOp)
>   ReplaceUse(TRUNCLOW, UZP EXTRACT_SUBVECTOR Lo).
> ```
> I believe that should then handle some of the other cases like efriedma mentioned.
Ah, I did not know you wanted to generate the uzp1 with undef.
Let me update the code.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:22451
+
+    if (ConstantSDNode *IdxCst =
+            dyn_cast<ConstantSDNode>(User->getOperand(1))) {
----------------
dmgreen wrote:
> This could use isNullConstant
Let me update it.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:22492
+  // Create uzp1, extract_high and extract_low.
+  EVT TRUNCHIGHVT = TRUNCHIGH.getValueType();
+  EVT TRUNCLOWVT = TRUNCLOW.getValueType();
----------------
dmgreen wrote:
> The LLVM naming scheme would be TruncHighVT I think.
Let me update the name.


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

https://reviews.llvm.org/D150969



More information about the llvm-commits mailing list