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

455

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:
   virtual bool shouldBuildLookupTables() const;
 
   /// \brief Return hardware support for population count.
   virtual PopcntSupportKind getPopcntSupport(unsigned IntTyWidthInBit) const;
 
   /// \brief Return true if the hardware has a fast square-root instruction.
   virtual bool haveFastSqrt(Type *Ty) const;
 
+  /// \brief Return the expected cost of supporting the floating point operation
+  /// of the specified type.
+  virtual unsigned getFPOpCost(Type *Ty) const;
+
   /// \brief Return the expected cost of materializing for the given integer
   /// immediate of the specified type.
   virtual 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.
   virtual unsigned getIntImmCost(unsigned Opc, unsigned Idx, const APInt &Imm,

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 PrevTTI->getPopcntSupport(IntTyWidthInBit);
 }
 
 bool TargetTransformInfo::haveFastSqrt(Type *Ty) const {
   return PrevTTI->haveFastSqrt(Ty);
 }
 
+unsigned TargetTransformInfo::getFPOpCost(Type *Ty) const {
+  return PrevTTI->getFPOpCost(Ty);
+}
+
 unsigned TargetTransformInfo::getIntImmCost(const APInt &Imm, Type *Ty) const {
   return PrevTTI->getIntImmCost(Imm, Ty);
 }
 
 unsigned TargetTransformInfo::getIntImmCost(unsigned Opc, unsigned Idx,
                                             const APInt &Imm, Type *Ty) const {
   return PrevTTI->getIntImmCost(Opc, Idx, Imm, Ty);
 }
struct NoTTI final : ImmutablePass, TargetTransformInfo {
   getPopcntSupport(unsigned IntTyWidthInBit) const override {
     return PSK_Software;
   }
 
   bool haveFastSqrt(Type *Ty) const override {
     return false;
   }
 
+  unsigned getFPOpCost(Type *Ty) const override {
+    // By default, FP instructions are no more expensive since they are
+    // implemented in HW.  Target specific TTI can override this.
+    return TCC_Basic;
+  }
+
   unsigned getIntImmCost(const APInt &Imm, Type *Ty) const override {
     return TCC_Basic;
   }
 
   unsigned getIntImmCost(unsigned Opcode, unsigned Idx, const APInt &Imm,
                          Type *Ty) const override {
     return TCC_Free;
   }

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.cpp

public:
 
   unsigned getMaxInterleaveFactor() const override {
     // These are out of order CPUs:
     if (ST->isCortexA15() || ST->isSwift())
       return 2;
     return 1;
   }
 
+  unsigned getFPOpCost(Type *Ty) const override;
+
   unsigned getShuffleCost(ShuffleKind Kind, Type *Tp,
                           int Index, Type *SubTp) const override;
 
   unsigned getCastInstrCost(unsigned Opcode, Type *Dst,
                             Type *Src) const override;
 
   unsigned getCmpSelInstrCost(unsigned Opcode, Type *ValTy,
                               Type *CondTy) const override;
unsigned ARMTTI::getAddressComputationCost(Type *Ty, bool IsComplex) const {
   if (Ty->isVectorTy() && IsComplex)
     return NumVectorInstToHideOverhead;
 
   // In many cases the address computation is not merged into the instruction
   // addressing mode.
   return 1;
 }
 
+unsigned ARMTTI::getFPOpCost(Type *Ty) const {
+  // Use the same logic that's in ARMISelLowering:
+  // SoftFloat can be enabled even if the underlying hardware supports it.
+  // Any ARM CPU with VFP2 has floating point, but Thumb1 didn't have access
+  // to VFP.
+
+  if (!TM->Options.UseSoftFloat && ST->hasVFP2() && !ST->isThumb1Only()) {
+    if (Ty->isFloatTy()) {
+      return TCC_Basic;
+    }
+
+    if (Ty->isDoubleTy()) {
+      return ST->isFPOnlySP() ? TCC_Expensive : TCC_Basic;
+    }
+  }
+
+  return TCC_Expensive;
+}
+
 unsigned ARMTTI::getShuffleCost(ShuffleKind Kind, Type *Tp, int Index,
                                 Type *SubTp) const {
   // We only handle costs of reverse and alternate shuffles for now.
   if (Kind != SK_Reverse && Kind != SK_Alternate)
     return TargetTransformInfo::getShuffleCost(Kind, Tp, Index, SubTp);
 
   if (Kind == 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" }