Value soft float calls as more expensive in the inliner.

[dirty]

Summary

When evaluating floating point instructions in the inliner, ask the TTI whether it is an expensive operation. By default, it's not an expensive operation. This keeps the default behavior the same as before. The ARM TTI has been updated to return back TCC_Expensive for targets which don't have hardware floating point.

[dirty]

Ping

Cameron Esfahani
[e-mail]

"Americans are very skilled at creating a custom meaning from something that's mass-produced."

Ann Powers

[t.p.northover]

Hi Cameron,

We could probably do with some tests for this, and I think the ARM logic needs a bit of work.

lib/Target/ARM/ARMSubtarget.h

316–318

I don't think this is quite right. The relevant code in ARMISelLowering is:

if (!TM.Options.UseSoftFloat && Subtarget->hasVFP2() &&
    !Subtarget->isThumb1Only())

(notably: any CPU with float has VFP2; SoftFloat can be specified even if the hardware supports it; and Thumb1 didn't have access to VFP).

lib/Target/ARM/ARMTargetTransformInfo.cpp

318

I don't think this is quite right. The relevant code in ARMISelLowering is:

if (!TM.Options.UseSoftFloat && Subtarget->hasVFP2() &&

!Subtarget->isThumb1Only())

(notably: any CPU with float has VFP2; SoftFloat can be specified even if the hardware supports it; and Thumb1 didn't have access to VFP).

[dirty]

I have a unit test for this patch, but it's in the clang side, so I will submit it after this patch gets approved.

I've changed the code to adopt your suggestions. I'll send out an updated patch soon.

Cameron Esfahani
[e-mail]

"I cannot for the life of me understand why, while people without driver's licenses are not allowed on public roads, in bookstores one can find any number of books by persons without decency - let alone knowledge."

"His Master's Voice", Stanislaw Lem

[echristo]

... I'm curious how the testcase is on the clang side.

-eric

[dirty]

It's in the CodeGen directory. Here's the current testcase:

[dirty@girlyouknowitstrue work]$ cat llvm/tools/clang/test/CodeGen/inline-fp.c
RUN: %clang -Oz -target armv7-apple-macosx10.8.0 -mcpu=cortex-a8 -march=armv7 -S -emit-llvm %s -o - | FileCheck %s
RUN: %clang -Oz -target armv7-apple-macosx10.8.0 -mcpu=cortex-m3 -march=armv7 -S -emit-llvm %s -o - | FileCheck -check-prefix NO-FP %s

extern float powf(float x, float y);
extern float fabsf(float x);

static float f1(int response, unsigned char value1) {

float value2 = 2620.0f * powf(1.01f, value1 - 1);
float responseDelta = (response - value2) / value2;

return (responseDelta);

}

extern void getX(int *responseX, unsigned char *valueX);
extern void getY(int *responseY, unsigned char *valueY);
extern void getZ(int *responseZ, unsigned char *valueZ);

CHECK-NOT: define internal arm_aapcscc float @f1(
NO-FP: define internal arm_aapcscc float @f1(
int test_all(void) {

int responseX;
int responseY;
int responseZ;
unsigned char valueX;
unsigned char valueY;
unsigned char valueZ;

getX(&responseX, &valueX);
getY(&responseY, &valueY);
getZ(&responseZ, &valueZ);

float responseDeltaX = f1(responseX, valueX);
float responseDeltaY = f1(responseY, valueY);
float responseDeltaZ = f1(responseZ, valueZ);

int success = 1;

if (fabsf(responseDeltaX) > 0.14f)
  success = 0;
else if (fabsf(responseDeltaY) > 0.14f)
  success = 0;
else if (fabsf(responseDeltaZ) > 0.14f)
  success = 0;

return (success);

}

Cameron Esfahani
[e-mail]

"Americans are very skilled at creating a custom meaning from something that's mass-produced."

Ann Powers

[dirty]

Integrate patch suggestions from Tim Northover: use the same logic in ARMTTI as in ARMISelLowering.

[echristo]

Yeah, not so much. Testcases like that need to be in the backend. No
optimization testcases in the front end if we can help it. (i.e. testing
options)

Thanks!

-eric

[dirty]

Okay, I'll move the test case. Thanks for the feedback.

[dirty]

Update patch to account for routines which have the "use-soft-float" attribute. Add test case where we make sure that routines marked with "use-soft-float" are not inlined, but routines without that attribute are.

[dirty]

Update patch to work with Chandler's changes to TTI. Removed reference to Options.UseSoftFloat, as it's eventually going away and function attribute "use-soft-float" is more appropriate.

[echristo]

Can probably expand the testcase a bit more with CHECK/CHECK-NOT for the various calls? One inline comment.

Otherwise LGTM.

-eric

lib/Analysis/IPA/InlineCost.cpp

911

s/will/may.

[dirty]

Update test case with feedback from echristo.

[dirty]

Clean up test a little more: use CHECK-LABEL and move tests to relevant routines.

[echristo]

LGTM. Can you commit or do you need someone to commit for you?

-eric

include/llvm/Analysis/TargetTransformInfo.h

public:
   bool shouldBuildLookupTables() const;
 
   /// \brief Return hardware support for population count.
   PopcntSupportKind getPopcntSupport(unsigned IntTyWidthInBit) const;
 
   /// \brief Return true if the hardware has a fast square-root instruction.
   bool haveFastSqrt(Type *Ty) const;
 
+  /// \brief Return the expected cost of supporting the floating point operation
+  /// of the specified type.
+  unsigned getFPOpCost(Type *Ty) const;
+
   /// \brief Return the expected cost of materializing for the given integer
   /// immediate of the specified type.
   unsigned getIntImmCost(const APInt &Imm, Type *Ty) const;
 
   /// \brief Return the expected cost of materialization for the given integer
   /// immediate of the specified type for a given instruction. The cost can be
   /// zero if the immediate can be folded into the specified instruction.
   unsigned getIntImmCost(unsigned Opc, unsigned Idx, const APInt &Imm,
virtual int getScalingFactorCost(Type *Ty, GlobalValue *BaseGV,
                                    int64_t Scale) = 0;
   virtual bool isTruncateFree(Type *Ty1, Type *Ty2) = 0;
   virtual bool isTypeLegal(Type *Ty) = 0;
   virtual unsigned getJumpBufAlignment() = 0;
   virtual unsigned getJumpBufSize() = 0;
   virtual bool shouldBuildLookupTables() = 0;
   virtual PopcntSupportKind getPopcntSupport(unsigned IntTyWidthInBit) = 0;
   virtual bool haveFastSqrt(Type *Ty) = 0;
+  virtual unsigned getFPOpCost(Type *Ty) = 0;
   virtual unsigned getIntImmCost(const APInt &Imm, Type *Ty) = 0;
   virtual unsigned getIntImmCost(unsigned Opc, unsigned Idx, const APInt &Imm,
                                  Type *Ty) = 0;
   virtual unsigned getIntImmCost(Intrinsic::ID IID, unsigned Idx,
                                  const APInt &Imm, Type *Ty) = 0;
   virtual unsigned getNumberOfRegisters(bool Vector) = 0;
   virtual unsigned getRegisterBitWidth(bool Vector) = 0;
   virtual unsigned getMaxInterleaveFactor() = 0;
public:
   unsigned getJumpBufSize() override { return Impl.getJumpBufSize(); }
   bool shouldBuildLookupTables() override {
     return Impl.shouldBuildLookupTables();
   }
   PopcntSupportKind getPopcntSupport(unsigned IntTyWidthInBit) override {
     return Impl.getPopcntSupport(IntTyWidthInBit);
   }
   bool haveFastSqrt(Type *Ty) override { return Impl.haveFastSqrt(Ty); }
+
+  unsigned getFPOpCost(Type *Ty) override {
+    return Impl.getFPOpCost(Ty);
+  }
+
   unsigned getIntImmCost(const APInt &Imm, Type *Ty) override {
     return Impl.getIntImmCost(Imm, Ty);
   }
   unsigned getIntImmCost(unsigned Opc, unsigned Idx, const APInt &Imm,
                          Type *Ty) override {
     return Impl.getIntImmCost(Opc, Idx, Imm, Ty);
   }
   unsigned getIntImmCost(Intrinsic::ID IID, unsigned Idx, const APInt &Imm,

include/llvm/Analysis/TargetTransformInfoImpl.h

public:
   bool shouldBuildLookupTables() { return true; }
 
   TTI::PopcntSupportKind getPopcntSupport(unsigned IntTyWidthInBit) {
     return TTI::PSK_Software;
   }
 
   bool haveFastSqrt(Type *Ty) { return false; }
 
+  unsigned getFPOpCost(Type *Ty) { return TargetTransformInfo::TCC_Basic; }
+
   unsigned getIntImmCost(const APInt &Imm, Type *Ty) { return TTI::TCC_Basic; }
 
   unsigned getIntImmCost(unsigned Opcode, unsigned Idx, const APInt &Imm,
                          Type *Ty) {
     return TTI::TCC_Free;
   }
 
   unsigned getIntImmCost(Intrinsic::ID IID, unsigned Idx, const APInt &Imm,

include/llvm/CodeGen/BasicTTIImpl.h

public:
 
   bool haveFastSqrt(Type *Ty) {
     const TargetLoweringBase *TLI = getTLI();
     EVT VT = TLI->getValueType(Ty);
     return TLI->isTypeLegal(VT) &&
            TLI->isOperationLegalOrCustom(ISD::FSQRT, VT);
   }
 
+  unsigned getFPOpCost(Type *Ty) {
+    // By default, FP instructions are no more expensive since they are
+    // implemented in HW.  Target specific TTI can override this.
+    return TargetTransformInfo::TCC_Basic;
+  }
+
   void getUnrollingPreferences(Loop *L, TTI::UnrollingPreferences &UP) {
     // This unrolling functionality is target independent, but to provide some
     // motivation for its intended use, for x86:
 
     // According to the Intel 64 and IA-32 Architectures Optimization Reference
     // Manual, Intel Core models and later have a loop stream detector (and
     // associated uop queue) that can benefit from partial unrolling.
     // The relevant requirements are:

lib/Analysis/IPA/InlineCost.cpp

for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != E; ++I) {
     // Skip ephemeral values.
     if (EphValues.count(I))
       continue;
 
     ++NumInstructions;
     if (isa<ExtractElementInst>(I) || I->getType()->isVectorTy())
       ++NumVectorInstructions;
 
+    // If the instruction is floating point, and the target says this operation is
+    // expensive or the function has the "use-soft-float" attribute, this will
+    // eventually become a library call.  Treat the cost as such.
+    if (I->getType()->isFloatingPointTy()) {
+      bool hasSoftFloatAttr = false;
+
+      // If the function has the "use-soft-float" attribute, mark it as expensive.
+      if (F.hasFnAttribute("use-soft-float")) {
+        Attribute Attr = F.getFnAttribute("use-soft-float");
+        StringRef Val = Attr.getValueAsString();
+        if (Val == "true")
+          hasSoftFloatAttr = true;
+      }
+
+      if (TTI.getFPOpCost(I->getType()) == TargetTransformInfo::TCC_Expensive ||
+          hasSoftFloatAttr)
+        Cost += InlineConstants::CallPenalty;
+    }
+
     // If the instruction simplified to a constant, there is no cost to this
     // instruction. Visit the instructions using our InstVisitor to account for
     // all of the per-instruction logic. The visit tree returns true if we
     // consumed the instruction in any way, and false if the instruction's base
     // cost should count against inlining.
     if (Base::visit(I))
       ++NumInstructionsSimplified;
     else

lib/Analysis/TargetTransformInfo.cpp

 TargetTransformInfo::getPopcntSupport(unsigned IntTyWidthInBit) const {
   return TTIImpl->getPopcntSupport(IntTyWidthInBit);
 }
 
 bool TargetTransformInfo::haveFastSqrt(Type *Ty) const {
   return TTIImpl->haveFastSqrt(Ty);
 }
 
+unsigned TargetTransformInfo::getFPOpCost(Type *Ty) const {
+  return TTIImpl->getFPOpCost(Ty);
+}
+
 unsigned TargetTransformInfo::getIntImmCost(const APInt &Imm, Type *Ty) const {
   return TTIImpl->getIntImmCost(Imm, Ty);
 }
 
 unsigned TargetTransformInfo::getIntImmCost(unsigned Opcode, unsigned Idx,
                                             const APInt &Imm, Type *Ty) const {
   return TTIImpl->getIntImmCost(Opcode, Idx, Imm, Ty);
 }

lib/Target/ARM/ARMSubtarget.h

public:
   bool hasVFP3() const { return HasVFPv3; }
   bool hasVFP4() const { return HasVFPv4; }
   bool hasFPARMv8() const { return HasFPARMv8; }
   bool hasNEON() const { return HasNEON;  }
   bool hasCrypto() const { return HasCrypto; }
   bool hasCRC() const { return HasCRC; }
   bool hasVirtualization() const { return HasVirtualization; }
   bool useNEONForSinglePrecisionFP() const {
-    return hasNEON() && UseNEONForSinglePrecisionFP; }
+    return hasNEON() && UseNEONForSinglePrecisionFP;
+  }
 
   bool hasDivide() const { return HasHardwareDivide; }
   bool hasDivideInARMMode() const { return HasHardwareDivideInARM; }
   bool hasT2ExtractPack() const { return HasT2ExtractPack; }
   bool hasDataBarrier() const { return HasDataBarrier; }
   bool hasAnyDataBarrier() const {
     return HasDataBarrier || (hasV6Ops() && !isThumb());
   }
   bool useMulOps() const { return UseMulOps; }
   bool useFPVMLx() const { return !SlowFPVMLx; }
   bool hasVMLxForwarding() const { return HasVMLxForwarding; }
   bool isFPBrccSlow() const { return SlowFPBrcc; }

lib/Target/ARM/ARMTargetTransformInfo.h

public:
   unsigned getCastInstrCost(unsigned Opcode, Type *Dst, Type *Src);
 
   unsigned getCmpSelInstrCost(unsigned Opcode, Type *ValTy, Type *CondTy);
 
   unsigned getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index);
 
   unsigned getAddressComputationCost(Type *Val, bool IsComplex);
 
+  unsigned getFPOpCost(Type *Ty);
+
   unsigned getArithmeticInstrCost(
       unsigned Opcode, Type *Ty,
       TTI::OperandValueKind Op1Info = TTI::OK_AnyValue,
       TTI::OperandValueKind Op2Info = TTI::OK_AnyValue,
       TTI::OperandValueProperties Opd1PropInfo = TTI::OP_None,
       TTI::OperandValueProperties Opd2PropInfo = TTI::OP_None);
 
   unsigned getMemoryOpCost(unsigned Opcode, Type *Src, unsigned Alignment,
                            unsigned AddressSpace);
 
   /// @}
 };
 
 } // end namespace llvm
 
 #endif

lib/Target/ARM/ARMTargetTransformInfo.cpp

unsigned ARMTTIImpl::getAddressComputationCost(Type *Ty, bool IsComplex) {
   if (Ty->isVectorTy() && IsComplex)
     return NumVectorInstToHideOverhead;
 
   // In many cases the address computation is not merged into the instruction
   // addressing mode.
   return 1;
 }
 
+unsigned ARMTTIImpl::getFPOpCost(Type *Ty) {
+  // Use similar logic that's in ARMISelLowering:
+  // Any ARM CPU with VFP2 has floating point, but Thumb1 didn't have access
+  // to VFP.
+
+  if (ST->hasVFP2() && !ST->isThumb1Only()) {
+    if (Ty->isFloatTy()) {
+      return TargetTransformInfo::TCC_Basic;
+    }
+
+    if (Ty->isDoubleTy()) {
+      return ST->isFPOnlySP() ? TargetTransformInfo::TCC_Expensive :
+        TargetTransformInfo::TCC_Basic;
+    }
+  }
+
+  return TargetTransformInfo::TCC_Expensive;
+}
+
 unsigned ARMTTIImpl::getShuffleCost(TTI::ShuffleKind Kind, Type *Tp, int Index,
                                     Type *SubTp) {
   // We only handle costs of reverse and alternate shuffles for now.
   if (Kind != TTI::SK_Reverse && Kind != TTI::SK_Alternate)
     return BaseT::getShuffleCost(Kind, Tp, Index, SubTp);
 
   if (Kind == TTI::SK_Reverse) {
     static const CostTblEntry<MVT::SimpleValueType> NEONShuffleTbl[] = {

test/Transforms/Inline/inline-fp.ll

+; RUN: opt -S -inline < %s | FileCheck %s
+; Make sure that soft float implementations are calculated as being more expensive
+; to the inliner.
+
+define i32 @test_nofp() #0 {
+entry:
+  %responseX = alloca i32, align 4
+  %responseY = alloca i32, align 4
+  %responseZ = alloca i32, align 4
+  %valueX = alloca i8, align 1
+  %valueY = alloca i8, align 1
+  %valueZ = alloca i8, align 1
+
+  call void @getX(i32* %responseX, i8* %valueX)
+  call void @getY(i32* %responseY, i8* %valueY)
+  call void @getZ(i32* %responseZ, i8* %valueZ)
+
+  %0 = load i32* %responseX
+  %1 = load i8* %valueX
+  %call = call float @f_nofp(i32 %0, i8 zeroext %1)
+  %2 = load i32* %responseZ
+  %3 = load i8* %valueZ
+  %call2 = call float @f_nofp(i32 %2, i8 zeroext %3)
+  %call3 = call float @fabsf(float %call)
+  %cmp = fcmp ogt float %call3, 0x3FC1EB8520000000
+  br i1 %cmp, label %if.end12, label %if.else
+
+if.else:                                          ; preds = %entry
+  %4 = load i32* %responseY
+  %5 = load i8* %valueY
+  %call1 = call float @f_nofp(i32 %4, i8 zeroext %5)
+  %call4 = call float @fabsf(float %call1)
+  %cmp5 = fcmp ogt float %call4, 0x3FC1EB8520000000
+  br i1 %cmp5, label %if.end12, label %if.else7
+
+if.else7:                                         ; preds = %if.else
+  %call8 = call float @fabsf(float %call2)
+  %cmp9 = fcmp ogt float %call8, 0x3FC1EB8520000000
+  br i1 %cmp9, label %if.then10, label %if.end12
+
+if.then10:                                        ; preds = %if.else7
+  br label %if.end12
+
+if.end12:                                         ; preds = %if.else, %entry, %if.then10, %if.else7
+  %success.0 = phi i32 [ 0, %if.then10 ], [ 1, %if.else7 ], [ 0, %entry ], [ 0, %if.else ]
+  ret i32 %success.0
+}
+
+define i32 @test_hasfp() #0 {
+entry:
+  %responseX = alloca i32, align 4
+  %responseY = alloca i32, align 4
+  %responseZ = alloca i32, align 4
+  %valueX = alloca i8, align 1
+  %valueY = alloca i8, align 1
+  %valueZ = alloca i8, align 1
+
+  call void @getX(i32* %responseX, i8* %valueX)
+  call void @getY(i32* %responseY, i8* %valueY)
+  call void @getZ(i32* %responseZ, i8* %valueZ)
+
+  %0 = load i32* %responseX
+  %1 = load i8* %valueX
+  %call = call float @f_hasfp(i32 %0, i8 zeroext %1)
+  %2 = load i32* %responseZ
+  %3 = load i8* %valueZ
+  %call2 = call float @f_hasfp(i32 %2, i8 zeroext %3)
+  %call3 = call float @fabsf(float %call)
+  %cmp = fcmp ogt float %call3, 0x3FC1EB8520000000
+  br i1 %cmp, label %if.end12, label %if.else
+
+if.else:                                          ; preds = %entry
+  %4 = load i32* %responseY
+  %5 = load i8* %valueY
+  %call1 = call float @f_hasfp(i32 %4, i8 zeroext %5)
+  %call4 = call float @fabsf(float %call1)
+  %cmp5 = fcmp ogt float %call4, 0x3FC1EB8520000000
+  br i1 %cmp5, label %if.end12, label %if.else7
+
+if.else7:                                         ; preds = %if.else
+  %call8 = call float @fabsf(float %call2)
+  %cmp9 = fcmp ogt float %call8, 0x3FC1EB8520000000
+  br i1 %cmp9, label %if.then10, label %if.end12
+
+if.then10:                                        ; preds = %if.else7
+  br label %if.end12
+
+if.end12:                                         ; preds = %if.else, %entry, %if.then10, %if.else7
+  %success.0 = phi i32 [ 0, %if.then10 ], [ 1, %if.else7 ], [ 0, %entry ], [ 0, %if.else ]
+  ret i32 %success.0
+}
+
+declare void @getX(i32*, i8*) #0
+
+declare void @getY(i32*, i8*) #0
+
+declare void @getZ(i32*, i8*) #0
+
+; f_nofp() is marked with the "use-soft-float", so it should never get inlined.
+; f_hasfp() should get inlined.
+; CHECK: define internal float @f_nofp
+; CHECK-NOT: define internal float @f_hasfp
+
+define internal float @f_hasfp(i32 %response, i8 zeroext %value1) #0 {
+entry:
+  %conv = zext i8 %value1 to i32
+  %sub = add nsw i32 %conv, -1
+  %conv1 = sitofp i32 %sub to float
+  %0 = tail call float @llvm.pow.f32(float 0x3FF028F5C0000000, float %conv1)
+  %mul = fmul float %0, 2.620000e+03
+  %conv2 = sitofp i32 %response to float
+  %sub3 = fsub float %conv2, %mul
+  %div = fdiv float %sub3, %mul
+  ret float %div
+}
+
+define internal float @f_nofp(i32 %response, i8 zeroext %value1) #1 {
+entry:
+  %conv = zext i8 %value1 to i32
+  %sub = add nsw i32 %conv, -1
+  %conv1 = sitofp i32 %sub to float
+  %0 = tail call float @llvm.pow.f32(float 0x3FF028F5C0000000, float %conv1)
+  %mul = fmul float %0, 2.620000e+03
+  %conv2 = sitofp i32 %response to float
+  %sub3 = fsub float %conv2, %mul
+  %div = fdiv float %sub3, %mul
+  ret float %div
+}
+
+declare float @fabsf(float) optsize minsize
+
+declare float @llvm.pow.f32(float, float) optsize minsize
+
+attributes #0 = { minsize optsize }
+attributes #1 = { minsize optsize "use-soft-float"="true" }