[PATCH] D49273: [InstCombine] Expand the simplification of pow() into exp2()

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/trunk/lib/Transforms/Utils/SimplifyLibCalls.cpp:1276
+      else
+        return emitUnaryFloatFnCall(FMul, TLI->getName(LibFunc_exp2), B, Attrs);
+    }
----------------
uabelho wrote:
> On my target, exp2f is available, but exp2 isn't, so the
> 
>  hasUnaryFloatFn(TLI, Ty, LibFunc_exp2, LibFunc_exp2f, LibFunc_exp2l)
> 
> guard above returns true, but then when we try to find the name of LibFunc_exp2 we get "".
> 
> emitUnaryFloatFnCall doesn't check that the input name is something nice, it just happily adds an "f" to the name, and we end up with code trying to call the function "f" which eventually leads to a linking error.
> 
> Any thoughts about such cases?
I think the issue you're describing affects every user of emitUnaryFloatFnCall/emitBinaryFloatFnCall.

emitUnaryFloatFnCall should be fixed so it takes the same list of LibFunc enums that hasUnaryFloatFn does, and gets the appropriate name using getName().  The "appendTypeSuffix" helper is clearly inconsistent with the way TargetLibraryInfo is supposed to work.  I doubt the issue you're describing affects any in-tree target, but I'd approve the patch anyway as a cleanup.


Repository:
  rL LLVM

https://reviews.llvm.org/D49273