TTI: Use a better default for areInlineCompatibl

[arsenm]

Summary

AArch64 and X86 do the exact same thing for this.
Rather than requiring the target-cpu exactly matches,
if target info is available check subtarget features.

Fixes not inlining OpenCL library functions on AMDGPU,
which don't have an explicitly set target-cpu.

[fhahn]

include/llvm/CodeGen/BasicTTIImpl.h

1092

I think this implementation is not conservative enough for some targets. For example, the ARM backend has a more conservative implementation https://github.com/llvm-mirror/llvm/blob/master/lib/Target/ARM/ARMTargetTransformInfo.cpp#L18

Some target-features in the ARM backend have impact on the generated code (e.g. thumb-mode). So if we allow inlining for all subsets, we could change the "thumbness" of the inline function. Other backends might have similar tricky target-features to deal with.

[MatzeB]

I also think this patch is too optimistic. Not comparing the CPU name/feature attributes seems daring but doable to me. But I assuming all features are additive so we just need to check whether the callees flags are a subset of the callers flags seems too optimistic to me.

include/llvm/Analysis/TargetTransformInfoImpl.h

void getMemcpyLoopResidualLoweringType(SmallVectorImpl<Type *> &OpsOut,
                                          unsigned SrcAlign,
                                          unsigned DestAlign) const {
     for (unsigned i = 0; i != RemainingBytes; ++i)
       OpsOut.push_back(Type::getInt8Ty(Context));
   }
 
   bool areInlineCompatible(const Function *Caller,
                            const Function *Callee) const {
+    // If there is no target machine, be very conservative.
     return (Caller->getFnAttribute("target-cpu") ==
             Callee->getFnAttribute("target-cpu")) &&
            (Caller->getFnAttribute("target-features") ==
             Callee->getFnAttribute("target-features"));
   }
 
   unsigned getLoadStoreVecRegBitWidth(unsigned AddrSpace) const { return 128; }
 

include/llvm/CodeGen/BasicTTIImpl.h

   /// \param F Called function, might be nullptr.
   /// \param RetTy Return value types.
   /// \param Tys Argument types.
   /// \returns The cost of Call instruction.
   unsigned getCallInstrCost(Function *F, Type *RetTy, ArrayRef<Type *> Tys) {
     return 10;
   }
 
