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

[Dávid Bolvanský via Phabricator via llvm-commits]

xbolva00 added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineInternal.h:256
 
+  MapVector<Function *, unsigned> HeapAllocEliminations;
+
----------------
efriedma wrote:
> This still doesn't really do what you want: opt -instcombine -instcombine will ignore the limit (and we effectively do that in a lot of places).  Maybe you could solve that by moving the transform into its own pass?  Not sure; I'll think about it over the weekend and get back to you.
Sure, thanks


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:2307
+  if (PointerMayBeCaptured(Malloc, true, true))
+    return false;
+
----------------
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)


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:2307
+  if (PointerMayBeCaptured(Malloc, true, true))
+    return false;
+
----------------
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);
}


https://reviews.llvm.org/D47345