> > Thinking about this some more, I think we should just add -fno-malloc-dce
> > option and do it even if ranges don't guarantee it won't be half of AS or
> > more, that is really just a special case and not too different from
> > doing 3 PTRDIFF_MAX - 10 allocations and expecting at least one of those
> > will fail, etc.
> > glibc tests can use -fno-malloc-dce, or add some optimization barrier
> > between the allocation and deallocation which makes compiler think that the
> > allocation is actually used.
> 
> I'd instead like to see -fallocation-dce be honored for malloc/free
> pairs as well - the default for C could be different from C++ here
> but I don't see a good reason to distinguish both?  Or do we want
> to treat malloc/free from C++ different than new/delete pairs?

C++ standard is worded to allow removal of operator new/delete, but I do
not think it has similar wording for malloc/free. We already handle
differently dirrect calls to ::new and ::delete functions because these
are not part of the C++ standard formulation.
For this reason it is not clear to me that we want to put malloc/free to
the same category as operator new/delete.

-fallocation-dce is already in middle end and accepted by C frontend, so
I can update the original patch (whtout range checks) to use it also for
malloc/free removal.
> 
> > Or the other option is decide not based on the size range, but what the
> > if (!ptr) code actually does, allow jumping around the freeing, allow
> > __builtin_unreachable, don't allow anything else.
> 
> I don't think the code easily allows for this.  I did expect the
> function call case to force the controlling predicate to survive
> though.

It does not.  I simply look for uses of the SSA name that holds the
return value of the malloc (or other allocation). If all uses are either
NULL pointer checks or call to paired frees, I consider it removable and
rewrite all conditionals for SSA name being non-NULL.
No aliasing orgacle is used here.

EH edges or external calls do not prevent the transformation as long
as the malloc return value is not stored somewhere.
> 
> Btw, I agree that the patch doesn't make the current situation worse
> and for strict conformance we should have an option to turn off the
> optimization.  Restricting it for some cases but not others doesn't
> make much sense IMO.

I guess we wnat to decide 
 1) whether we want to keep only -fallocation-dce and extend it to also
    cover malloc/free (and derived functions) or have also -fmalloc-dce 
 2) whether the flag should also control the malloc/free removal we
    already do by default (without specific flag)
 3) what should be the default for C

My personabl preference would be to have -fmalloc-dce, make it control
also the existing malloc/free removal and keep it enabled for C.
But I see that Alexander would preffer disabling the optimization for C
which is also fine with me, even though I think we are giving up on
useful optimization in order to not break very artificial code.

Honza

Reply via email to