[AArch64] Explicitly use v1i64 type for llvm.aarch64.neon.pmull64

[mingmingl]

Summary

Without this, the intrinsic will be expanded to an integer; thereby an
explicit copy (from GPR to SIMD register) will be codegen'd. This matches the
general convention of using "v1" types to represent scalar integer operations in
vector registers.

The similar approach is observed in D56616, and the pattern likely applies on
other intrinsic that accepts integer scalars (e.g.,
int_aarch64_neon_sqdmulls_scalar)

[mingmingl]

run 'clang-format' and polish comment

[dmgreen]

Thanks, this sounds good to me.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

4539

Can we do this for all operands, not just loads? We should end up adding the i64->v1i64 copy in either case.

4540

Op1.getValueType()

llvm/test/CodeGen/AArch64/pmull-ldr-merge.ll

3

I think use -mattr=+aes. (Even is that is not technically correct, it is what the instruction uses)
And it doesn't need CHECK-SDAG

33

The test could be simpler if it wasn't in a loop.

[mingmingl]

address comments.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

4539

Generalize by doing this for all operands, except when they are higher half of the SIMD register, and added comments to explain why.

• When operands are higher half, this tablegen pattern is optimal -> test_pmull_high_64 shows this.

Also add test2 and test3 for this generalization

• https://gcc.godbolt.org/z/79sajTcf7 is codegen of trunk
• test2 has a load and an extract-element
• test3 has a load and a direct use of i64

Also I'd be glad to send an NFC patch of the test case first, and the diffs become more obvious with this patch.

[dmgreen]

Thanks, LGTM.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

4544

We can drop the brackets around single statements.

[mingmingl]

drop the bracket around single statements.

[mingmingl]

In D130548#3681576, @dmgreen wrote:

Thanks, LGTM.

Thanks for the quick reviews and feedback!

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

if (Ty == MVT::i64) {
       Result = DAG.getNode(ISD::ABS, dl, MVT::v1i64, Result);
       return DAG.getNode(ISD::BITCAST, dl, MVT::i64, Result);
     } else if (Ty.isVector() && Ty.isInteger() && isTypeLegal(Ty)) {
       return DAG.getNode(ISD::ABS, dl, Ty, Op.getOperand(1));
     } else {
       report_fatal_error("Unexpected type for AArch64 NEON intrinic");
     }
   }
+    case Intrinsic::aarch64_neon_pmull64: {
+    // Matches the general convention of using "v1" types to represent scalar integer operations in vector registers.
+    // In this way, load 
+    SDValue Op1 = Op.getOperand(1);
+    SDValue Op2 = Op.getOperand(2);
+    if (Op1.getNode()->getOpcode() == ISD::LOAD && Op1.getNode()->getValueType(0) == MVT::i64) {
+      Op1 = DAG.getNode(ISD::BITCAST, dl, MVT::v1i64, Op1);
+    }
+    if (Op2.getNode()->getOpcode() == ISD::LOAD && Op2.getNode()->getValueType(0) == MVT::i64) {
+      Op2 = DAG.getNode(ISD::BITCAST, dl, MVT::v1i64, Op2);
+    }
+    return DAG.getNode(
+        ISD::INTRINSIC_WO_CHAIN, dl, Op.getValueType(),
+        DAG.getConstant(Intrinsic::aarch64_neon_pmull64, dl, MVT::i32), Op1,
+        Op2);
+  }
   case Intrinsic::aarch64_neon_smax:
     return DAG.getNode(ISD::SMAX, dl, Op.getValueType(),
                        Op.getOperand(1), Op.getOperand(2));
   case Intrinsic::aarch64_neon_umax:
     return DAG.getNode(ISD::UMAX, dl, Op.getValueType(),
                        Op.getOperand(1), Op.getOperand(2));
   case Intrinsic::aarch64_neon_smin:
     return DAG.getNode(ISD::SMIN, dl, Op.getValueType(),
                        Op.getOperand(1), Op.getOperand(2));
   case Intrinsic::aarch64_neon_umin:
     return DAG.getNode(ISD::UMIN, dl, Op.getValueType(),
                        Op.getOperand(1), Op.getOperand(2));
 
   case Intrinsic::aarch64_sve_sunpkhi:
     return DAG.getNode(AArch64ISD::SUNPKHI, dl, Op.getValueType(),
                        Op.getOperand(1));
   case Intrinsic::aarch64_sve_sunpklo:
     return DAG.getNode(AArch64ISD::SUNPKLO, dl, Op.getValueType(),
                        Op.getOperand(1));
   case Intrinsic::aarch64_sve_uunpkhi:
     return DAG.getNode(AArch64ISD::UUNPKHI, dl, Op.getValueType(),
                        Op.getOperand(1));

llvm/test/CodeGen/AArch64/pmull-ldr-merge.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs  -mtriple=aarch64-linux-gnu -mattr=+v8r,+neon,+crc,+crypto -o - %s| FileCheck %s --check-prefixes=CHECK,CHECK-SDAG
+
+define void @func(ptr %0, i64 %1, i64 %2) {
+; CHECK-LABEL: func:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    subs x8, x2, x1
+; CHECK-NEXT:    b.ls .LBB0_3
+; CHECK-NEXT:  // %bb.1:
+; CHECK-NEXT:    add x9, x0, #8
+; CHECK-NEXT:    lsl x10, x1, #4
+; CHECK-NEXT:  .LBB0_2: // =>This Inner Loop Header: Depth=1
+; CHECK-NEXT:    ldr d0, [x9, x10]
+; CHECK-NEXT:    subs x8, x8, #1
+; CHECK-NEXT:    ldr d1, [x9]
+; CHECK-NEXT:    pmull v0.1q, v1.1d, v0.1d
+; CHECK-NEXT:    stur q0, [x9, #-8]
+; CHECK-NEXT:    add x9, x9, #16
+; CHECK-NEXT:    b.ne .LBB0_2
+; CHECK-NEXT:  .LBB0_3:
+; CHECK-NEXT:    ret
+  %4 = icmp ugt i64 %2, %1
+  br i1 %4, label %5, label %7
+
+5:
+  %6 = sub i64 %2, %1
+  br label %8
+
+7:
+  ret void
+
+8:
+  %9 = phi i64 [ %18, %8 ], [ 0, %5 ]
+  %10 = phi i64 [ %12, %8 ], [ %1, %5 ]
+  %11 = getelementptr inbounds <2 x i64>, ptr %0, i64 %9
+  %12 = add nuw i64 %10, 1
+  %13 = getelementptr inbounds <2 x i64>, ptr %0, i64 %9, i64 1
+  %14 = load i64, ptr %13, align 8
+  %15 = getelementptr inbounds <2 x i64>, ptr %0, i64 %10, i64 1
+  %16 = load i64, ptr %15, align 8
+  %17 = tail call <16 x i8> @llvm.aarch64.neon.pmull64(i64 %14, i64 %16)
+  store <16 x i8> %17, ptr %11, align 16
+  %18 = add i64 %9, 1
+  %19 = icmp eq i64 %18, %6
+  br i1 %19, label %7, label %8
+}
+
+declare <16 x i8> @llvm.aarch64.neon.pmull64(i64, i64)
+;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
+; CHECK-SDAG: {{.*}}