Re: [PATCH] Add -feliminate-malloc to enable/disable elimination of redundant malloc/free pairs

[Andrew Pinski <pinskia at gmail dot com>]

On Wed, May 9, 2012 at 9:32 AM, Xinliang David Li <davidxl@google.com> wrote:
> On Wed, May 9, 2012 at 1:12 AM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Tue, May 8, 2012 at 6:18 PM, Xinliang David Li <davidxl@google.com> wrote:
>>> To be clear, this flag is for malloc implementation (such as tcmalloc)
>>> with side effect unknown to the compiler. Using -fno-builtin-xxx is
>>> too conservative for that purpose.
>>
>> I don't think that flys. ÂBtw, the patch also guards alloca - alloca is purely
>> GCC internal.
>>
>> What's the "unknown side-effects" that are also important to preserve
>> for free(malloc(4))?
>>
>
> The side effect is the user registered malloc hooks.


IIRC future versions of glibc will have those malloc hooks removed.
Because of this problem.

Thanks,
Andrew Pinski


>
> The pattern 'free(malloc(4)', or loops like
>
> for (..)
> Â{
> Â Âmalloc(..); // result not used
> Â}
>
> itself is not interesting. They only appear in the test code and the
> problem can be worked around by using -fno-builtin-malloc. The option
> is to avoid disable this and all similar malloc optimizations (in the
> future) for malloc implementation with hooks.
>
> Thanks,
>
> David
>
>> Richard.
>>
>>> David
>>>
>>> On Tue, May 8, 2012 at 7:43 AM, Dehao Chen <dehao@google.com> wrote:
>>>> Hello,
>>>>
>>>> This patch adds a flag to guard the optimization that optimize the
>>>> following code away:
>>>>
>>>> free (malloc (4));
>>>>
>>>> In some cases, we'd like this type of malloc/free pairs to remain in
>>>> the optimized code.
>>>>
>>>> Tested with bootstrap, and no regression in the gcc testsuite.
>>>>
>>>> Is it ok for mainline?
>>>>
>>>> Thanks,
>>>> Dehao
>>>>
>>>> gcc/ChangeLog
>>>> 2012-05-08 ÂDehao Chen Â<dehao@google.com>
>>>>
>>>> Â Â Â Â* common.opt (feliminate-malloc): New.
>>>> Â Â Â Â* doc/invoke.texi: Document it.
>>>> Â Â Â Â* tree-ssa-dce.c (mark_stmt_if_obviously_necessary): Honor it.
>>>>
>>>> gcc/testsuite/ChangeLog
>>>> 2012-05-08 ÂDehao Chen Â<dehao@google.com>
>>>>
>>>> Â Â Â Â* gcc.dg/free-malloc.c: Check if -fno-eliminate-malloc is working
>>>> Â Â Â Âas expected.
>>>>
>>>> Index: gcc/doc/invoke.texi
>>>> ===================================================================
>>>> --- gcc/doc/invoke.texi (revision 187277)
>>>> +++ gcc/doc/invoke.texi (working copy)
>>>> @@ -360,7 +360,8 @@
>>>> Â-fcx-limited-range @gol
>>>> Â-fdata-sections -fdce -fdelayed-branch @gol
>>>> Â-fdelete-null-pointer-checks -fdevirtualize -fdse @gol
>>>> --fearly-inlining -fipa-sra -fexpensive-optimizations -ffat-lto-objects @gol
>>>> +-fearly-inlining -feliminate-malloc -fipa-sra -fexpensive-optimizations @gol
>>>> +-ffat-lto-objects @gol
>>>> Â-ffast-math -ffinite-math-only -ffloat-store
>>>> -fexcess-precision=@var{style} @gol
>>>> Â-fforward-propagate -ffp-contract=@var{style} -ffunction-sections @gol
>>>> Â-fgcse -fgcse-after-reload -fgcse-las -fgcse-lm -fgraphite-identity @gol
>>>> @@ -6238,6 +6239,7 @@
>>>> Â-fdefer-pop @gol
>>>> Â-fdelayed-branch @gol
>>>> Â-fdse @gol
>>>> +-feliminate-malloc @gol
>>>> Â-fguess-branch-probability @gol
>>>> Â-fif-conversion2 @gol
>>>> Â-fif-conversion @gol
>>>> @@ -6762,6 +6764,11 @@
>>>> ÂPerform dead store elimination (DSE) on RTL@.
>>>> ÂEnabled by default at @option{-O} and higher.
>>>>
>>>> +@item -feliminate-malloc
>>>> +@opindex feliminate-malloc
>>>> +Eliminate unnecessary malloc/free pairs.
>>>> +Enabled by default at @option{-O} and higher.
>>>> +
>>>> Â@item -fif-conversion
>>>> Â@opindex fif-conversion
>>>> ÂAttempt to transform conditional jumps into branch-less equivalents. ÂThis
>>>> Index: gcc/testsuite/gcc.dg/free-malloc.c
>>>> ===================================================================
>>>> --- gcc/testsuite/gcc.dg/free-malloc.c Â(revision 0)
>>>> +++ gcc/testsuite/gcc.dg/free-malloc.c Â(revision 0)
>>>> @@ -0,0 +1,12 @@
>>>> +/* { dg-do compile } */
>>>> +/* { dg-options "-O2 -fno-eliminate-malloc" } */
>>>> +/* { dg-final { scan-assembler-times "malloc" 2} } */
>>>> +/* { dg-final { scan-assembler-times "free" 2} } */
>>>> +
>>>> +extern void * malloc (unsigned long);
>>>> +extern void free (void *);
>>>> +
>>>> +void test ()
>>>> +{
>>>> + Âfree (malloc (10));
>>>> +}
>>>> Index: gcc/common.opt
>>>> ===================================================================
>>>> --- gcc/common.opt   Â(revision 187277)
>>>> +++ gcc/common.opt   Â(working copy)
>>>> @@ -1474,6 +1474,10 @@
>>>> ÂCommon Var(flag_dce) Init(1) Optimization
>>>> ÂUse the RTL dead code elimination pass
>>>>
>>>> +feliminate-malloc
>>>> +Common Var(flag_eliminate_malloc) Init(1) Optimization
>>>> +Eliminate unnecessary malloc/free pairs
>>>> +
>>>> Âfdse
>>>> ÂCommon Var(flag_dse) Init(1) Optimization
>>>> ÂUse the RTL dead store elimination pass
>>>> Index: gcc/tree-ssa-dce.c
>>>> ===================================================================
>>>> --- gcc/tree-ssa-dce.c Â(revision 187277)
>>>> +++ gcc/tree-ssa-dce.c Â(working copy)
>>>> @@ -309,6 +309,8 @@
>>>> Â Â Â Â Â Âcase BUILT_IN_CALLOC:
>>>> Â Â Â Â Â Âcase BUILT_IN_ALLOCA:
>>>> Â Â Â Â Â Âcase BUILT_IN_ALLOCA_WITH_ALIGN:
>>>> + Â Â Â Â Â Â if (!flag_eliminate_malloc)
>>>> + Â Â Â Â Â Â Â mark_stmt_necessary (stmt, true);
>>>> Â Â Â Â Â Â Âreturn;
>>>>
>>>> Â Â Â Â Â Âdefault:;