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

[Anatoly Trosinenko via Phabricator via llvm-commits]

atrosinenko added a subscriber: aykevl.
atrosinenko added a comment.

The idea of introducing `SizeOfInt` to `TargetLibraryInfo` looks quite reasonable to me, although I am not too familiar with this part of target description.

Maybe @aykevl has some comments on this?



================
Comment at: llvm/include/llvm/Analysis/TargetLibraryInfo.h:55
   bool ShouldExtI32Param, ShouldExtI32Return, ShouldSignExtI32Param;
+  unsigned SizeOfInt = 32;
 
----------------
This initialization is probably redundant.


================
Comment at: llvm/lib/Analysis/TargetLibraryInfo.cpp:1461
             FTy.getReturnType() == FTy.getParamType(0) &&
-            FTy.getParamType(1)->isIntegerTy(32));
+            FTy.getParamType(1)->isIntegerTy(getIntSize()));
 
----------------
I wonder whether every support library implementation on every 16-bit target satisfies this. Another question is about other usages of `->isIntegerTy(32)` in this file. Meanwhile, at least one such usage, for `htonl`/`ntohl`, seems to be exactly according to specification. Maybe this should be postponed to other patches (provided the issues can be detected easily enough).

//Just as a note: there are 21 occurrences of `inIntegerTy(32)`, 17 ones of `isIntegerTy()` (of any size) and even 1 `isIntegerTy(16)` (for `htons`/`ntohs`).//


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:1810
   if (AllowApprox && (isa<SIToFPInst>(Expo) || isa<UIToFPInst>(Expo))) {
-    if (Value *ExpoI = getIntToFPVal(Expo, B))
+    if (Value *ExpoI = getIntToFPVal(Expo, B, 32))
       return createPowWithIntegerExponent(Base, ExpoI, M, B);
----------------
Shouldn't it be `TLI->getIntSize()` as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99438