+  bool areInlineCompatible(const Function *Caller,
+                           const Function *Callee) const {
+    const TargetMachine &TM = getTLI()->getTargetMachine();
+
+    const FeatureBitset &CallerBits =
+      TM.getSubtargetImpl(*Caller)->getFeatureBits();
+    const FeatureBitset &CalleeBits =
+      TM.getSubtargetImpl(*Callee)->getFeatureBits();
+
+    // Inline a callee if its target-features are a subset of the callers
+    // target-features.
+    //
+    // Targets can override if this is too limiting by including subtarget
+    // features that we might not care about for inlining, but it is
+    // conservatively correct.
+    return (CallerBits & CalleeBits) == CalleeBits;
+  }
+
   unsigned getNumberOfParts(Type *Tp) {
     std::pair<unsigned, MVT> LT = getTLI()->getTypeLegalizationCost(DL, Tp);
     return LT.first;
   }
 
   unsigned getAddressComputationCost(Type *Ty, ScalarEvolution *,
                                      const SCEV *) {
     return 0;

lib/Target/AArch64/AArch64TargetTransformInfo.h

class AArch64TTIImpl : public BasicTTIImplBase<AArch64TTIImpl> {
   bool isWideningInstruction(Type *Ty, unsigned Opcode,
                              ArrayRef<const Value *> Args);
 
 public:
   explicit AArch64TTIImpl(const AArch64TargetMachine *TM, const Function &F)
       : BaseT(TM, F.getParent()->getDataLayout()), ST(TM->getSubtargetImpl(F)),
         TLI(ST->getTargetLowering()) {}
 
-  bool areInlineCompatible(const Function *Caller,
-                           const Function *Callee) const;
-
   /// \name Scalar TTI Implementations
   /// @{
 
   using BaseT::getIntImmCost;
   int getIntImmCost(int64_t Val);
   int getIntImmCost(const APInt &Imm, Type *Ty);
   int getIntImmCost(unsigned Opcode, unsigned Idx, const APInt &Imm, Type *Ty);
   int getIntImmCost(Intrinsic::ID IID, unsigned Idx, const APInt &Imm,

lib/Target/AArch64/AArch64TargetTransformInfo.cpp

 #include <algorithm>
 using namespace llvm;
 
 #define DEBUG_TYPE "aarch64tti"
 
 static cl::opt<bool> EnableFalkorHWPFUnrollFix("enable-falkor-hwpf-unroll-fix",
                                                cl::init(true), cl::Hidden);
 
-bool AArch64TTIImpl::areInlineCompatible(const Function *Caller,
-                                         const Function *Callee) const {
-  const TargetMachine &TM = getTLI()->getTargetMachine();
-
-  const FeatureBitset &CallerBits =
-      TM.getSubtargetImpl(*Caller)->getFeatureBits();
-  const FeatureBitset &CalleeBits =
-      TM.getSubtargetImpl(*Callee)->getFeatureBits();
-
-  // Inline a callee if its target-features are a subset of the callers
-  // target-features.
-  return (CallerBits & CalleeBits) == CalleeBits;
-}
-
 /// \brief Calculate the cost of materializing a 64-bit value. This helper
 /// method might only calculate a fraction of a larger immediate. Therefore it
 /// is valid to return a cost of ZERO.
 int AArch64TTIImpl::getIntImmCost(int64_t Val) {
   // Check if the immediate can be encoded within an instruction.
   if (Val == 0 || AArch64_AM::isLogicalImmediate(Val, 64))
     return 0;
 

lib/Target/X86/X86TargetTransformInfo.h

public:
 
   int getIntImmCost(unsigned Opcode, unsigned Idx, const APInt &Imm, Type *Ty);
   int getIntImmCost(Intrinsic::ID IID, unsigned Idx, const APInt &Imm,
                     Type *Ty);
   bool isLegalMaskedLoad(Type *DataType);
   bool isLegalMaskedStore(Type *DataType);
   bool isLegalMaskedGather(Type *DataType);
   bool isLegalMaskedScatter(Type *DataType);
-  bool areInlineCompatible(const Function *Caller,
-                           const Function *Callee) const;
   bool expandMemCmp(Instruction *I, unsigned &MaxLoadSize);
   bool enableInterleavedAccessVectorization();
 private:
   int getGSScalarCost(unsigned Opcode, Type *DataTy, bool VariableMask,
                       unsigned Alignment, unsigned AddressSpace);
   int getGSVectorCost(unsigned Opcode, Type *DataTy, Value *Ptr,
                       unsigned Alignment, unsigned AddressSpace);
 
   /// @}
 };
 
 } // end namespace llvm
 
 #endif

lib/Target/X86/X86TargetTransformInfo.cpp

bool X86TTIImpl::isLegalMaskedGather(Type *DataTy) {
   // AVX-512 allows gather and scatter
   return (DataWidth == 32 || DataWidth == 64) && ST->hasAVX512();
 }
 
 bool X86TTIImpl::isLegalMaskedScatter(Type *DataType) {
   return isLegalMaskedGather(DataType);
 }
 
-bool X86TTIImpl::areInlineCompatible(const Function *Caller,
-                                     const Function *Callee) const {
-  const TargetMachine &TM = getTLI()->getTargetMachine();
-
-  // Work this as a subsetting of subtarget features.
-  const FeatureBitset &CallerBits =
-      TM.getSubtargetImpl(*Caller)->getFeatureBits();
-  const FeatureBitset &CalleeBits =
-      TM.getSubtargetImpl(*Callee)->getFeatureBits();
-
-  // FIXME: This is likely too limiting as it will include subtarget features
-  // that we might not care about for inlining, but it is conservatively
-  // correct.
-  return (CallerBits & CalleeBits) == CalleeBits;
-}
-
 bool X86TTIImpl::expandMemCmp(Instruction *I, unsigned &MaxLoadSize) {
   // TODO: We can increase these based on available vector ops.
   MaxLoadSize = ST->is64Bit() ? 8 : 4;
   return true;
 }
 
 bool X86TTIImpl::enableInterleavedAccessVectorization() {
   // TODO: We expect this to be beneficial regardless of arch,

test/Transforms/Inline/AMDGPU/inline-target-cpu.ll

+; RUN: opt -mtriple=amdgcn-amd-amdhsa -S -inline < %s | FileCheck %s
+; RUN: opt -mtriple=amdgcn-amd-amdhsa -S -passes='cgscc(inline)' < %s | FileCheck %s
+
+; CHECK-LABEL: @func_no_target_cpu(
+define i32 @func_no_target_cpu() #0 {
+  ret i32 0
+}
+
+; CHECK-LABEL: @target_cpu_call_no_target_cpu(
+; CHECK-NEXT: ret i32 0
+define i32 @target_cpu_call_no_target_cpu() #1 {
+  %call = call i32 @func_no_target_cpu()
+  ret i32 %call
+}
+
+; CHECK-LABEL: @target_cpu_target_features_call_no_target_cpu(
+; CHECK-NEXT: ret i32 0
+define i32 @target_cpu_target_features_call_no_target_cpu() #2 {
+  %call = call i32 @func_no_target_cpu()
+  ret i32 %call
+}
+
+; CHECK-LABEL: @fp32_denormals(
+define i32 @fp32_denormals() #3 {
+  ret i32 0
+}
+
+; CHECK-LABEL: @no_fp32_denormals_call_f32_denormals(
+; CHECK-NEXT: call i32 @fp32_denormals()
+define i32 @no_fp32_denormals_call_f32_denormals() #4 {
+  %call = call i32 @fp32_denormals()
+  ret i32 %call
+}
+
+; Make sure gfx9 can call unspecified functions because of movrel
+; feature change.
+; CHECK-LABEL: @gfx9_target_features_call_no_target_cpu(
+; CHECK-NEXT: ret i32 0
+define i32 @gfx9_target_features_call_no_target_cpu() #5 {
+  %call = call i32 @func_no_target_cpu()
+  ret i32 %call
+}
+
+attributes #0 = { nounwind }
+attributes #1 = { nounwind "target-cpu"="fiji" }
+attributes #2 = { nounwind "target-cpu"="fiji" "target-features"="+fp32-denormals" }
+attributes #3 = { nounwind "target-features"="+fp32-denormals" }
+attributes #4 = { nounwind "target-features"="-fp32-denormals" }
+attributes #5 = { nounwind "target-cpu"="gfx900" }

test/Transforms/Inline/AMDGPU/lit.local.cfg

+if not 'AMDGPU' in config.root.targets:
+    config.unsupported = True