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

[reed kotler]

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.

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.

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.

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