[PATCH] D53370: Add a emitUnaryFloatFnCall version that fetches the function name from TLI

[Mikael Holmén via Phabricator via llvm-commits]

uabelho added a comment.

In https://reviews.llvm.org/D53370#1267950, @efriedma wrote:

> LGTM.
>
> It looks like you didn't touch LibCallSimplifier::optimizeLog, but I guess that's not necessary (it probably shouldn't be using emitUnaryFloatFnCall anyway).


Yes, I didn't touch optimizeLog or optimizeDoubleFP since they didn't use TLI->getName(), on the "double" version and thus wouldn't risk getting an empty name back from it.

But I'll submit this as is now then.

If you want me to do something with optimizeLog and/or optimizeDoubleFP I can do that in a follow-up commit.

Thanks!



================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1081-1082
   else
     R = isBinary ? emitBinaryFloatFnCall(V[0], V[1], CalleeNm, B, CalleeAt)
                  : emitUnaryFloatFnCall(V[0], CalleeNm, B, CalleeAt);
 
----------------
efriedma wrote:
> uabelho wrote:
> > I don't know if these calls pose any problem? Since we use the name of the function that we already called I suppose we at least shouldn't end up with a name like ""?
> > 
> > Also since we can deal with many different kinds of lib functions here I'm not sure how messy it is to find the double/float/long double versions of the function and use the new interface?
> > 
> > Suggestions?
> In theory it's possible to run into a similar problem: the float version might not exist, or might not have the expected name.  We should be using the LibFunc enum, rather than appending "f" to the name.  And it's probably not that hard to fix; all the callers of optimizeBinaryDoubleFP and optimizeUnaryDoubleFP know exactly what function they're optimizing.
> 
> That said, all the callers of optimizeDoubleFP call hasFloatVersion(), which I think prevents any practical problems.
Ok, yes we could let the callers of optimizeDoubleFP pass in the three lib versions then. Perhaps the same with optimizeLog?


Repository:
  rL LLVM

https://reviews.llvm.org/D53370