[llvm] r218632 - Add soft-float to the key for the subtarget lookup in the TargetMachine

David Blaikie dblaikie at gmail.com
Mon Oct 6 11:46:14 PDT 2014 

On Mon, Oct 6, 2014 at 10:26 AM, Eric Christopher <echristo at gmail.com>
wrote:

> On Mon, Oct 6, 2014 at 9:37 AM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> > On Mon, Sep 29, 2014 at 2:57 PM, Eric Christopher <echristo at gmail.com>
> > wrote:
> >>
> >> Author: echristo
> >> Date: Mon Sep 29 16:57:54 2014
> >> New Revision: 218632
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=218632&view=rev
> >> Log:
> >> Add soft-float to the key for the subtarget lookup in the TargetMachine
> >> map, this makes sure that we can compile the same code for two different
> >> ABIs (hard and soft float) in the same module.
> >
> >
> > Not sure I asked/gave the credit here - does this mean hard/soft float
> > handling has transitioned to your new varying-subtarget infrastructure?
> (&
> > the test demonstrates that it's not just a drop-in replacement for
> existing
> > functionality, but addresses bugs in the previous implementation?)
> >
>
> Correct.
>
> > Oh, I see, it was 218492 - awesome. So that actually enabled per-function
> > cpu feature attributes & such on MIPS? Very cool. (sounds like that
> change
> > could've had at least one test that demonstrated the per-function
> attributes
> > varying - your test change, while necessary for the tests to pass after
> the
> > change (& thus, somewhat demonstrating the change in behavior) would now
> > equally pass before your change, if I understand correctly - so not
> really
> > testing to ensure we don't regress this behavior)
> >
>
> Correct. Well, it had plenty because of all the mips16 tests, I just
> had a bug with the subtarget with soft float bit :)
>
> Basically the bug was that there was a piece of global state that was
> being used in subtarget initialization that I wasn't paying attention
> to in per-function subtarget initialization. Also, it wouldn't have
> been really noticed until someone using the mips backend decided to
> LTO some functions together that were soft-float/hard-float and
> mips16. But it was there and should be fixed.
>

Right - and your extra (reordered) test case does expose the bug. I had
something confused in thinking the test cases would pass both before and
after your change (maybe I was getting some of the changes confused with
your other commit *goes to look again*)


> > (do we get to delete any code that was handling this in other ways?)
>
> Sadly, not yet. That's what that obnoxious todo means. But yes, we
> will get to eventually I think. UseSoftFloat should probably disappear
> from the Options struct and just be on the function. The only reason I
> haven't done this yet is that it's not a bad place to put a global
> default for the target either.
>
> Make sense?
>

Vaguely (only due to my only vague understanding of this code in general -
only going off what we've discussed previously for the most part). Thanks
for the info.

- David


>
> -eric
>
>
> >
> >>
> >> Update one testcase accordingly (and fix some confusing naming) and
> >> add a new testcase as well with the ordering swapped which would
> >> highlight the problem.
> >>
> >> Added:
> >>     llvm/trunk/test/CodeGen/Mips/mips16-hf-attr-2.ll
> >>       - copied, changed from r218631,
> >> llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll
> >> Modified:
> >>     llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp
> >>     llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll
> >>
> >> Modified: llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp?rev=218632&r1=218631&r2=218632&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp (original)
> >> +++ llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp Mon Sep 29 16:57:54
> >> 2014
> >> @@ -105,12 +105,24 @@ MipsTargetMachine::getSubtargetImpl(cons
> >>        !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
> >>
> >> Copied: llvm/trunk/test/CodeGen/Mips/mips16-hf-attr-2.ll (from r218631,
> >> llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll)
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Mips/mips16-hf-attr-2.ll?p2=llvm/trunk/test/CodeGen/Mips/mips16-hf-attr-2.ll&p1=llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll&r1=218631&r2=218632&rev=218632&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll (original)
> >> +++ llvm/trunk/test/CodeGen/Mips/mips16-hf-attr-2.ll Mon Sep 29 16:57:54
> >> 2014
> >> @@ -3,16 +3,16 @@
> >>  ; RUN: llc -mtriple=mipsel-linux-gnu -march=mipsel \
> >>  ; RUN:     -mcpu=mips16 -relocation-model=pic < %s | FileCheck %s
> >>
> >> -define void @bar_sf() #0 {
> >> +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
> >>  }
> >>
> >> -define void @bar_hf() #1 {
> >> +define void @bar_hf() #0 {
> >>  ; CHECK: bar_hf:
> >>  entry:
> >>    %call1 = call float @foo(float 1.000000e+00)
> >>
> >> Modified: 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=218632&r1=218631&r2=218632&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll (original)
> >> +++ llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll Mon Sep 29 16:57:54
> >> 2014
> >> @@ -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
> >>  }
> >>
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141006/d1338678/attachment.html>

More information about the llvm-commits mailing list