[ArgPromotion][TTI] Pass types to ABI compatibility hook

[nikic]

Summary

The areFunctionArgsABICompatible() hook currently accepts a list of pointer arguments, though what we're actually interested in is the ABI compatibility after these pointer arguments have been converted into value arguments.

This means that a) the current API is incompatible with opaque pointers (because it requires inspection of pointee types) and b) it can only be used in the specific context of ArgPromotion. I would like to reuse the API when inspecting calls during inlining.

This patch converts it into an areTypesABICompatible() hook, which accepts a list of types. This makes the method more generally usable, and compatible with opaque pointers from an API perspective (the actual usage in ArgPromotion/Attributor is still incompatible, I'll follow up on that in a separate patch).

[dblaikie]

Sounds OK. Another possibility might be splitting the operations - one to test function compatibility (eg: the subtarget AVX checks in x86) and another to test a particular arg/return type. This would mean more virtual calls (one for each type instead of a batch - well, the API could still take ArrayRef for cases where you already have multiple types in a buffer) but wouldn't require allocating a new buffer to fill with types to then pass in? But maybe the number's usually low, so the buffer creation overhead isn't high?

[nikic]

In D116031#3205086, @dblaikie wrote:

Sounds OK. Another possibility might be splitting the operations - one to test function compatibility (eg: the subtarget AVX checks in x86) and another to test a particular arg/return type. This would mean more virtual calls (one for each type instead of a batch - well, the API could still take ArrayRef for cases where you already have multiple types in a buffer) but wouldn't require allocating a new buffer to fill with types to then pass in? But maybe the number's usually low, so the buffer creation overhead isn't high?

I think generally the check for target features and types does need to be combined, because the specific difference in target attributes decides whether the call ABI for a particular type changes or not. None of our existing implementations actually try to model things that precisely, but from a conceptual point of view these are interdependent. In any case, I don't think compile-time really matters for this API.

[dblaikie]

In D116031#3205388, @nikic wrote:

In D116031#3205086, @dblaikie wrote:

Sounds OK. Another possibility might be splitting the operations - one to test function compatibility (eg: the subtarget AVX checks in x86) and another to test a particular arg/return type. This would mean more virtual calls (one for each type instead of a batch - well, the API could still take ArrayRef for cases where you already have multiple types in a buffer) but wouldn't require allocating a new buffer to fill with types to then pass in? But maybe the number's usually low, so the buffer creation overhead isn't high?

I think generally the check for target features and types does need to be combined, because the specific difference in target attributes decides whether the call ABI for a particular type changes or not. None of our existing implementations actually try to model things that precisely, but from a conceptual point of view these are interdependent. In any case, I don't think compile-time really matters for this API.

Ah, sounds good!

[krasimir]

Added a dependent quick fix https://github.com/llvm/llvm-project/commit/37c8d5915f37432f49fd4b0e6ec318ae68750af1.

[nikic]

In D116031#3206098, @krasimir wrote:

Added a dependent quick fix https://github.com/llvm/llvm-project/commit/37c8d5915f37432f49fd4b0e6ec318ae68750af1.

Thanks!

llvm/include/llvm/Analysis/TargetTransformInfo.h

void getMemcpyLoopResidualLoweringType(
       unsigned RemainingBytes, unsigned SrcAddrSpace, unsigned DestAddrSpace,
       unsigned SrcAlign, unsigned DestAlign) const;
 
   /// \returns True if the two functions have compatible attributes for inlining
   /// purposes.
   bool areInlineCompatible(const Function *Caller,
                            const Function *Callee) const;
 
-  /// \returns True if the caller and callee agree on how \p Args will be passed
+  /// \returns True if the caller and callee agree on how \p Types will be
+  /// passed to or returned from the callee.
   /// to the callee.
-  /// \param[out] Args The list of compatible arguments.  The implementation may
-  /// filter out any incompatible args from this list.
-  bool areFunctionArgsABICompatible(const Function *Caller,
-                                    const Function *Callee,
-                                    SmallPtrSetImpl<Argument *> &Args) const;
+  /// \param Types List of types to check.
+  bool areTypesABICompatible(const Function *Caller, const Function *Callee,
+                             const ArrayRef<Type *> &Types) const;
 
   /// The type of load/store indexing.
   enum MemIndexedMode {
     MIM_Unindexed, ///< No indexing.
     MIM_PreInc,    ///< Pre-incrementing.
     MIM_PreDec,    ///< Pre-decrementing.
     MIM_PostInc,   ///< Post-incrementing.
     MIM_PostDec    ///< Post-decrementing.
virtual Type *getMemcpyLoopLoweringType(LLVMContext &Context, Value *Length,
                                           unsigned SrcAlign,
                                           unsigned DestAlign) const = 0;
   virtual void getMemcpyLoopResidualLoweringType(
       SmallVectorImpl<Type *> &OpsOut, LLVMContext &Context,
       unsigned RemainingBytes, unsigned SrcAddrSpace, unsigned DestAddrSpace,
       unsigned SrcAlign, unsigned DestAlign) const = 0;
   virtual bool areInlineCompatible(const Function *Caller,
                                    const Function *Callee) const = 0;
-  virtual bool
-  areFunctionArgsABICompatible(const Function *Caller, const Function *Callee,
-                               SmallPtrSetImpl<Argument *> &Args) const = 0;
+  virtual bool areTypesABICompatible(const Function *Caller,
+                                     const Function *Callee,
+                                     const ArrayRef<Type *> &Types) const;
   virtual bool isIndexedLoadLegal(MemIndexedMode Mode, Type *Ty) const = 0;
   virtual bool isIndexedStoreLegal(MemIndexedMode Mode, Type *Ty) const = 0;
   virtual unsigned getLoadStoreVecRegBitWidth(unsigned AddrSpace) const = 0;
   virtual bool isLegalToVectorizeLoad(LoadInst *LI) const = 0;
   virtual bool isLegalToVectorizeStore(StoreInst *SI) const = 0;
   virtual bool isLegalToVectorizeLoadChain(unsigned ChainSizeInBytes,
                                            Align Alignment,
                                            unsigned AddrSpace) const = 0;
void getMemcpyLoopResidualLoweringType(
     Impl.getMemcpyLoopResidualLoweringType(OpsOut, Context, RemainingBytes,
                                            SrcAddrSpace, DestAddrSpace,
                                            SrcAlign, DestAlign);
   }
   bool areInlineCompatible(const Function *Caller,
                            const Function *Callee) const override {
     return Impl.areInlineCompatible(Caller, Callee);
   }
-  bool areFunctionArgsABICompatible(
-      const Function *Caller, const Function *Callee,
-      SmallPtrSetImpl<Argument *> &Args) const override {
-    return Impl.areFunctionArgsABICompatible(Caller, Callee, Args);
+  bool areTypesABICompatible(const Function *Caller, const Function *Callee,
+                             const ArrayRef<Type *> &Types) const override {
+    return Impl.areTypesABICompatible(Caller, Callee, Types);
   }
   bool isIndexedLoadLegal(MemIndexedMode Mode, Type *Ty) const override {
     return Impl.isIndexedLoadLegal(Mode, Ty, getDataLayout());
   }
   bool isIndexedStoreLegal(MemIndexedMode Mode, Type *Ty) const override {
     return Impl.isIndexedStoreLegal(Mode, Ty, getDataLayout());
   }
   unsigned getLoadStoreVecRegBitWidth(unsigned AddrSpace) const override {

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

public:
   bool areInlineCompatible(const Function *Caller,
                            const Function *Callee) const {
     return (Caller->getFnAttribute("target-cpu") ==
             Callee->getFnAttribute("target-cpu")) &&
            (Caller->getFnAttribute("target-features") ==
             Callee->getFnAttribute("target-features"));
   }
 
-  bool areFunctionArgsABICompatible(const Function *Caller,
-                                    const Function *Callee,
-                                    SmallPtrSetImpl<Argument *> &Args) const {
+  bool areTypesABICompatible(const Function *Caller, const Function *Callee,
+                             const ArrayRef<Type *> &Types) const {
     return (Caller->getFnAttribute("target-cpu") ==
             Callee->getFnAttribute("target-cpu")) &&
            (Caller->getFnAttribute("target-features") ==
             Callee->getFnAttribute("target-features"));
   }
 
   bool isIndexedLoadLegal(TTI::MemIndexedMode Mode, Type *Ty,
                           const DataLayout &DL) const {

llvm/lib/Analysis/TargetTransformInfo.cpp

TTIImpl->getMemcpyLoopResidualLoweringType(OpsOut, Context, RemainingBytes,
                                              SrcAlign, DestAlign);
 }
 
 bool TargetTransformInfo::areInlineCompatible(const Function *Caller,
                                               const Function *Callee) const {
   return TTIImpl->areInlineCompatible(Caller, Callee);
 }
 
-bool TargetTransformInfo::areFunctionArgsABICompatible(
+bool TargetTransformInfo::areTypesABICompatible(
     const Function *Caller, const Function *Callee,
-    SmallPtrSetImpl<Argument *> &Args) const {
-  return TTIImpl->areFunctionArgsABICompatible(Caller, Callee, Args);
+    const ArrayRef<Type *> &Types) const {
+  return TTIImpl->areTypesABICompatible(Caller, Callee, Types);
 }
 
 bool TargetTransformInfo::isIndexedLoadLegal(MemIndexedMode Mode,
                                              Type *Ty) const {
   return TTIImpl->isIndexedLoadLegal(Mode, Ty);
 }
 
 bool TargetTransformInfo::isIndexedStoreLegal(MemIndexedMode Mode,

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.h

InstructionCost getMemoryOpCost(unsigned Opcode, Type *Src,
                                   TTI::TargetCostKind CostKind,
                                   const Instruction *I = nullptr);
   InstructionCost getInterleavedMemoryOpCost(
       unsigned Opcode, Type *VecTy, unsigned Factor, ArrayRef<unsigned> Indices,
       Align Alignment, unsigned AddressSpace, TTI::TargetCostKind CostKind,
       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;
+  bool areTypesABICompatible(const Function *Caller, const Function *Callee,
+                             const ArrayRef<Type *> &Types) const;
   bool hasActiveVectorLength(unsigned Opcode, Type *DataType,
                              Align Alignment) const;
   InstructionCost getVPMemoryOpCost(unsigned Opcode, Type *Src, Align Alignment,
                                     unsigned AddressSpace,
                                     TTI::TargetCostKind CostKind,
                                     const Instruction *I = nullptr);
 
 private:

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 {
+bool PPCTTIImpl::areTypesABICompatible(const Function *Caller,
+                                       const Function *Callee,
+                                       const ArrayRef<Type *> &Types) const {
 
   // 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.
-  if (!BaseT::areFunctionArgsABICompatible(Caller, Callee, Args))
+  if (!BaseT::areTypesABICompatible(Caller, Callee, Types))
     return false;
 
-  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 llvm::none_of(Types, [](Type *Ty) {
+    if (Ty->isSized())
+      return Ty->isIntOrIntVectorTy(1) && Ty->getPrimitiveSizeInBits() > 128;
     return false;
   });
 }
 
 bool PPCTTIImpl::canSaveCmp(Loop *L, BranchInst **BI, ScalarEvolution *SE,
                             LoopInfo *LI, DominatorTree *DT,
                             AssumptionCache *AC, TargetLibraryInfo *LibInfo) {
   // Process nested loops first.

llvm/lib/Target/X86/X86TargetTransformInfo.h

public:
   bool isLegalMaskedGather(Type *DataType, Align Alignment);
   bool isLegalMaskedScatter(Type *DataType, Align Alignment);
   bool isLegalMaskedExpandLoad(Type *DataType);
   bool isLegalMaskedCompressStore(Type *DataType);
   bool hasDivRemOp(Type *DataType, bool IsSigned);
   bool isFCmpOrdCheaperThanFCmpZero(Type *Ty);
   bool areInlineCompatible(const Function *Caller,
                            const Function *Callee) const;
-  bool areFunctionArgsABICompatible(const Function *Caller,
-                                    const Function *Callee,
-                                    SmallPtrSetImpl<Argument *> &Args) const;
+  bool areTypesABICompatible(const Function *Caller, const Function *Callee,
+                             const ArrayRef<Type *> &Type) const;
   TTI::MemCmpExpansionOptions enableMemCmpExpansion(bool OptSize,
                                                     bool IsZeroCmp) const;
   bool prefersVectorizedAddressing() const;
   bool supportsEfficientVectorElementLoadStore() const;
   bool enableInterleavedAccessVectorization();
 
 private:
   bool supportsGather() const;

llvm/lib/Target/X86/X86TargetTransformInfo.cpp

bool X86TTIImpl::areInlineCompatible(const Function *Caller,
   const FeatureBitset &CalleeBits =
       TM.getSubtargetImpl(*Callee)->getFeatureBits();
 
   FeatureBitset RealCallerBits = CallerBits & ~InlineFeatureIgnoreList;
   FeatureBitset RealCalleeBits = CalleeBits & ~InlineFeatureIgnoreList;
   return (RealCallerBits & RealCalleeBits) == RealCalleeBits;
 }
 
-bool X86TTIImpl::areFunctionArgsABICompatible(
-    const Function *Caller, const Function *Callee,
-    SmallPtrSetImpl<Argument *> &Args) const {
-  if (!BaseT::areFunctionArgsABICompatible(Caller, Callee, Args))
+bool X86TTIImpl::areTypesABICompatible(const Function *Caller,
+                                       const Function *Callee,
+                                       const ArrayRef<Type *> &Types) const {
+  if (!BaseT::areTypesABICompatible(Caller, Callee, Types))
     return false;
 
   // If we get here, we know the target features match. If one function
   // considers 512-bit vectors legal and the other does not, consider them
   // incompatible.
   const TargetMachine &TM = getTLI()->getTargetMachine();
 
   if (TM.getSubtarget<X86Subtarget>(*Caller).useAVX512Regs() ==
       TM.getSubtarget<X86Subtarget>(*Callee).useAVX512Regs())
     return true;
 
   // Consider the arguments compatible if they aren't vectors or aggregates.
   // FIXME: Look at the size of vectors.
   // FIXME: Look at the element types of aggregates to see if there are vectors.
-  // FIXME: The API of this function seems intended to allow arguments
-  // to be removed from the set, but the caller doesn't check if the set
-  // becomes empty so that may not work in practice.
-  return llvm::none_of(Args, [](Argument *A) {
-    auto *EltTy = cast<PointerType>(A->getType())->getElementType();
-    return EltTy->isVectorTy() || EltTy->isAggregateType();
-  });
+  return llvm::none_of(Types,
+      [](Type *T) { return T->isVectorTy() || T->isAggregateType(); });
 }
 
 X86TTIImpl::TTI::MemCmpExpansionOptions
 X86TTIImpl::enableMemCmpExpansion(bool OptSize, bool IsZeroCmp) const {
   TTI::MemCmpExpansionOptions Options;
   Options.MaxNumLoads = TLI->getMaxExpandSizeMemcmp(OptSize);
   Options.NumLoadsPerBlock = 2;
   // All GPR and vector loads can be unaligned.

llvm/lib/Transforms/IPO/ArgumentPromotion.cpp

static bool canPaddingBeAccessed(Argument *arg) {
 
   return false;
 }
 
 bool ArgumentPromotionPass::areFunctionArgsABICompatible(
     const Function &F, const TargetTransformInfo &TTI,
     SmallPtrSetImpl<Argument *> &ArgsToPromote,
     SmallPtrSetImpl<Argument *> &ByValArgsToTransform) {
+  // TODO: Check individual arguments so we can promote a subset?
+  SmallVector<Type *, 32> Types;
+  for (Argument *Arg : ArgsToPromote)
+    Types.push_back(Arg->getType()->getPointerElementType());
+  for (Argument *Arg : ByValArgsToTransform)
+    Types.push_back(Arg->getParamByValType());
+
   for (const Use &U : F.uses()) {
     CallBase *CB = dyn_cast<CallBase>(U.getUser());
     if (!CB)
       return false;
     const Function *Caller = CB->getCaller();
     const Function *Callee = CB->getCalledFunction();
-    if (!TTI.areFunctionArgsABICompatible(Caller, Callee, ArgsToPromote) ||
-        !TTI.areFunctionArgsABICompatible(Caller, Callee, ByValArgsToTransform))
+    if (!TTI.areTypesABICompatible(Caller, Callee, Types))
       return false;
   }
   return true;
 }
 
 /// PromoteArguments - This method checks the specified function to see if there
 /// are any promotable arguments and if it is safe to promote the function (for
 /// example, all callers are direct).  If safe to promote some arguments, it