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

[Eric Christopher]

On Mon, Sep 29, 2014 at 2:18 PM, reed kotler <rkotler at mips.com> wrote:
> Hi Eric,
>
> It's been a while since I worked on Mips16 floating point.
>
> Be careful though because in LLVM target independent terms, Mips16 hard
> float is still soft float;
> the difference just being which version of the fp emulation library is being
> used.
>

Yes, with the exception that the calling convention is soft float, but
mips16 hard float can still use floating point operations in
libraries.

> When Mips16 hard float is on, there is a lot of difference though internally
> to the Mips specific targe code.
>
> Be careful in this area too because it's very complicated and we have this
> ability to switch between Mips16 and Mips32
> in the same .c file and that was tricky for me to get to work within LLVM.
>

Yes. I'm fixing up some of those issues.

> There is one test that is failing locally for us that started to fail around
> the time when you were making changes
> and it may be unrelated to that, but it's a test for this dual 32/16 mode
> switching ability. I have been busy and
> did not look at this failure yet because I did not know about it when it
> first happened because we had some other
> issues unrelated to that in our local build bots at exactly that moment that
> masked the problem.
>

Likely the problem I'm fixing here. If you change the order of
functions in the test suite for the test I have modified you'll see a
failure.

> This patch Simon made was to fix some problem we had in Mips16 build bots
> for test-suite and I think it's correct
> and for sure fixed the problem we were having. But I don't remember any of
> the issues.

I'm almost certain it's not correct, but may have successfully hidden
the problem. I've also verified that with gcc and soft-float enabled
the stub is not emitted.

Since I've verified the behavior with gcc I'm going to go ahead and
commit here. Let me know if you see any further problems.

-eric

>
> I will try and find some time later this week to really think about this and
> remember the issues.
>
> Reed
>
>
>
>
> On 09/29/2014 12:18 AM, Eric Christopher wrote:
>>
>> 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
>
>