[PATCH] D18974: Make the allocsize attribute useful in LLVM

[George Burgess IV via llvm-commits]

george.burgess.iv added inline comments.

================
Comment at: lib/Analysis/MemoryBuiltins.cpp:53
@@ -53,25 +52,3 @@
 // know which functions are nounwind, noalias, nocapture parameters, etc.
-static const AllocFnsTy AllocationFnData[] = {
-  {LibFunc::malloc,              MallocLike,  1, 0,  -1},
-  {LibFunc::valloc,              MallocLike,  1, 0,  -1},
-  {LibFunc::Znwj,                OpNewLike,   1, 0,  -1}, // new(unsigned int)
-  {LibFunc::ZnwjRKSt9nothrow_t,  MallocLike,  2, 0,  -1}, // new(unsigned int, nothrow)
-  {LibFunc::Znwm,                OpNewLike,   1, 0,  -1}, // new(unsigned long)
-  {LibFunc::ZnwmRKSt9nothrow_t,  MallocLike,  2, 0,  -1}, // new(unsigned long, nothrow)
-  {LibFunc::Znaj,                OpNewLike,   1, 0,  -1}, // new[](unsigned int)
-  {LibFunc::ZnajRKSt9nothrow_t,  MallocLike,  2, 0,  -1}, // new[](unsigned int, nothrow)
-  {LibFunc::Znam,                OpNewLike,   1, 0,  -1}, // new[](unsigned long)
-  {LibFunc::ZnamRKSt9nothrow_t,  MallocLike,  2, 0,  -1}, // new[](unsigned long, nothrow)
-  {LibFunc::msvc_new_int,         OpNewLike,   1, 0,  -1}, // new(unsigned int)
-  {LibFunc::msvc_new_int_nothrow, MallocLike,  2, 0,  -1}, // new(unsigned int, nothrow)
-  {LibFunc::msvc_new_longlong,         OpNewLike,   1, 0,  -1}, // new(unsigned long long)
-  {LibFunc::msvc_new_longlong_nothrow, MallocLike,  2, 0,  -1}, // new(unsigned long long, nothrow)
-  {LibFunc::msvc_new_array_int,         OpNewLike,   1, 0,  -1}, // new[](unsigned int)
-  {LibFunc::msvc_new_array_int_nothrow, MallocLike,  2, 0,  -1}, // new[](unsigned int, nothrow)
-  {LibFunc::msvc_new_array_longlong,         OpNewLike,   1, 0,  -1}, // new[](unsigned long long)
-  {LibFunc::msvc_new_array_longlong_nothrow, MallocLike,  2, 0,  -1}, // new[](unsigned long long, nothrow)
-  {LibFunc::calloc,              CallocLike,  2, 0,   1},
-  {LibFunc::realloc,             ReallocLike, 2, 1,  -1},
-  {LibFunc::reallocf,            ReallocLike, 2, 1,  -1},
-  {LibFunc::strdup,              StrDupLike,  1, -1, -1},
-  {LibFunc::strndup,             StrDupLike,  2, 1,  -1}
+static const std::pair<LibFunc::Func, AllocFnsTy> AllocationFnData[] = {
+  {LibFunc::malloc,              {MallocLike,  1, 0,  -1}},
----------------
joker.eph wrote:
> Is this related to your attribute change? If it just cleanup, commit separately now.
> Is this related to your attribute change?

This patch depends on this change, but the change isn't dependent on the rest of the patch.

> If it just cleanup, commit separately now

I don't think it's strictly a cleanup, because it really doesn't simplify anything or add value, aside from enabling us to make new `AllocFnsTy` structs at runtime, which is only useful for `allocsize`.

If you still feel that I should check it in separately, I'm happy to do so.

================
Comment at: lib/Analysis/MemoryBuiltins.cpp:113
@@ +112,3 @@
+  // If it has allocsize, we can skip checking if it's a known function
+  LLVM_CONSTEXPR auto AllocSizeAllocTy = MallocLike;
+  if ((AllocTy & AllocSizeAllocTy) != 0 &&
----------------
joker.eph wrote:
> Why requires `MallocLike` here?
Our choices here are `OpNewLike`, `MallocLike`, `CallocLike`, `ReallocLike`, `StrDupLike`, `AllocLike` (which is `Malloc|Calloc|StrDup`), and `AnyAlloc` (which is `AllocLike|ReallocLike`).

- `CallocLike` won't work, because it requires the function to return zeroed memory. `allocsize` makes no such guarantee, so, `AllocLike`, `AnyAlloc`, and `CallocLike` are out.

- `StrDupLike` isn't correct either, because we only deal in integer sizes.

- `ReallocLike` isn't guaranteed, because we may have this on `malloc`.

- `allocsize` makes no guarantees about the result of the function it's tagged on being non-null, so `OpNewLike` is out.

...So we're left with `MallocLike`. :)

Added a comment addressing why `MallocLike` was chosen.


http://reviews.llvm.org/D18974