[PATCH] D28030: [TLI] Add prototype checking for all remaining LibFuncs.

[Davide Italiano via Phabricator via llvm-commits]

davide added a comment.

It would be great if you can add per-libcall tests to check they're correct (and make sure they won't bitrot).



================
Comment at: lib/Analysis/TargetLibraryInfo.cpp:701-703
+  case LibFunc::reallocf:
     return (NumParams == 2 && FTy.getParamType(0)->isPointerTy() &&
             FTy.getReturnType()->isPointerTy());
----------------
Maybe you can check also that the second argument to `realloc()` and `reallocf()` is an integer (`size_t`) ? 
Also, `isPointerTy()` only matches `void *` or also `T *` with, let's say `T = int` or `T = float`?


================
Comment at: lib/Analysis/TargetLibraryInfo.cpp:967-969
+  case LibFunc::atan2:
+  case LibFunc::atan2f:
+  case LibFunc::atan2l:
----------------
`atan2/atan2f/atan2l` , they all take two arguments.


================
Comment at: lib/Analysis/TargetLibraryInfo.cpp:1075-1081
+  case LibFunc::ldexp:
+  case LibFunc::ldexpf:
+  case LibFunc::ldexpl:
+    return (NumParams == 2 && FTy.getReturnType()->isFloatingPointTy() &&
+            FTy.getReturnType() == FTy.getParamType(0) &&
+            FTy.getParamType(1)->isIntegerTy(32));
+
----------------
Are you sure `isIntegerTy(32)` is correct? In other words, is always correct to assume that an int is `32-bit` ?


================
Comment at: lib/Analysis/TargetLibraryInfo.cpp:1126
+  case LibFunc::posix_memalign:
+    return (NumParams == 3 && FTy.getReturnType()->isIntegerTy() &&
+            FTy.getParamType(0)->isPointerTy() &&
----------------
Here the return type is `int` but you're just checking `isIntegerTy()`.
In other places where the return type is `int` you check `isIntegerTy(32)`.
Not sure which one is better, but we should be consistent.


https://reviews.llvm.org/D28030