[PowerPC] Prevent argument promotion of types with size greater than 128 bits

[saghir]

Summary

This patch prevents argument promotion of types having
type size greater than 128 bits.

Fixes Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=49952

[saghir]

Fixed typo in commit message.

[amyk]

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp

1232

Would it make sense to add some documentation, and perhaps add why we check up to 128 bits?

[nemanjai]

This will turn off argument promotion for all types wider than 128 bits. An example of a test case that might suffer from worse codegen:

#include <stdio.h>
typedef int __attribute__((vector_size(64))) v8si;
static void __attribute__((noinline)) printWideVec(v8si *ptr) {
  v8si WideVec = *ptr;
  printf("Vector: { ");
  for (int i = 0; i < 7; i++)
    printf("%d, ", WideVec[i]);
  printf("%d }\n", WideVec[7]);
}

void test(vector signed int a, vector signed int b) {
  v8si WideVec = (v8si) { a[0], a[1], a[2], a[3], b[0], b[1], b[2], b[3] };
  printWideVec(&WideVec);
}

Since we can actually check that the type is <N x i1> for N > 128, we should do that and ensure we are only disabling arg promotion on the MMA types.

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp

1223

// We need to ensure that argument promotion does not
// attempt to promote pointers to MMA types (__vector_pair
// and __vector_quad) since these types explicitly cannot be
// passed as arguments. Both of these types are larger than
// the 128-bit Altivec vectors and have a scalar size of 1 bit.

1229–1240

I don't think we need to check the type size in the data layout for this. The primitive type size should be sufficient. This can be simplified to:

return llvm::none_of(Args, [](Argument *A) {
  auto *EltTy = cast<PointerType>(A->getType())->getElementType();
  if (EltTy->isSized())
    return (EltTy->isIntOrIntVectorTy(1)) &&
           EltTy->getPrimitiveSizeInBits() > 128;
  return false;
});

[nemanjai]

Also, this is another one we should merge to 12.0.1 so please open the bug, mark it as a blocker and link it here.

[saghir]

Addressed review comments to check for primitive type size.

[nemanjai]

In the previous revision I suggested the following test case:

#include <stdio.h>
typedef int __attribute__((vector_size(64))) v8si;
static void __attribute__((noinline)) printWideVec(v8si *ptr) {
  v8si WideVec = *ptr;
  printf("Vector: { ");
  for (int i = 0; i < 7; i++)
    printf("%d, ", WideVec[i]);
  printf("%d }\n", WideVec[7]);
}

void test(vector signed int a, vector signed int b) {
  v8si WideVec = (v8si) { a[0], a[1], a[2], a[3], b[0], b[1], b[2], b[3] };
  printWideVec(&WideVec);
}

Can you please add it to this new test case to show that argument promotion is still happening for that case?

[saghir]

Added another test case

[saghir]

Rebased before committing

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.h

InstructionCost getMemoryOpCost(unsigned Opcode, Type *Src,
                                   const Instruction *I = nullptr);
   InstructionCost getInterleavedMemoryOpCost(
       unsigned Opcode, Type *VecTy, unsigned Factor, ArrayRef<unsigned> Indices,
       Align Alignment, unsigned AddressSpace,
       TTI::TargetCostKind CostKind = TTI::TCK_SizeAndLatency,
       bool UseMaskForCond = false, bool UseMaskForGaps = false);
   InstructionCost getIntrinsicInstrCost(const IntrinsicCostAttributes &ICA,
                                         TTI::TargetCostKind CostKind);
-
+  bool areFunctionArgsABICompatible(const Function *Caller,
+                                    const Function *Callee,
+                                    SmallPtrSetImpl<Argument *> &Args) const;
   /// @}
 };
 
 } // end namespace llvm
 
 #endif

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp

 }
 
 InstructionCost
 PPCTTIImpl::getIntrinsicInstrCost(const IntrinsicCostAttributes &ICA,
                                   TTI::TargetCostKind CostKind) {
   return BaseT::getIntrinsicInstrCost(ICA, CostKind);
 }
 
