[LLVMdev] Floating point instructions patch

[Chris Lattner]

On Fri, 29 Apr 2005, Morten Ofstad wrote:
> Chris Lattner wrote:
>> The patches I didn't apply are these:
>> 2. Codegen fsin/fcos to fsin/fcos for X86.  We cannot do this, except
>>    under the control of something like -enable-unsafe-fp-math.  These
>>    instructions are architected to have a limited range.
>
> OK, I will add this flag.

Great, thanks!

>> 3. Codegen fsqrt/fsqrtf C functions to the FSQRT dag node.  These
>>    functions can set errno, so this is not a safe transformation.  The
>>    proper way to do this is to introduce an llvm.sqrt.f32/llvm.sqrt.f64
>>    pair of intrinsics to represent these operations without errno.  The
>>    optimizer can then turn fsqrt calls into these intrinsics when errno is
>>    provably never used or if a compiler option is specified to ignore
>>    errno.   Also, your work could just generate llvm.sqrt.* calls
>>    directly.
>
> Actually, in my first patch these instructions (including fabs) were all 
> intrinsics. My code generator used to generate llvm.sqrt calls, so it's 
> no problem to go back to that.

Sounds good.  The difference between sin/cos and sqrt is that sqrt sets 
errno.  Because of this, we have to introduce a new intrinsic (llvm.sqrt) 
which is undefined on range error.

>> 4. Codegen of fsin/fcos libm calls to FSIN/FCOS dag nodes.  We really do
>>    need to be able to legalize these for targets that don't support them
>>    before we can do this.  The easiest way to do this is to check the
>>    TargetLowering information for the operation to see if they are
>>    supported.  If not, leave them as function calls in the
>>    SelectionDAGISel.cpp file.  The alternative way is to have legalize
>>    lower them to libcalls.  This is also needed for fsqrt, though sin/cos
>>    don't set errno.
>
> The first solution is what I went for the first time around, but I was told 
> that it should be lowered to libcalls. I was unable to implement this as I 
> don't see how I get the information about what type (float/double) the call 
> originally used. When the DAG is legalized these are all promoted to f64, 
> right? If you just tell me how, I will fix this one also...

That is definitely one way to do it, but I think there are some minor 
pieces missing before this can be implemented (I'll get to it eventually 
;-).  In the mean time, it will be safe if you just check in the 
SelectionDAGISel.cpp code if the target supports these operations.  If so, 
introducing the FSIN/FCOS nodes should be ok, otherwise just generate 
normal calls.

The problem with this approach happens if anything else could generate 
FSIN/FCOS nodes.  Perhaps in the future this will be a problem, but it 
shouldn't be for now.  If your patch includes a big FIXME, I will be happy 
to commit it and fix the issue the correct way when I get a chance.

Thanks a lot Morten!

-Chris

-- 
http://nondot.org/sabre/
http://llvm.cs.uiuc.edu/