On Tue, Dec 8, 2015 at 1:01 PM, Richard Biener
<richard.guent...@gmail.com> wrote:
> 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.
Testing a patch.
Richard.
> Thanks,
> Richard.
>
>
>> Martin
