Bug 21639 - [4.1 Regression] poisoned ggc memory used for -ftree-vectorize
Summary: [4.1 Regression] poisoned ggc memory used for -ftree-vectorize
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.1.0
: P2 normal
Target Milestone: 4.1.0
Assignee: Not yet assigned to anyone
URL: http://gcc.gnu.org/ml/gcc-patches/200...
Keywords: GC, ice-on-valid-code, patch
Depends on:
Blocks:
 
Reported: 2005-05-18 00:13 UTC by Janis Johnson
Modified: 2005-08-25 23:12 UTC (History)
4 users (show)

See Also:
Host: powerpc64-linux
Target: powerpc64-linux
Build: powerpc64-linux
Known to work:
Known to fail:
Last reconfirmed: 2005-05-24 13:46:09


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Janis Johnson 2005-05-18 00:13:49 UTC
The SPEC CPU2000 tests bzip2, gap, and gcc cause mainline GCC to
segfault when built on powerpc64-linux with:
                                                                                
  -m32 -O2 -ftree-vectorize -maltivec -mabi=altivec \
  --param ggc-min-expand=0 --param ggc-min-heapsize=0
                                                                                
Every single CPU2000 test fails to build with those options and -m64
instead of -m32.  gap, facerec, and apsi sometimes fail with -m64
without the ggc params defined, as mentioned in comment 4 of PR 21155.
                                                                                
This minimized test case demonstrates the bug:
                                                                                
extern void bar (unsigned char *, int, int);
unsigned char l[6][4];
int c[6][4];
                                                                                
void
foo (int n, int a)
{
  int t, i;
  int x;
                                                                                
  for (t = 0; t < n; t++)
    {
      for (i = 0; i < a; i++)
        x = l[t][i];
      bar (&l[t][0], x, a);
    }
}
                                                                                
gdb shows the segfault at gcc/tree-chrec.c:1048 ct=0xa5a5a5a5.
                                                                                
The backtrace is too messy for cut-and-paste but shows
  chrec_convert
  interpret_rhs_modify_expr
  analyze_scalar_evolution_1
  analyze_scalar_evolution
  simple_iv
  number_of_iterations_exit
  number_of_iterations_in_loop
  canonicalize_loop_induction_variables
  tree_unroll_loops_completely
  tree_complete_unroll
  execute_pass_list
  ...
                                                                                
SPEC CPU2000 with the ggc params set to zero all build with "-O2" and
with "-O2 -ftree-loop-linear".
                                                                                
The failure shows up with this patch from radkver:
                                                                                
  http://gcc.gnu.org/ml/gcc-cvs/2005-05/msg00302.html
                                                                                
Just to make things even more interesting, add "-fprofile-generate"
and the test case starts failing in the same place on 20050428 with
this patch from bonzini, which suggests it's a latent bug unrelated
to these patches:
                                                                                
  http://gcc.gnu.org/ml/gcc-cvs/2005-04/msg01463.html
Comment 1 Dorit Naishlos 2005-05-24 11:53:35 UTC
I can reproduce this problem on powerpc-apple-darwin, even without the 
options "-maltivec -mabi-altivec" (which means that no simd vectorization takes 
place). What happens is that the scalar_evolution_info hash table is corrupted 
between vectorization pass and tree_unroll_loops_completely pass. It looks like 
this corruption takes place during pass_lower_vector_ssa (which is enabled by -
ftree-vectorize), when executing the todo_flags_finish for this pass.

The scalar_evolution_info htab before pass_lower_vector_ssa looks like this:

