Re: [PATCH] ipa: call destructors on lattices before freeing them (PR 113476)

[Martin Jambor <mjambor@suse.cz>]

Hi,

On Mon, Feb 12 2024, Jan Hubicka wrote:
>> Hi,
>> 
>> In PR 113476 we have discovered that ipcp_param_lattices is no longer
>> a POD and should be destructed.  This patch does that, calling
>> destructor on each element of the array containing them when the
>> corresponding summary of a node is freed.  An alternative would be to
>> change the XCNEWVEC-and-placement-new to initializations in
>> constructors of all things in ipcp_param_lattices and then simply use
>> normal operators new and delete.  I am not sure, the initialization
>> through XCNEWVEC may be a bit more efficient although that is probably
>> not a big concern.  In the end, I opted for a simpler solution for
>> stage 4.
>> 
>> I have verified that valgrind no longer reports lost memory blocks
>> allocated within ipcp_vr_lattice::meet_with_1 on the preprocessed source
>> (dwarf2out.i) attached to Bugzilla.  The patch also passes bootstrap and
>> LTO bootstrap and testing on x86_64-linux.
>> 
>> OK for master?
>> 
>> Thanks,
>> 
>> Martin
>> 
>> 
>> gcc/ChangeLog:
>> 
>> 2024-02-09  Martin Jambor  <mjambor@suse.cz>
>> 
>> 	PR tree-optimization/113476
>> 	* ipa-prop.h (ipa_node_params::~ipa_node_params): Moved...
>> 	* ipa-cp.cc (ipa_node_params::~ipa_node_params): ...here.  Added
>> 	destruction of lattices.
>
> OK.
> So you do not use vectors (which would also handle the destruction)
> basically to save space needed to keep the
> size of the vector since that is known from the parameter count?
>

OK, so when I started looking at converting lattices to vector, it
immediately became clear why it is an array.  The type of the element of
the array (ipcp_param_lattices and all it contains) is only forward
declared in ipa-prop.h where ipa_node_params is defined which can
therefore just contain a pointer.  The actual definition of
ipcp_param_lattices is then done only in ipa-cp.c.

Converting the array to a vector would means moving ipcp_param_lattices
together with ipcp_lattice, ipcp_value, ipcp_value_base,
ipcp_agg_lattice, ipcp_bits_lattice, ipcp_vr_lattice from ipa-cp.c to
ipa-prop.h.  Or an ipa-cp.h which ipa-prop.h would require/include.  But
perhaps that is the proper C++ thing to do :-/

Martin