[llvm] r198019 - [Mips] Does not take in account 'use-soft-float' attribute's value when

[Eric Christopher]

Hi Simon,

Sorry for the thread necromancy here, but I don't think this patch is correct.

a) It's a bit confusing that you have bar_hf for the function with
use-soft-float set to true and vice versa for bar_sf, but
b) As far as I can recall, the stubs are only used for calling hard
float functions but the testcase currently tests that we emit it for
both.
c) The emission of the stubs should be dependent upon the subtarget
and the value of the soft-float on the function (which would be the
TM.Options.UseSoftFloat that is reset between each function, but only
in SelectionDAG or the version in the function attributes).

With the work I've been doing plus the attached patch we'll get this
as far as I can tell correct for this - it's been a while since I've
hacked on this aspect of mips.

-eric


On Wed, Dec 25, 2013 at 9:00 AM, Simon Atanasyan <simon at atanasyan.com> wrote:
> Author: atanasyan
> Date: Wed Dec 25 11:00:27 2013
> New Revision: 198019
>
> URL: http://llvm.org/viewvc/llvm-project?rev=198019&view=rev
> Log:
> [Mips] Does not take in account 'use-soft-float' attribute's value when
> consider to generate stubs for mips16 hard-float mode.
>
> The patch reviewed by Reed Kotler.
>
> Added:
>     llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll
> Modified:
>     llvm/trunk/lib/Target/Mips/Mips16ISelLowering.cpp
>
> Modified: llvm/trunk/lib/Target/Mips/Mips16ISelLowering.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/Mips16ISelLowering.cpp?rev=198019&r1=198018&r2=198019&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/Mips/Mips16ISelLowering.cpp (original)
> +++ llvm/trunk/lib/Target/Mips/Mips16ISelLowering.cpp Wed Dec 25 11:00:27 2013
> @@ -429,8 +429,7 @@ getOpndList(SmallVectorImpl<SDValue> &Op
>    const char* Mips16HelperFunction = 0;
>    bool NeedMips16Helper = false;
>
> -  if (getTargetMachine().Options.UseSoftFloat &&
> -      Subtarget->inMips16HardFloat()) {
> +  if (Subtarget->inMips16HardFloat()) {
>      //
>      // currently we don't have symbols tagged with the mips16 or mips32
>      // qualifier so we will assume that we don't know what kind it is.
>
> Added: llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll?rev=198019&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll (added)
> +++ llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll Wed Dec 25 11:00:27 2013
> @@ -0,0 +1,45 @@
> +; Check that stubs generation for mips16 hard-float mode does not depend
> +; on the function 'use-soft-float' attribute's value.
> +; RUN: llc -mtriple=mipsel-linux-gnu -march=mipsel \
> +; RUN:     -mcpu=mips16 -relocation-model=pic < %s | FileCheck %s
> +
> +define void @bar_sf() #0 {
> +; CHECK: bar_sf:
> +entry:
> +  %call1 = call float @foo(float 1.000000e+00)
> +; CHECK: lw $2, %call16(foo)($3)
> +; CHECK: lw $5, %got(__mips16_call_stub_sf_1)($3)
> +  ret void
> +}
> +
> +define void @bar_hf() #1 {
> +; CHECK: bar_hf:
> +entry:
> +  %call1 = call float @foo(float 1.000000e+00)
> +; CHECK: lw $2, %call16(foo)($3)
> +; CHECK: lw $5, %got(__mips16_call_stub_sf_1)($3)
> +  ret void
> +}
> +
> +declare float @foo(float) #2
> +
> +attributes #0 = {
> +  nounwind
> +  "less-precise-fpmad"="false" "no-frame-pointer-elim"="true"
> +  "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false"
> +  "no-nans-fp-math"="false" "stack-protector-buffer-size"="8"
> +  "unsafe-fp-math"="false" "use-soft-float"="false"
> +}
> +attributes #1 = {
> +  nounwind
> +  "less-precise-fpmad"="false" "no-frame-pointer-elim"="true"
> +  "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false"
> +  "no-nans-fp-math"="false" "stack-protector-buffer-size"="8"
> +  "unsafe-fp-math"="false" "use-soft-float"="true"
> +}
> +attributes #2 = {
> +  "less-precise-fpmad"="false" "no-frame-pointer-elim"="true"
> +  "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false"
> +  "no-nans-fp-math"="false" "stack-protector-buffer-size"="8"
> +  "unsafe-fp-math"="false" "use-soft-float"="true"
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
diff --git a/lib/Target/Mips/MipsTargetMachine.cpp b/lib/Target/Mips/MipsTargetMachine.cpp
index 542f24f..29761f4 100644
--- a/lib/Target/Mips/MipsTargetMachine.cpp
+++ b/lib/Target/Mips/MipsTargetMachine.cpp
@@ -105,12 +105,23 @@ MipsTargetMachine::getSubtargetImpl(const Function &F) const {
       !FnAttrs.getAttribute(AttributeSet::FunctionIndex, "nomips16")
            .hasAttribute(Attribute::None);
 
+  // FIXME: This is related to the code below to reset the target options,
+  // we need to know whether or not the soft float flag is set on the
+  // function before we can generate a subtarget. We also need to use
+  // it as a key for the subtarget since that can be the only difference
+  // between two functions.
+  Attribute SFAttr = FnAttrs.getAttribute(AttributeSet::FunctionIndex, "use-soft-float");
+  bool softFloat = !SFAttr.hasAttribute(Attribute::None)
+                       ? (SFAttr.getValueAsString() == "true" ? true : false)
+                       : Options.UseSoftFloat;
+
   if (hasMips16Attr)
     FS += FS.empty() ? "+mips16" : ",+mips16";
   else if (hasNoMips16Attr)
     FS += FS.empty() ? "-mips16" : ",-mips16";
 
-  auto &I = SubtargetMap[CPU + FS];
+  auto &I = SubtargetMap[CPU + FS + (softFloat ? "use-soft-float=true"
+                                               : "use-soft-float=false")];
   if (!I) {
     // This needs to be done before we create a new subtarget since any
     // creation will depend on the TM and the code generation flags on the
diff --git a/test/CodeGen/Mips/mips16-hf-attr.ll b/test/CodeGen/Mips/mips16-hf-attr.ll
index d9ad629..c6ad442 100644
--- a/test/CodeGen/Mips/mips16-hf-attr.ll
+++ b/test/CodeGen/Mips/mips16-hf-attr.ll
@@ -3,8 +3,8 @@
 ; RUN: llc -mtriple=mipsel-linux-gnu -march=mipsel \
 ; RUN:     -mcpu=mips16 -relocation-model=pic < %s | FileCheck %s
 
-define void @bar_sf() #0 {
-; CHECK: bar_sf:
+define void @bar_hf() #0 {
+; CHECK: bar_hf:
 entry:
   %call1 = call float @foo(float 1.000000e+00)
 ; CHECK: lw $2, %call16(foo)($3)
@@ -12,12 +12,12 @@ entry:
   ret void
 }
 
-define void @bar_hf() #1 {
-; CHECK: bar_hf:
+define void @bar_sf() #1 {
+; CHECK: bar_sf:
 entry:
   %call1 = call float @foo(float 1.000000e+00)
-; CHECK: lw $2, %call16(foo)($3)
-; CHECK: lw $5, %got(__mips16_call_stub_sf_1)($3)
+; CHECK: lw $3, %call16(foo)($2)
+; CHECK-NOT: lw $5, %got(__mips16_call_stub_sf_1)($3)
   ret void
 }