[PowerPC] MMA - Add __builtin_vsx_build_pair and __builtin_mma_build_acc builtins

[saghir]

Summary

This patch adds the following built-ins:

builtin_vsx_build_pair
builtin_mma_build_acc

[lei]

This comment was removed by lei.

[lei]

Actually we should not be removing the deprecated bultins. Just need to add the new ones.

[nemanjai]

In D107647#2937706, @lei wrote:

Actually we should not be removing the deprecated bultins. Just need to add the new ones.

Yes, and also the semantics are different.

[saghir]

Change the implementation to add new builtins and keep the depracated builtins.

[saghir]

Added some more tests.

[saghir]

Added comments, and re-organized tests.

[lei]

clang/lib/CodeGen/CGBuiltin.cpp

15919

doesn't look like we need the interm var IsLE. Just use the call directly within the if stmt.

if (getTarget().isLittleEndian()) {

15921–15923

unsigned NumVecs = (BuiltinID == PPC::BI__builtin_mma_build_acc) ? 4 : 2;

clang/test/CodeGen/builtins-ppc-pair-mma.c

2 ↗

(On Diff #368176)

future -> pwr10
We need to add BE tests.

clang/test/Sema/ppc-pair-mma-types.c

2–4

this should be -target-cpu pwer10 now.

2–4

Please add BE testing.

268

This looks like a dup of testVPLocal(). Why not just add the new call line to that function right below the call to the deprecated function?

clang/test/SemaCXX/ppc-pair-mma-types.cpp

2–4

please update to pwr10

[nemanjai]

There are some questions to answer here before proceeding:

1. Why do we not get paired vector loads and stores in the back end test cases for either little or big endian and regardless of whether we use the old or new builtins/intrinsics?
2. Is the code generated the same as that for GCC when:
   • The inputs are all from registers
   • The inputs are all from memory
   • The inputs are a mix of memory and registers
3. What do execution tests show for both little endian and big endian targets? Namely, write execution test cases that do everything mentioned in 2. above for both sets of builtins and confirm:
   • The behaviour is the same both with GCC and Clang for both LE and BE
   • The behaviour for new builtins is the same on LE and BE with both GCC and Clang
   • The behaviour for old builtins is different on LE and BE with both GCC and Clang

I am requesting changes until these questions are adequately addressed.

llvm/include/llvm/IR/IntrinsicsPowerPC.td

1442 ↗

(On Diff #368176)

I find the need for this rather surprising. The new builtins should do the exact same thing as the old builtins but with elements in reversed order. So why do we need new intrinsics? Can we not just call the old intrinsics in the front end codegen but with arguments in reversed order?

llvm/test/CodeGen/PowerPC/paired-vector-intrinsics.ll

73 ↗

(On Diff #368176)

This indicates some kind of problem. Why are we moving a value from v2 to v3 and then not using v3?

[saghir]

Addressed review comments.

[nemanjai]

LGTM other than the code can be simplified as suggested.

clang/lib/CodeGen/CGBuiltin.cpp

15919–15926

This entire block seems to simply be std::reverse(Ops.begin() + 1, Ops.end())

Also, please add a note that the very first operand is the pointer to the pair/accumulator that is actually being built.

[lei]

LGTM once the code is simplified as Nemanja suggested.

[saghir]

Addressed review comments.

clang/include/clang/Basic/BuiltinsPPC.def

 // The fourth argument is set to true if the built-in accumulates its result into
 // its given accumulator.
 
 // Provided builtins with _mma_ prefix for compatibility.
 CUSTOM_BUILTIN(mma_lxvp, vsx_lxvp, "W256SLiW256C*", false)
 CUSTOM_BUILTIN(mma_stxvp, vsx_stxvp, "vW256SLiW256C*", false)
 CUSTOM_BUILTIN(mma_assemble_pair, vsx_assemble_pair, "vW256*VV", false)
 CUSTOM_BUILTIN(mma_disassemble_pair, vsx_disassemble_pair, "vv*W256*", false)
+CUSTOM_BUILTIN(vsx_build_pair, vsx_assemble_pair,  "vW256*VV", false)
+CUSTOM_BUILTIN(mma_build_acc, mma_assemble_acc, "vW512*VVVV", false)
 
 // UNALIASED_CUSTOM_BUILTIN macro is used for built-ins that have
 // the same name as that of the intrinsic they generate, i.e. the
 // ID and INTR are the same.
 // This avoids repeating the ID and INTR in the macro expression.
 
 UNALIASED_CUSTOM_BUILTIN(vsx_lxvp, "W256SLiW256C*", false)
 UNALIASED_CUSTOM_BUILTIN(vsx_stxvp, "vW256SLiW256C*", false)

clang/lib/CodeGen/CGBuiltin.cpp

if (BuiltinID == PPC::BI__builtin_mma_disassemble_acc ||
       for (unsigned i=0; i<NumVecs; i++) {
         Value *Vec = Builder.CreateExtractValue(Call, i);
         llvm::ConstantInt* Index = llvm::ConstantInt::get(IntTy, i);
         Value *GEP = Builder.CreateInBoundsGEP(VTy, Ptr, Index);
         Builder.CreateAlignedStore(Vec, GEP, MaybeAlign(16));
       }
       return Call;
     }
+    if (BuiltinID == PPC::BI__builtin_vsx_build_pair ||
+        BuiltinID == PPC::BI__builtin_mma_build_acc) {
+      // Reverse the order of the operands for LE, so the
+      // same builtin call can be used on both LE and BE
+      // without the need for the programmer to swap operands.
+      // The operands are reversed starting from the second argument,
+      // the first operand is the pointer to the pair/accumulator
+      // that is being built.
+      if (getTarget().isLittleEndian())
+        std::reverse(Ops.begin() + 1, Ops.end());
+    }
     bool Accumulate;
     switch (BuiltinID) {
   #define CUSTOM_BUILTIN(Name, Intr, Types, Acc) \
     case PPC::BI__builtin_##Name: \
       ID = Intrinsic::ppc_##Intr; \
       Accumulate = Acc; \
       break;
   #include "clang/Basic/BuiltinsPPC.def"
     }
     if (BuiltinID == PPC::BI__builtin_vsx_lxvp ||
         BuiltinID == PPC::BI__builtin_vsx_stxvp ||

clang/test/CodeGen/builtins-ppc-build-pair-mma.c

+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -O3 -triple powerpc64le-unknown-unknown -target-cpu pwr10 \
+// RUN: -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-LE
+// RUN: %clang_cc1 -O3 -triple powerpc64-unknown-unknown -target-cpu pwr10 \
+// RUN: -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-BE
+
+// CHECK-LE-LABEL: @test1(
+// CHECK-LE-NEXT:  entry:
+// CHECK-LE-NEXT:    [[TMP0:%.*]] = tail call <512 x i1> @llvm.ppc.mma.assemble.acc(<16 x i8> [[VC4:%.*]], <16 x i8> [[VC3:%.*]], <16 x i8> [[VC2:%.*]], <16 x i8> [[VC1:%.*]])
+// CHECK-LE-NEXT:    [[TMP1:%.*]] = bitcast i8* [[RESP:%.*]] to <512 x i1>*
+// CHECK-LE-NEXT:    store <512 x i1> [[TMP0]], <512 x i1>* [[TMP1]], align 64, !tbaa [[TBAA2:![0-9]+]]
+// CHECK-LE-NEXT:    ret void
+//
+// CHECK-BE-LABEL: @test1(
+// CHECK-BE-NEXT:  entry:
+// CHECK-BE-NEXT:    [[TMP0:%.*]] = tail call <512 x i1> @llvm.ppc.mma.assemble.acc(<16 x i8> [[VC1:%.*]], <16 x i8> [[VC2:%.*]], <16 x i8> [[VC3:%.*]], <16 x i8> [[VC4:%.*]])
+// CHECK-BE-NEXT:    [[TMP1:%.*]] = bitcast i8* [[RESP:%.*]] to <512 x i1>*
+// CHECK-BE-NEXT:    store <512 x i1> [[TMP0]], <512 x i1>* [[TMP1]], align 64, !tbaa [[TBAA2:![0-9]+]]
+// CHECK-BE-NEXT:    ret void
+//
+void test1(unsigned char *vqp, unsigned char *vpp, vector unsigned char vc1, vector unsigned char vc2,
+            vector unsigned char vc3, vector unsigned char vc4, unsigned char *resp) {
+  __vector_quad vq = *((__vector_quad *)vqp);
+  __vector_pair vp = *((__vector_pair *)vpp);
+  __vector_quad res;
+  __builtin_mma_build_acc(&res, vc1, vc2, vc3, vc4);
+  *((__vector_quad *)resp) = res;
+}
+
+// CHECK-LE-LABEL: @test2(
+// CHECK-LE-NEXT:  entry:
+// CHECK-LE-NEXT:    [[TMP0:%.*]] = tail call <256 x i1> @llvm.ppc.vsx.assemble.pair(<16 x i8> [[VC2:%.*]], <16 x i8> [[VC1:%.*]])
+// CHECK-LE-NEXT:    [[TMP1:%.*]] = bitcast i8* [[RESP:%.*]] to <256 x i1>*
+// CHECK-LE-NEXT:    store <256 x i1> [[TMP0]], <256 x i1>* [[TMP1]], align 32, !tbaa [[TBAA6:![0-9]+]]
+// CHECK-LE-NEXT:    ret void
+//
+// CHECK-BE-LABEL: @test2(
+// CHECK-BE-NEXT:  entry:
+// CHECK-BE-NEXT:    [[TMP0:%.*]] = tail call <256 x i1> @llvm.ppc.vsx.assemble.pair(<16 x i8> [[VC1:%.*]], <16 x i8> [[VC2:%.*]])
+// CHECK-BE-NEXT:    [[TMP1:%.*]] = bitcast i8* [[RESP:%.*]] to <256 x i1>*
+// CHECK-BE-NEXT:    store <256 x i1> [[TMP0]], <256 x i1>* [[TMP1]], align 32, !tbaa [[TBAA6:![0-9]+]]
+// CHECK-BE-NEXT:    ret void
+//
+void test2(unsigned char *vqp, unsigned char *vpp, vector unsigned char vc1,
+            vector unsigned char vc2, unsigned char *resp) {
+  __vector_quad vq = *((__vector_quad *)vqp);
+  __vector_pair vp = *((__vector_pair *)vpp);
+  __vector_pair res;
+  __builtin_vsx_build_pair(&res, vc1, vc2);
+  *((__vector_pair *)resp) = res;
+}

clang/test/Sema/ppc-pair-mma-types.c

 // RUN: %clang_cc1 -triple powerpc64le-unknown-unknown -fsyntax-only \
 // RUN:   -target-cpu pwr10 %s -verify
 // RUN: %clang_cc1 -triple powerpc64-unknown-unknown -fsyntax-only \
 // RUN:   -target-cpu pwr10 %s -verify
 
 // The use of PPC MMA types is strongly restricted. Non-pointer MMA variables
 // can only be declared in functions and a limited number of operations are
 // supported on these types. This test case checks that invalid uses of MMA
 // types are correctly prevented.
 
 // vector quad
 

 vp_t globalvp_t; // expected-error {{invalid use of PPC MMA type}}
 
 // local
 void testVPLocal(int *ptr, vector unsigned char vc) {
   __vector_pair *vpp = (__vector_pair *)ptr;
   __vector_pair vp1 = *vpp;
   __vector_pair vp2;
   __builtin_vsx_assemble_pair(&vp2, vc, vc);
+  __builtin_vsx_build_pair(&vp2, vc, vc);
   __vector_pair vp3;
   __vector_quad vq;
   __builtin_mma_xvf64ger(&vq, vp3, vc);
   *vpp = vp3;
 }
 
 // struct field
 struct TestVPStruct {
   int a;
   float b;
   __vector_pair c; // expected-error {{invalid use of PPC MMA type}}
   __vector_pair *vp;
 };
 
 // sizeof / alignof
 int testVPSizeofAlignof(int *ptr) {
   __vector_pair *vpp = (__vector_pair *)ptr;
   __vector_pair vp = *vpp;
   unsigned sizet = sizeof(__vector_pair);
   unsigned alignt = __alignof__(__vector_pair);
   unsigned sizev = sizeof(vp);
   unsigned alignv = __alignof__(vp);
   return sizet + alignt + sizev + alignv;
 }

clang/test/SemaCXX/ppc-pair-mma-types.cpp

 // RUN: %clang_cc1 -triple powerpc64le-unknown-unknown -fsyntax-only \
 // RUN:   -fcxx-exceptions -target-cpu pwr10 %s -verify
 // RUN: %clang_cc1 -triple powerpc64-unknown-unknown -fsyntax-only \
 // RUN:   -fcxx-exceptions -target-cpu pwr10 %s -verify
 
 // vector quad
 
 // alias
 using vq_t = __vector_quad;
 void testVQAlias(int *inp, int *outp) {
   vq_t *vqin = (vq_t *)inp;
   vq_t *vqout = (vq_t *)outp;
auto f1 = [](void *ptr) -> __vector_pair {
     __vector_pair *vpp = (__vector_pair *)ptr;
     return *vpp; // expected-error {{invalid use of PPC MMA type}}
   };
   auto f2 = [](void *ptr) {
     __vector_pair *vpp = (__vector_pair *)ptr;
     return *vpp; // expected-error {{invalid use of PPC MMA type}}
   };
   auto f3 = [](vector unsigned char vc) { __vector_pair vp; __builtin_vsx_assemble_pair(&vp, vc, vc); return vp; }; // expected-error {{invalid use of PPC MMA type}}
+  auto f4 = [](vector unsigned char vc) { __vector_pair vp; __builtin_vsx_build_pair(&vp, vc, vc); return vp; }; // expected-error {{invalid use of PPC MMA type}}
 }
 
 // cast
 void TestVPCast() {
   __vector_pair vp;
   int *ip = reinterpret_cast<int *>(&vp);
   __vector_pair *vp2 = reinterpret_cast<__vector_pair *>(ip);
 }
 
 // throw
 void TestVPThrow() {
   __vector_pair vp;
   throw vp; // expected-error {{invalid use of PPC MMA type}}
 }