sin vs floor : clang bug

Eli Friedman eli.friedman at gmail.com
Mon Jul 29 18:27:46 PDT 2013


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



More information about the cfe-commits mailing list