[PATCH] D49514: [compiler-rt] [builtins] Add logb/logbf/logbl methods to compiler-rt to avoid libm dependencies when possible.

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht marked an inline comment as done.
rupprecht added a comment.

In https://reviews.llvm.org/D49514#1172432, @efriedma wrote:

> Maybe just make this a static inline function in fp_lib.h?


Thanks, done. That also makes these symbols not exported by the library, which is nice.

> Maybe worth implementing something like ilogb instead, to avoid an extra int->float->int conversion?  I guess it's not that important.

That sounds like a good idea, although the change is a little more invasive (e.g. all the div methods check against logb == infinity). I'd prefer to do that as a followup change if that's OK.

> 
> 
>> Fixes PR32279 and PR28652.
> 
> Is this really enough to fix PR32279?  That report mentions some other symbols (scalbn*, fmaxl).

Hmm, this fixes it enough for us, but you're right this isn't providing scalbn*/fmax*. We must be adding a -lc automatically somewhere, which provides those methods. I'll update the patch description that this is only a partial fix.



================
Comment at: lib/builtins/ppc/divtc3.c:26
 	int ilogbw = 0;
-	const double logbw = crt_logb(crt_fmax(crt_fabs(cDD.s.hi), crt_fabs(dDD.s.hi) ));
-	
-	if (crt_isfinite(logbw))
+	const double logbw = __compiler_rt_logbl(
+		crt_fmax(crt_fabs(cDD.s.hi), crt_fabs(dDD.s.hi)));
----------------
efriedma wrote:
> This is wrong. The original code is calling logb, not logbl.
> This is wrong. The original code is calling logb, not logbl.
> 
Yes, you're right.
I must have used logbl here because this is providing complex division for long doubles (c.f. non-ppc divtc3 uses logbl). But despite that, the method used is (non-long) doubles, due to ppc's floating point format here. I've fixed to use logb and left a comment to avoid that confusion.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D49514