[PATCH] D47345: [InstructionCombining] Replace small heap allocations with local variables

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:2307
+  if (PointerMayBeCaptured(Malloc, true, true))
+    return false;
+
----------------
xbolva00 wrote:
> xbolva00 wrote:
> > xbolva00 wrote:
> > > efriedma wrote:
> > > > This check isn't sufficient to ensure some other function doesn't call free() on the pointer (actually, this doesn't even handle the case where there are two calls to free() along different codepaths).
> > > Other function...
> > > 
> > > void free_ptr(void **p);
> > > void capture(void*p);
> > > 
> > > void small_alloc_3(void) {
> > >     void *s = malloc(10);
> > >     capture(s);
> > >     free(s);
> > > }
> > > 
> > > void small_alloc_4(void) {
> > >     void *s = malloc(10);
> > >     free_ptr(&s);
> > >     free(s);
> > > }
> > > 
> > > These case are not optimized (PointerMayBeCaptured solves it)
> > But yes, the second case is a problem.. Not sure what to do.
> > 
> > void small_alloc_bad2(int n, int i) {
> >     char *s = malloc(10);
> >     puts(s);
> >     if (n)
> >             free(s);
> > 
> >     if (i)
> >             free(s);
> > }
> Check uses of "s"? If there is only one "free" ?
Your "capture" function captures the pointer, but it's possible to write a function which takes the pointer as a parameter but doesn't capture it.  For example, given the function `void f(void *p) { free(p); }`, LLVM will mark the parameter "p" nocapture.


https://reviews.llvm.org/D47345