Convert finite to builtin

[danielcdh]

Summary

This patch converts finite/__finite to builtin functions so that it will be inlined by compiler.

[efriedma]

"finite()" is a BSD function, not a C standard function, so ALL_LANGUAGES isn't appropriate.

I can't seem to find documentation for "__finite()"; where is it defined?

[danielcdh]

In D24483#540551, @efriedma wrote:

"finite()" is a BSD function, not a C standard function, so ALL_LANGUAGES isn't appropriate.

Thanks for the comment. I'm not quite familiar with BSD function. Could you suggest which flag I should use here? C_LANG?

I can't seem to find documentation for "__finite()"; where is it defined?

It's just that if I include <math.h> and use isfinite(), it will automatically expand to finite() instead of finite(). But if I call finite() directly, it will expand to finite(). Thus I think I need to handle both finite and finite. Any suggestions on this?

Thanks,
Dehao

[efriedma]

GNU_LANG is the flag you're looking for.

"it will automatically expand to finite()"

What exactly is your build configuration (operating system, compiler, libc version if applicable)?

Also, this is missing a test (see test/CodeGen/builtins.c).

[danielcdh]

update

[danielcdh]

In D24483#540597, @efriedma wrote:

GNU_LANG is the flag you're looking for.

"it will automatically expand to finite()"

What exactly is your build configuration (operating system, compiler, libc version if applicable)?

I'm running on ubuntu with clang 4.0.

I found that the __finite is expanded from math.h:

define isfinite(x) \

(sizeof (x) == sizeof (float)                                            \
 ? __finitef (x)                                                         \
 : sizeof (x) == sizeof (double)                                         \
 ? __finite (x) : __finitel (x))

finite() is expanded from /usr/include/x86_64-linux-gnu/bits/mathcalls.h

/* Return nonzero if VALUE is finite and not NaN. */
MATHDECL_1 (int,finite,, (_Mdouble_ value)) attribute ((const));

which is expanded to:
extern int finite (double value) throw () attribute__ ((const));

Not sure what is the right thing to do here...

Thanks,
Dehao

Also, this is missing a test (see test/CodeGen/builtins.c).

added

[efriedma]

Oh, I see...

It's probably worth adding a comment explaining where __finite comes from, since it isn't obvious. Also, should be okay to mark it ALL_LANGUAGES (since it has a double-underscore prefix).

[danielcdh]

update

[danielcdh]

Thanks for the review, Eli. Do you have other comments about this patch?

Thanks,
Dehao

[efriedma]

LGTM after you change the comment.

include/clang/Basic/Builtins.def

922

This comment isn't really clear... maybe something more like "glibc's math.h generates calls to __finite"?

[danielcdh]

update comment

include/clang/Basic/Builtins.def

 LIBBUILTIN(copysign, "ddd", "fnc", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(copysignf, "fff", "fnc", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(copysignl, "LdLdLd", "fnc", "math.h", ALL_LANGUAGES)
 
 LIBBUILTIN(fabs, "dd", "fnc", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(fabsf, "ff", "fnc", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(fabsl, "LdLd", "fnc", "math.h", ALL_LANGUAGES)
 
+LIBBUILTIN(finite, "id", "fnc", "math.h", GNU_LANG)
+LIBBUILTIN(finitef, "if", "fnc", "math.h", GNU_LANG)
+LIBBUILTIN(finitel, "iLd", "fnc", "math.h", GNU_LANG)
+// glibc's math.h generates calls to __finite
+LIBBUILTIN(__finite, "id", "fnc", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(__finitef, "if", "fnc", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(__finitel, "iLd", "fnc", "math.h", ALL_LANGUAGES)
+
 LIBBUILTIN(fmod, "ddd", "fne", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(fmodf, "fff", "fne", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(fmodl, "LdLdLd", "fne", "math.h", ALL_LANGUAGES)
 
 LIBBUILTIN(frexp, "ddi*", "fn", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(frexpf, "ffi*", "fn", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(frexpl, "LdLdi*", "fn", "math.h", ALL_LANGUAGES)
 

lib/CodeGen/CGBuiltin.cpp

case Builtin::BI__builtin_isunordered: {
     return RValue::get(Builder.CreateZExt(LHS, ConvertType(E->getType())));
   }
   case Builtin::BI__builtin_isnan: {
     Value *V = EmitScalarExpr(E->getArg(0));
     V = Builder.CreateFCmpUNO(V, V, "cmp");
     return RValue::get(Builder.CreateZExt(V, ConvertType(E->getType())));
   }
 
+  case Builtin::BIfinite:
+  case Builtin::BI__finite:
+  case Builtin::BIfinitef:
+  case Builtin::BI__finitef:
+  case Builtin::BIfinitel:
+  case Builtin::BI__finitel:
   case Builtin::BI__builtin_isinf:
   case Builtin::BI__builtin_isfinite: {
     // isinf(x)    --> fabs(x) == infinity
     // isfinite(x) --> fabs(x) != infinity
     // x != NaN via the ordered compare in either case.
     Value *V = EmitScalarExpr(E->getArg(0));
     Value *Fabs = EmitFAbs(*this, V);
     Constant *Infinity = ConstantFP::getInfinity(V->getType());

test/CodeGen/builtins.c

void test_float_builtins(float F, double D, long double LD) {
   // CHECK:  %[[ISNEG:.*]] = icmp slt i80 %[[BITCAST]], 0
   // CHECK:  %[[SIGN:.*]] = select i1 %[[ISNEG]], i32 -1, i32 1
   // CHECK:  select i1 %[[ISINF]], i32 %[[SIGN]], i32 0
 
   res = __builtin_isfinite(F);
   // CHECK: call float @llvm.fabs.f32(float
   // CHECK: fcmp one float {{.*}}, 0x7FF0000000000000
 
+  res = finite(D);
+  // CHECK: call double @llvm.fabs.f64(double
+  // CHECK: fcmp one double {{.*}}, 0x7FF0000000000000
+
   res = __builtin_isnormal(F);
   // CHECK: fcmp oeq float
   // CHECK: call float @llvm.fabs.f32(float
   // CHECK: fcmp ult float {{.*}}, 0x7FF0000000000000
   // CHECK: fcmp uge float {{.*}}, 0x3810000000000000
   // CHECK: and i1
   // CHECK: and i1
 }