On Wed, 6 Nov 2024, Jan Hubicka wrote: > > > 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.
Ah, you unconditionally skip processing the GIMPLE_COND - so when the controlling predicate is marked necessary we do not mark the allocation necessary. So for Jakubs idea we'd have to distinguish different "reasons" for the control predicate to become necessary, like for example a (recursive) call, but not an abort (). I guess it's reasonable to handle it like you did, so indeed it will be quite aggressive. > 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 Let's do -fmalloc-dce then, but it's unfortunate -fallocation-dce is named so "generic". > 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 Since we had malloc/free pair removal for quite some time I think it should stay on by default. Richard. > 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 > -- Richard Biener <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
