[LLVMdev] Floating point instructions patch
sabre at nondot.org
Fri Apr 29 06:20:54 PDT 2005
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.
>> 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
> 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
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!
More information about the llvm-dev