+bool PPCTTIImpl::areFunctionArgsABICompatible(
+    const Function *Caller, const Function *Callee,
+    SmallPtrSetImpl<Argument *> &Args) const {
+  if (!BaseT::areFunctionArgsABICompatible(Caller, Callee, Args))
+    return false;
+
+  const DataLayout &CallerDL = Caller->getParent()->getDataLayout();
+  const DataLayout &CalleeDL = Callee->getParent()->getDataLayout();
+
+  return llvm::none_of(Args, [CallerDL, CalleeDL](Argument *A) {
+    auto *EltTy = cast<PointerType>(A->getType())->getElementType();
+    if (EltTy->isSized()) {
+      unsigned CallerTypeSize = CallerDL.getTypeSizeInBits(EltTy);
+      unsigned CalleeTypeSize = CalleeDL.getTypeSizeInBits(EltTy);
+      return (CallerTypeSize > 128 || CalleeTypeSize > 128);
+    }
+    return false;
+  });
+}
+
 bool PPCTTIImpl::canSaveCmp(Loop *L, BranchInst **BI, ScalarEvolution *SE,
                             LoopInfo *LI, DominatorTree *DT,
                             AssumptionCache *AC, TargetLibraryInfo *LibInfo) {
   // Process nested loops first.
   for (Loop::iterator I = L->begin(), E = L->end(); I != E; ++I)
     if (canSaveCmp(*I, BI, SE, LI, DT, AC, LibInfo))
       return false; // Stop search.
 

llvm/test/CodeGen/PowerPC/arg_promotion.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -argpromotion -mtriple=powerpc64le-unknown-linux-gnu < %s | FileCheck %s
+; RUN: opt -S -passes=argpromotion -mtriple=powerpc64le-unknown-linux-gnu < %s | FileCheck %s
+
+; Test to check that we do not promote arguments when the
+; type size is greater than 128 bits.
+
+define internal fastcc void @print_acc(<512 x i1>* nocapture readonly %a) nounwind {
+; CHECK-LABEL: @print_acc(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TMP0:%.*]] = load <512 x i1>, <512 x i1>* [[A:%.*]], align 64
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call { <16 x i8>, <16 x i8>, <16 x i8>, <16 x i8> } @llvm.ppc.mma.disassemble.acc(<512 x i1> [[TMP0]])
+; CHECK-NEXT:    [[TMP2:%.*]] = extractvalue { <16 x i8>, <16 x i8>, <16 x i8>, <16 x i8> } [[TMP1]], 0
+; CHECK-NEXT:    ret void
+;
+entry:
+  %0 = load <512 x i1>, <512 x i1>* %a, align 64
+  %1 = tail call { <16 x i8>, <16 x i8>, <16 x i8>, <16 x i8> } @llvm.ppc.mma.disassemble.acc(<512 x i1> %0)
+  %2 = extractvalue { <16 x i8>, <16 x i8>, <16 x i8>, <16 x i8> } %1, 0
+  ret void
+}
+
+declare { <16 x i8>, <16 x i8>, <16 x i8>, <16 x i8> } @llvm.ppc.mma.disassemble.acc(<512 x i1>) nounwind
+
+define dso_local void @test(<512 x i1>* nocapture %a, <16 x i8> %ac) {
+; CHECK-LABEL: @test(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TMP0:%.*]] = tail call <512 x i1> @llvm.ppc.mma.xvf32ger(<16 x i8> [[AC:%.*]], <16 x i8> [[AC]])
+; CHECK-NEXT:    store <512 x i1> [[TMP0]], <512 x i1>* [[A:%.*]], align 64
+; CHECK-NEXT:    tail call fastcc void @print_acc(<512 x i1>* nonnull [[A]])
+; CHECK-NEXT:    ret void
+;
+entry:
+  %0 = tail call <512 x i1> @llvm.ppc.mma.xvf32ger(<16 x i8> %ac, <16 x i8> %ac)
+  store <512 x i1> %0, <512 x i1>* %a, align 64
+  tail call fastcc void @print_acc(<512 x i1>* nonnull %a)
+  ret void
+}
+
+declare <512 x i1> @llvm.ppc.mma.xvf32ger(<16 x i8>, <16 x i8>) nounwind