[PATCH] D16337: [LibCallSimplifier] fold memset(malloc(x), 0, x) --> calloc(1, x)

[Sanjay Patel via llvm-commits]

spatel added inline comments.

================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:942
@@ +941,3 @@
+                               const TargetLibraryInfo &TLI) {
+  // This has to be a memset of zeros (bzero).
+  auto *FillValue = dyn_cast<ConstantInt>(Memset->getArgOperand(1));
----------------
mcrosier wrote:
> Would it make more sense to check for calloc here?
> 
> if (!TLI.getLibFunc("calloc", Func) || !TLI.has(Func))
>     return nullptr;
> 
> It would avoid some unnecessary work in the event we don't have calloc.  However, I assume we generally have calloc, so this might not be a great suggestion...  Just a suggestion that doesn't necessarily have to be acted upon.
If we view this in isolation, that sounds reasonable, but the calloc existence check is in emitCalloc because that's the pattern used by the other emit* functions in BuildLibCalls. 

I'd prefer to leave this as-is pending resolution of the 'TODO' at line 918 about moving emitCalloc() over to BuildLibCalls to be with its siblings (even though that entire separate file is currently unnecessary because it's only ever used here AFAICT).

It's also possible (there's always a chance!) that someone will come up with some other transform here in LibCallSimplifier that needs a calloc(). In that case, I think it'd also be better to leave the existence check where it is.


http://reviews.llvm.org/D16337