[PATCH] D18513: Simplify isfinite/isnan/isinf in finite-math-only mode

[Hal Finkel via llvm-commits]

hfinkel created this revision.
hfinkel added reviewers: scanon, chandlerc, spatel, beanz, mclow.lists, joerg, lhames.
hfinkel added a subscriber: llvm-commits.
Herald added subscribers: mcrosier, emaste.

This patch adds simplification for isfinite, isnan and isinf when we know that we don't have NaNs or Infs (based on the corresponding function attributes). Doing this requires a small infrastructure improvement to TargetLibraryInfo, as I'll explain below.

C/POSIX specify that math.h provides a set of macros, of which these are a subset:

  int isfinite(x);
  int isnan(x);
  int isinf(x);

where x is some floating-point type (float, double, long double). When we're compiling with -ffast-math (or specifically with -ffinite-math-only), it is profitable to statically simplify calls to these functions. For a motivating use case, consider compiling this code using libc++:

  #include <complex>
  using namespace std;

  complex<float> bar(complex<float> C);
  complex<float> foo(complex<float> C) {
    return bar(C)*C;
  }

and you'll quickly see that, even at -O3 -ffast-math, we produce a mess of code including calls to __isnanf and __isinff. Why those functions? This comes down to how glibc implements these macros:

  #  define isnan(x) \
       (sizeof (x) == sizeof (float)                                            \
        ? __isnanf (x)                                                          \
        : sizeof (x) == sizeof (double)                                         \
        ? __isnan (x) : __isnanl (x))

Other systems use similar macro expansions, with some variation in the names of the underlying functions. OSX here has a split system (or at least used to). When using finite-math-only mode, you get function calls:

  #define isfinite(x)                                               \
      ( sizeof(x) == sizeof(float)  ? __isfinitef((float)(x))       \
      : sizeof(x) == sizeof(double) ? __isfinited((double)(x))      \
                                    : __isfinitel((long double)(x)))

but when in IEEE-conforming mode, you get faster inline implementations:

  #define isfinite(x)                                                      \
      ( sizeof(x) == sizeof(float)  ? __inline_isfinitef((float)(x))       \
      : sizeof(x) == sizeof(double) ? __inline_isfinited((double)(x))      \
                                    : __inline_isfinitel((long double)(x)))

where the headers define things like:

  __header_always_inline int __inline_isfinitef(float __x) {
      return __x == __x && __builtin_fabsf(__x) != __builtin_inff();
  }

  __header_always_inline int __inline_isinff(float __x) {
      return __builtin_fabsf(__x) == __builtin_inff();
  }

  __header_always_inline int __inline_isnanf(float __x) {
      return __x != __x;
  }

so some effort has been made to preserve the full functioning of these calls even when otherwise compiling in finite-math-only mode. This optimization would purposely break that feature (in favor of lowering abstraction penalties). If we do this and a user wishes to check his or her inputs for NaNs, Infs, etc. the user must do so in a translation unit where such values are permitted to exist. To be fair, gcc's manual does not define finite-math-only mode in this way, but rather:

  Allow optimizations for floating-point arithmetic that assume that arguments and results are not NaNs or +-Infs.

perhaps implying that it is fine to check numbers for NaN/Inf that you did not compute via some arithmetic operation. We could certainly do it this way (i.e. based on an operand's fast-math flags -- although the implementation is not completely trivial because we need to look through PHIs, not just at direct function arguments), although that has the obvious problems with users being surprised by the effects of function inlining. Also, we have -fno-builtin-foo, although it would need some enhancements to work easily in this case because of the macros.

As another data point, FreeBSD seems to always use an inline version of isnan, but has function calls for isinf/isfinite.

Some alternatives (not all mutually exclusive with this one):

 1. As mentioned above, base the folding decision on the fast-math flags of the inputs (looking through phis) instead of the caller's function attributes
 2. In non-finite-math-only mode, replace the calls with a direct implementation (i.e. have the compiler do on all platforms what the OSX math.h header does)
 3. Always replace the calls with inline versions, but mark the instructions somehow so that they don't be removed by later optimizatons
 4. Enhance libc++ to contain some __FINITE_MATH_ONLY__ ifdefs

Regarding the infrastructure enhancement, we currently check for known library calls by name like this in SimplifyLibCalls:

   if (TLI->getLibFunc(FuncName, Func) && TLI->has(Func)) {

but currently this does not work if a library function's name is not the default name, but rather one substituted with TLI.setAvailableWithName. This is because we simply never search these custom names when looking for known functions by name (we know only to generate the custom name if we already have its LibFunc identifier). To make this work (necessary for this case because systems disagree on __finite vs. __isfinite vs. __isfinited, etc.) I've added an additional StringMap to TargetLibraryInfo used to lookup LibFunc identifiers based on custom names. As it turns out, this also requires adding a copy constructor to StringMap (D18506).


http://reviews.llvm.org/D18513

Files:
  include/llvm/Analysis/TargetLibraryInfo.def
  include/llvm/Analysis/TargetLibraryInfo.h
  include/llvm/Transforms/Utils/SimplifyLibCalls.h
  lib/Analysis/TargetLibraryInfo.cpp
  lib/Transforms/Utils/SimplifyLibCalls.cpp
  test/Transforms/InstCombine/fp-classify-libcalls.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D18513.51760.patch
Type: text/x-patch
Size: 10523 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160328/45611b96/attachment.bin>