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 <mjam...@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