==============================
after execute pass:
(classify_chrec {0, +, 1}_1
  affine_univariate
)
(classify_chrec 
(classify_chrec {1, +, 1}_2
  affine_univariate
) 
(classify_chrec {0, +, 1}_2
  affine_univariate
)

( 
-----------------------------------------
3       affine univariate chrecs
0       affine multivariate chrecs
0       degree greater than 2 polynomials
0       chrec_dont_know chrecs
-----------------------------------------
4       total chrecs
1       with undetermined coefficients
-----------------------------------------
21      chrecs in the scev database
55      sets in the scev database
73      gets in the scev database
-----------------------------------------
)
==============================


After pass_lower_vector_ssa the scalar_evolution_info htab looks like this:

==============================
(classify_chrec <<< Unknown tree:  >>>

)
(classify_chrec
(classify_chrec <<< Unknown tree:  >>>

)
(classify_chrec <<< Unknown tree:  >>>

)

(
-----------------------------------------
0       affine univariate chrecs
0       affine multivariate chrecs
0       degree greater than 2 polynomials
0       chrec_dont_know chrecs
-----------------------------------------
4       total chrecs
1       with undetermined coefficients
-----------------------------------------
21      chrecs in the scev database
55      sets in the scev database
73      gets in the scev database
-----------------------------------------
)
==============================


Removing the "TODO_ggc_collect" from the todo_flags_finish of 
pass_lower_vector_ssa solves the problem (in tree-complex.c):

==============================
 struct tree_opt_pass pass_lower_vector_ssa = 
 {
   "veclower",                          /* name */
   gate_expand_vector_operations,       /* gate */
   expand_vector_operations,            /* execute */
   NULL,                                        /* sub */
   NULL,                                        /* next */
   0,                                   /* static_pass_number */
   0,                                   /* tv_id */
   PROP_cfg,                            /* properties_required */
   0,                                   /* properties_provided */
   0,                                   /* properties_destroyed */
   0,                                   /* todo_flags_start */
   TODO_dump_func | TODO_update_ssa     /* todo_flags_finish */
-    | TODO_ggc_collect | TODO_verify_ssa
+    | TODO_verify_ssa
     | TODO_verify_stmts | TODO_verify_flow,
   0                                    /* letter */
 };
==============================

Paolo, is the above solution ok with you? If so, I'll go ahead and prepare a 
patch. Alternatively, if ggc_collect is really required here, then adding a 
call to scev_reset() would also solve the problem (again, in tree-complex.c):

==============================
 expand_vector_operations (void)
 {
   block_stmt_iterator bsi;
   basic_block bb;
 
   FOR_EACH_BB (bb)
     {
       for (bsi = bsi_start (bb); !bsi_end_p (bsi); bsi_next (&bsi))
        {
          expand_vector_operations_1 (&bsi);
          update_stmt_if_modified (bsi_stmt (bsi));
        }
     }
+
+  scev_reset ();
+
 }
==============================
Comment 2 paolo.bonzini@lu.unisi.ch 2005-05-24 11:59:46 UTC
Subject: Re:  poisoned ggc memory used for -ftree-vectorize


>Paolo, is the above solution ok with you? If so, I'll go ahead and prepare a 
>patch. Alternatively, if ggc_collect is really required here, then adding a 
>call to scev_reset() would also solve the problem (again, in tree-complex.c):
>  
>
Is there a rule that ggc_collect should not be called during loop 
optimizations?

If not, you should not have to modify my pass since it is only showing 
the problem by doing TODO_ggc_collect.  You have to make sure that 
scev_reset is called at the appropriate time, or that the contents of 
the scalar evolution hash table are reached during garbage collection.  
In other words, Janis is correct saying that this is a latent bug.

Doing a TODO_ggc_collect after lower_vector_ssa may be overkill, and I'm 
ok with removing it, but then you are only papering over the real problem.

If you have some time to spare, use a --enable-checking=gcac compiler, 
and the failure will likely happen much earlier.

Paolo
Comment 3 Dorit Naishlos 2005-05-24 12:54:49 UTC
> Is there a rule that ggc_collect should not be called during loop 
optimizations?

I don't know. (Zdenek?)

I think passes within the loop optimizer can assume that the scev information 
cached in the scev htab can be safely used. During vectorization analysis some 
new entries in the scev htab are initialized, and these are valid to be used by 
subsequent passes. If no transformation takes place during vectorization then 
there's no need to invalidate this information. So I don't think we're missing 
a call to scev_reset in the vectorizer.

The problem is more likely around the fact that ggc_collect, for some reason 
seems to free up data that is still pointed to from scalar_evolution_info. 


> If you have some time to spare, use a --enable-checking=gcac compiler, 
and the failure will likely happen much earlier.

thanks for the tip 


> You have to make sure that ... the contents of 
the scalar evolution hash table are reached during garbage collection.

any (other) hints on how to do that? 

(by the way, I actually found a few places where don't properly free up 
information in the vectorizer, but fixing those didn't solve this particular 
problem).
Comment 4 Paolo Bonzini 2005-05-24 13:46:08 UTC
scev_info_str should be ggc_alloc-ed.

Then, the hash table with the cache should be marked with GTY((param_is (struct
scev_info_str))) 

If this fixes the bug, you can remove the TODO_ggc_collect from lower_vector_ssa.
Comment 5 Zdenek Dvorak 2005-05-24 14:24:14 UTC
Subject: Re:  poisoned ggc memory used for -ftree-vectorize

> > Is there a rule that ggc_collect should not be called during loop 
> optimizations?
> 
> I don't know. (Zdenek?)

there are several places in loop opts that are not GGC-safe (in
particular the tree nodes refered from loop structures are not
seen by garbage collector, and I think there are some other instances).

So at the moment, you cannot run ggc_collect inside loop opts.

> I think passes within the loop optimizer can assume that the scev information 
> cached in the scev htab can be safely used. During vectorization analysis some 
> new entries in the scev htab are initialized, and these are valid to be used by 
> subsequent passes. If no transformation takes place during vectorization then 
> there's no need to invalidate this information. So I don't think we're missing 
> a call to scev_reset in the vectorizer.
> 
> The problem is more likely around the fact that ggc_collect, for some reason 
> seems to free up data that is still pointed to from scalar_evolution_info. 
> 
> 
> > If you have some time to spare, use a --enable-checking=gcac compiler, 
> and the failure will likely happen much earlier.
> 
> thanks for the tip 
> 
> 
> > You have to make sure that ... the contents of 
> the scalar evolution hash table are reached during garbage collection.
> 
> any (other) hints on how to do that? 
> 
> (by the way, I actually found a few places where don't properly free up 
> information in the vectorizer, but fixing those didn't solve this particular 
> problem).
> 
> -- 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |rakdver at atrey dot karlin
>                    |                            |dot mff dot cuni dot cz
> 
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21639
> 
> ------- You are receiving this mail because: -------
> You are on the CC list for the bug, or are watching someone who is.
Comment 6 paolo.bonzini@lu.unisi.ch 2005-05-24 14:26:41 UTC
Subject: Re:  poisoned ggc memory used for -ftree-vectorize


>there are several places in loop opts that are not GGC-safe (in
>particular the tree nodes refered from loop structures are not
>seen by garbage collector, and I think there are some other instances).
>
>So at the moment, you cannot run ggc_collect inside loop opts.
>  
>
Is this going to change?

If not, I guess removing TODO_ggc_collect is the simpliest thing to do.  
But then please a PR on this should be opened, or this one should be 
modified appropriately, as an enhancement request for making loop 
optimizations GGC-safe.

Paolo
Comment 7 Zdenek Dvorak 2005-05-24 15:06:19 UTC
Subject: Re:  poisoned ggc memory used for -ftree-vectorize

> >there are several places in loop opts that are not GGC-safe (in
> >particular the tree nodes refered from loop structures are not
> >seen by garbage collector, and I think there are some other instances).
> >
> >So at the moment, you cannot run ggc_collect inside loop opts.
> >  
> >
> Is this going to change?
> 
> If not, I guess removing TODO_ggc_collect is the simpliest thing to do.  
> But then please a PR on this should be opened, or this one should be 
> modified appropriately, as an enhancement request for making loop 
> optimizations GGC-safe.

It will need some work (most of the loop structures are mallocated at
the moment), so this might indeed be a good idea.
Comment 8 Dorit Naishlos 2005-05-24 15:16:22 UTC
(In reply to comment #7)
> > >So at the moment, you cannot run ggc_collect inside loop opts.
> > >  
> > >
> > Is this going to change?
> > 
> > If not, I guess removing TODO_ggc_collect is the simpliest thing to do.  
> > But then please a PR on this should be opened, or this one should be 
> > modified appropriately, as an enhancement request for making loop 
> > optimizations GGC-safe.
> It will need some work (most of the loop structures are mallocated at
> the moment), so this might indeed be a good idea.

I'll go ahead and prepare the patch then.
Comment 9 Dorit Naishlos 2005-05-26 12:02:04 UTC
patch: http://gcc.gnu.org/ml/gcc-patches/2005-05/msg02477.html

Comment 10 CVS Commits 2005-05-29 06:47:13 UTC
Subject: Bug 21639

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	dorit@gcc.gnu.org	2005-05-29 06:47:08

Modified files:
	gcc            : ChangeLog tree-complex.c 

Log message:
	PR tree-optimization/21639
	* tree-complex.c (pass_lower_vector_s): Remove TODO_ggc_collect.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.8936&r2=2.8937
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/tree-complex.c.diff?cvsroot=gcc&r1=2.24&r2=2.25

Comment 11 Andrew Pinski 2005-05-29 14:40:53 UTC
Fixed.