[PATCH] D99438: [SimplifyLibCalls] Take size of int into consideration when emitting ldexp/ldexpf

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

Seems like there is little interest in this, but I was not sure really who to call for review from the beginning. Please let me know if you aren't interested in reviewing this by removing yourselves as reviewer (that way I know I need to find some other candidates).



================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:1559
+    if (Value *ExpoI = getIntToFPVal(Expo, B, TLI->getIntSize()))
       return emitBinaryFloatFnCall(ConstantFP::get(Ty, 1.0), ExpoI, TLI,
                                    LibFunc_ldexp, LibFunc_ldexpf, LibFunc_ldexpl,
----------------
One thing that could be noticed is that emitBinaryFloatFnCall uses Module::getOrInsertFunction. Without this patch the getOrInsertFunction helper actually identifies that the function prototype is wrong, so it returns a function pointer that is bitcasted to the requested function pointer type. Isn't that weird in most cases, at least when we actually is emitting a call to the function?
I suspect that the bitcasted function pointer only is needed in certain scenarious when doing some kind of instrumentation (e.g. when the address is of interest rather than when one actually wants to emit a call to the function).

Maybe there should be a second flavor of Module::getOrInsertFunction that rather asserts when there is a mismatch rather than returning a bitcasted function pointer, that can be used in most cases, such as in emitBinaryFloatFnCall? That way problems like this could end up as ICE rather than miscompile.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99438/new/

https://reviews.llvm.org/D99438