| Message ID | 20130301200254.GB12913@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, 1 Mar 2013, Jakub Jelinek wrote: > Hi! > > This fixes leaks like > ==31176== 176 bytes in 2 blocks are definitely lost in loss record 432 of 541 > ==31176== at 0x4A0881C: malloc (vg_replace_malloc.c:270) > ==31176== by 0x114DF6F: xrealloc (xmalloc.c:177) > ==31176== by 0x85E0AA: void va_heap::reserve<gimple_statement_d*>(vec<gimple_statement_d*, va_heap, vl_embed>*&, unsigned int, bool) (vec.h:300) > ==31176== by 0x85DE23: vec<gimple_statement_d*, va_heap, vl_ptr>::reserve(unsigned int, bool) (vec.h:1468) > ==31176== by 0xA7961C: vec<gimple_statement_d*, va_heap, vl_ptr>::reserve_exact(unsigned int) (vec.h:1482) > ==31176== by 0xA79402: vec<gimple_statement_d*, va_heap, vl_ptr>::create(unsigned int) (vec.h:1497) > ==31176== by 0xC7189C: new_loop_vec_info(loop*) (tree-vect-loop.c:871) > ==31176== by 0xC725EB: vect_analyze_loop_form(loop*) (tree-vect-loop.c:1251) > ==31176== by 0xC71DEE: vect_analyze_loop_1(loop*) (tree-vect-loop.c:1000) > ==31176== by 0xC71F6D: vect_analyze_loop_form(loop*) (tree-vect-loop.c:1098) > ==31176== by 0xC73815: vect_analyze_loop(loop*) (tree-vect-loop.c:1764) > ==31176== by 0xC8F9EA: vectorize_loops() (tree-vectorizer.c:113) > and various others, when vect_analyze_loop_form calls destroy_loop_vec_info > with false as second argument, we leak various fields. My understanding > is that back when clean_stmts argument has been added, the if (!clean_stmts) > switch got a copy of all the cleanups the other code path did, except for the loop > actually freeing stmt_vec_info, but already shortly after it new fields > have been added and cleanup for those has been added to just one spot instead > of two. > > The patch fixes it by doing all the cleanups for !clean_stmts too in the > same code path, except for the stmt_vec_info freeing. With -O3 -mavx > I didn't see any code differences on all of gcc.dg/vect/*.c. > > Also, passing false to the second to last call to destroy_loop_vec_info > where we return NULL looks like a typo to my eyes, so I've changed that too. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Ok. Thanks, Richard. > 2013-03-01 Jakub Jelinek <jakub@redhat.com> > > PR middle-end/56461 > * tree-vect-loop.c (destroy_loop_vec_info): For !clean_stmts, just > set nbbs to 0 instead of having separate code path. > (vect_analyze_loop_form): Call destroy_loop_vec_info with true > instead of false as last argument if returning NULL. > > --- gcc/tree-vect-loop.c.jj 2013-02-27 22:40:24.000000000 +0100 > +++ gcc/tree-vect-loop.c 2013-03-01 11:27:53.205162019 +0100 > @@ -905,23 +905,9 @@ destroy_loop_vec_info (loop_vec_info loo > loop = LOOP_VINFO_LOOP (loop_vinfo); > > bbs = LOOP_VINFO_BBS (loop_vinfo); > - nbbs = loop->num_nodes; > + nbbs = clean_stmts ? loop->num_nodes : 0; > swapped = LOOP_VINFO_OPERANDS_SWAPPED (loop_vinfo); > > - if (!clean_stmts) > - { > - free (LOOP_VINFO_BBS (loop_vinfo)); > - free_data_refs (LOOP_VINFO_DATAREFS (loop_vinfo)); > - free_dependence_relations (LOOP_VINFO_DDRS (loop_vinfo)); > - LOOP_VINFO_LOOP_NEST (loop_vinfo).release (); > - LOOP_VINFO_MAY_MISALIGN_STMTS (loop_vinfo).release (); > - LOOP_VINFO_MAY_ALIAS_DDRS (loop_vinfo).release (); > - > - free (loop_vinfo); > - loop->aux = NULL; > - return; > - } > - > for (j = 0; j < nbbs; j++) > { > basic_block bb = bbs[j]; > @@ -1244,7 +1230,7 @@ vect_analyze_loop_form (struct loop *loo > dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, > "not vectorized: number of iterations = 0."); > if (inner_loop_vinfo) > - destroy_loop_vec_info (inner_loop_vinfo, false); > + destroy_loop_vec_info (inner_loop_vinfo, true); > return NULL; > } > > > Jakub > >
--- gcc/tree-vect-loop.c.jj 2013-02-27 22:40:24.000000000 +0100 +++ gcc/tree-vect-loop.c 2013-03-01 11:27:53.205162019 +0100 @@ -905,23 +905,9 @@ destroy_loop_vec_info (loop_vec_info loo loop = LOOP_VINFO_LOOP (loop_vinfo); bbs = LOOP_VINFO_BBS (loop_vinfo); - nbbs = loop->num_nodes; + nbbs = clean_stmts ? loop->num_nodes : 0; swapped = LOOP_VINFO_OPERANDS_SWAPPED (loop_vinfo); - if (!clean_stmts) - { - free (LOOP_VINFO_BBS (loop_vinfo)); - free_data_refs (LOOP_VINFO_DATAREFS (loop_vinfo)); - free_dependence_relations (LOOP_VINFO_DDRS (loop_vinfo)); - LOOP_VINFO_LOOP_NEST (loop_vinfo).release (); - LOOP_VINFO_MAY_MISALIGN_STMTS (loop_vinfo).release (); - LOOP_VINFO_MAY_ALIAS_DDRS (loop_vinfo).release (); - - free (loop_vinfo); - loop->aux = NULL; - return; - } - for (j = 0; j < nbbs; j++) { basic_block bb = bbs[j]; @@ -1244,7 +1230,7 @@ vect_analyze_loop_form (struct loop *loo dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, "not vectorized: number of iterations = 0."); if (inner_loop_vinfo) - destroy_loop_vec_info (inner_loop_vinfo, false); + destroy_loop_vec_info (inner_loop_vinfo, true); return NULL; }