[llvm] r198019 - [Mips] Does not take in account 'use-soft-float' attribute's value when
reed kotler
rkotler at mips.com
Mon Sep 29 14:36:46 PDT 2014
Ok.
We'll see what happens when our build bots pick up your change.
I was clearly trying to match gcc semantics for mips16 hard float so places
where they don't match is most likely a bug in the llvm port. There is
no manual
other than the gcc compiler itself on how this hard float for mips16 is
supposed to work.
In some cases there are differences due to intrinsics but everything
else should
match.
There is more work to do in this part of the compiler; not so much for
functionality
but to make it a better implementation.
Some things I do in an earlier pass should be moved into the back end more.
These are the places where inline assembly code is generated.
There is a better model for how to do that now and I moved a bunch of
things to a better place
but mostly it was new work and some of the older parts are still in a
special clang IR pass.
Reed
On 09/29/2014 02:29 PM, Eric Christopher wrote:
> 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
>>
More information about the llvm-commits
mailing list