[cfe-dev] sin vs floor : clang bug

[reed kotler]

On 07/29/2013 06:27 PM, Eli Friedman wrote:
> On Mon, Jul 29, 2013 at 6:24 PM, reed kotler <rkotler at mips.com> wrote:
>> On 07/29/2013 06:12 PM, Eli Friedman wrote:
>>> On Mon, Jul 29, 2013 at 5:19 PM, reed kotler <rkotler at mips.com> wrote:
>>>> I've finally figured out why sin is working in mips16 fp and floor not.
>>>>
>>>> This turns out to be a combination clang bug and llvm hack.
>>>>
>>>> If you compile a call to "sin" and a call to "floor", the attributes are
>>>> different.
>>>>
>>>> For floor they are { nounwind readnone }.
>>>> For sin they are { nounwind } .
>>>>
>>>> This makes a difference in llvm because they use this information to
>>>> figure
>>>> out if something
>>>> is an intrinsic in SelectionDAGBuilder::visitUnaryFloatCall (seems like
>>>> this
>>>> is the wrong way
>>>> to do this; there should be a separate attribute for this).
>>>>
>>>> Sin ends up failing this test in the beginning of the function.
>>>>
>>>>     // Sanity check that it really is a unary floating-point call.
>>>>     if (I.getNumArgOperands() != 1 ||
>>>>         !I.getArgOperand(0)->getType()->isFloatingPointTy() ||
>>>>         I.getType() != I.getArgOperand(0)->getType() ||
>>>>         !I.onlyReadsMemory())
>>>>       return false;
>>>>
>>>> Because onlyReadsMemory() becomes false.
>>>>
>>>> This bug makes Sin work for Mips16 because the proper signature for the
>>>> external exists at that time and the call can be emitted.
>>>>
>>>> When this bug is fixed, then sin will also fail for mips16; however I
>>>> have a
>>>> way to deal with this problem so it's not an issue for me but I wanted to
>>>> understand the root cause of the problem.
>>> The clang "bug" isn't actually a bug; we do this intentionally so
>>> -fmath-errno works.
>>
>> Well, then the llvm half of this is wrong?
>> It's expecting to pass this test for sin and others.
> The LLVM half is also working correctly: we can't treat sin as an
> intrinsic in -fmath-errno mode.  (Try comparing the clang output with
> -fmath-errno and -fno-math-errno.)
>
> -Eli
Gotcha.

In any case, during lower call,
the Args and Rety should be the same and not depend on any of this. For 
soft float, they are turning into Int64 in the case of double, for 
example. Those values usually just reflect the
actual function signature.

I have a workaround for this issue.

I will need to think about whether I need to do something to handle this 
errno case for mips16 fp. I will see how gcc handles this.



Reed