On Tue, Dec 8, 2015 at 12:30 PM, Martin Liška <mli...@suse.cz> wrote:
> Hello.
>
> The patch removes memory leaks that are caused by overwriting an existing
> item in stmt_vec_info_vec (in set_vinfo_for_stmt). My first attempt was to
> call
> free_stmt_vec_info for old entries that are overwritten, but it caused double
> frees
> as there are some references between stmt_vec_infos.
>
> So that I've decided to allocate all stmt_vec_info structures from a memory
> pool, which
> is released in free_stmt_vec_info_vec routine. Replacing 'vec' (used for
> simd_clone_info and same_align_regs) to 'auto_vec'
> helps to reduce another leaks. To be honest, the solution is not ideal as
> destructor
> of there auto_vec is not called, however with the patch applied, there is
> just a single memory leak
> in the whole test-suite related to tree-vect-stmts.c (which is unrelated to
> these vectors).
>
> Patch can bootstrap and survives regression tests on x86_64-unknown-linux-gnu.
>
> Ready for trunk?
new_stmt_vec_info (gimple *stmt, vec_info *vinfo)
{
stmt_vec_info res;
- res = (stmt_vec_info) xcalloc (1, sizeof (struct _stmt_vec_info));
+ res = stmt_vec_info_pool.allocate ();
previously it was zeroed memory now it is no longer (AFAIK).
ICK. You do
+struct _stmt_vec_info
+{
+ _stmt_vec_info (): type ((enum stmt_vec_info_type) 0), live (false),
+ in_pattern_p (false), stmt (NULL), vinfo (0), vectype (NULL_TREE),
+ vectorized_stmt (NULL), data_ref_info (0), dr_base_address (NULL_TREE),
+ dr_init (NULL_TREE), dr_offset (NULL_TREE), dr_step (NULL_TREE),
+ dr_aligned_to (NULL_TREE), loop_phi_evolution_base_unchanged (NULL_TREE),
+ loop_phi_evolution_part (NULL_TREE), related_stmt (NULL),
pattern_def_seq (0),
+ same_align_refs (0), simd_clone_info (0), def_type ((enum vect_def_type) 0),
+ slp_type ((enum slp_vect_type) 0), first_element (NULL), next_element (NULL),
+ same_dr_stmt (NULL), size (0), store_count (0), gap (0), min_neg_dist (0),
+ relevant ((enum vect_relevant) 0), vectorizable (false),
+ gather_scatter_p (false), strided_p (false), simd_lane_access_p (false),
+ v_reduc_type ((enum vect_reduction_type) 0) {}
where I think a
_stmt_vec_info () { memset (this, 0, sizeof (_stmt_vec_info)); }
would be much nicer. Or just keep the struct as a POD and add that memset
at the allocation point. (and double-check the C++ alloc-pool doesn't end up
zeroing everything twice that way...)
Note that overwriting an existing entry in stmt_vec_info_vec should not happen.
In which path does that it happen? We probably should assert the entry
is NULL.
Thus your fix shouldn't be necessary.
Thanks,
Richard.
> Martin
