[PATCH] D41338: [CodeGen] lower math intrinsics to finite version of libcalls when possible (PR35672)

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:4049
   case ISD::STRICT_FEXP:
-    Results.push_back(ExpandFPLibCall(Node, RTLIB::EXP_F32, RTLIB::EXP_F64,
-                                      RTLIB::EXP_F80, RTLIB::EXP_F128,
-                                      RTLIB::EXP_PPCF128));
+    if (CanUseFiniteLibCall && DAG.getLibInfo().has(LibFunc_exp_finite))
+      Results.push_back(ExpandFPLibCall(Node, RTLIB::EXP_FINITE_F32,
----------------
efriedma wrote:
> hfinkel wrote:
> > efriedma wrote:
> > > "getLibInfo().has(LibFunc_exp_finite)" probably isn't the right check.  There are three __exp*_finite variants; each of them may or may not be legal, and you need to check that the long double variant actually accepts the input float type.
> > > 
> > > Also, it looks like TargetLibraryInfo doesn't contain the appropriate checks to disable them (it assumes any non-Windows platform has __exp_finite, which is clearly false).
> > > and you need to check that the long double variant actually accepts the input float type.
> > 
> > How would we check this?
> > 
> > > (it assumes any non-Windows platform has __exp_finite, which is clearly false).
> > 
> > Hrmm. We could start with disabling them for any non-GNU triple.
> For figuring out whether the long-double variant is the one we need, I guess TargetLibraryInfo should know?  We need to avoid generating a call with undefined behavior somehow.  And we probably need to eventually teach this code how to generate a call to __expf128_finite when it's available.
> For figuring out whether the long-double variant is the one we need, I guess TargetLibraryInfo should know? We need to avoid generating a call with undefined behavior somehow.

I don't think that anything knows right now (except Clang). I think that we can generate <foo>l/__<foo>l_finite calls for ppcf128/f80 and <foo>f128/__<foo>f128_finite for f128 for x86 and ppc specifically (as we have special types, ppcf128 and f80, for long double on those platforms), and calls to <foo>l/__<foo>l_finite for f128 everywhere else. No?

> And we probably need to eventually teach this code how to generate a call to __expf128_finite when it's available.

I agree. glibc added a whole bunch of these now.



https://reviews.llvm.org/D41338