Message ID | Pine.LNX.4.64.1109030345450.25354@wotan.suse.de |
---|---|
State | New |
Headers | show |
On Sat, Sep 3, 2011 at 3:51 AM, Michael Matz <matz@suse.de> wrote: > Hi, > > On Fri, 2 Sep 2011, Michael Matz wrote: > >> > > Ok. Time to make get_var_ann private? >> >> As I feared the call to get_var_ann in set_is_used right now really is >> still needed, privatizing it hence isn't that straight forward. > > After pondering I concluded that it's not necessary to call set_is_used > for variables without var annotation. Those won't be in referenced_vars > (or local_decls) and therefore also won't be removed from those lists no > matter how hard we try. > > Regstrapped on x86_64-linux (without Ada). Okay for trunk? No. We call mark_all_vars_used on trees contained in GIMPLE non-debug statements. All vars we can reach that way _have_ to be in the list of referenced vars. That they are not is the bug. So - can you investigate which var doesn't have an annotation an why it isn't in referenced vars? Richard. > > Ciao, > Michael. > -- > * tree-flow.h (get_var_ann): Don't declare. > * tree-flow-inline.h (get_var_ann): Remove. > (set_is_used): Use var_ann, not get_var_ann. > * tree-dfa.c (add_referenced_var): Inline body of get_var_ann. > * tree-ssa-live.c (mark_all_vars_used_1): Check var_ann before > calling set_is_used. > > Index: tree-flow-inline.h > =================================================================== > --- tree-flow-inline.h (Revision 178488) > +++ tree-flow-inline.h (Arbeitskopie) > @@ -145,16 +145,6 @@ var_ann (const_tree t) > return p ? *p : NULL; > } > > -/* Return the variable annotation for T, which must be a _DECL node. > - Create the variable annotation if it doesn't exist. */ > -static inline var_ann_t > -get_var_ann (tree var) > -{ > - var_ann_t *p = DECL_VAR_ANN_PTR (var); > - gcc_checking_assert (p); > - return *p ? *p : create_var_ann (var); > -} > - > /* Get the number of the next statement uid to be allocated. */ > static inline unsigned int > gimple_stmt_max_uid (struct function *fn) > @@ -568,7 +558,7 @@ phi_arg_index_from_use (use_operand_p us > static inline void > set_is_used (tree var) > { > - var_ann_t ann = get_var_ann (var); > + var_ann_t ann = var_ann (var); > ann->used = true; > } > > Index: tree-flow.h > =================================================================== > --- tree-flow.h (Revision 178488) > +++ tree-flow.h (Arbeitskopie) > @@ -278,7 +278,6 @@ typedef struct immediate_use_iterator_d > typedef struct var_ann_d *var_ann_t; > > static inline var_ann_t var_ann (const_tree); > -static inline var_ann_t get_var_ann (tree); > static inline void update_stmt (gimple); > static inline int get_lineno (const_gimple); > > Index: tree-dfa.c > =================================================================== > --- tree-dfa.c (Revision 178488) > +++ tree-dfa.c (Arbeitskopie) > @@ -580,8 +580,9 @@ set_default_def (tree var, tree def) > bool > add_referenced_var (tree var) > { > - get_var_ann (var); > gcc_assert (DECL_P (var)); > + if (!*DECL_VAR_ANN_PTR (var)) > + create_var_ann (var); > > /* Insert VAR into the referenced_vars hash table if it isn't present. */ > if (referenced_var_check_and_insert (var)) > Index: tree-ssa-live.c > =================================================================== > --- tree-ssa-live.c (Revision 178408) > +++ tree-ssa-live.c (Arbeitskopie) > @@ -376,7 +376,10 @@ mark_all_vars_used_1 (tree *tp, int *wal > { > if (data != NULL && bitmap_clear_bit ((bitmap) data, DECL_UID (t))) > mark_all_vars_used (&DECL_INITIAL (t), data); > - set_is_used (t); > + /* If something has no annotation it's neither in referenced_vars, > + nor in local_decls. No use in marking it as used. */ > + if (var_ann (t)) > + set_is_used (t); > } > /* remove_unused_scope_block_p requires information about labels > which are not DECL_IGNORED_P to tell if they might be used in the IL. */ >
Hi, On Sat, 3 Sep 2011, Richard Guenther wrote: > >> As I feared the call to get_var_ann in set_is_used right now really > >> is still needed, privatizing it hence isn't that straight forward. > > > > After pondering I concluded that it's not necessary to call > > set_is_used for variables without var annotation. Those won't be in > > referenced_vars (or local_decls) and therefore also won't be removed > > from those lists no matter how hard we try. > > > > Regstrapped on x86_64-linux (without Ada). Okay for trunk? > > No. We call mark_all_vars_used on trees contained in GIMPLE non-debug > statements. All vars we can reach that way _have_ to be in the list of > referenced vars. Sometimes global variables (non-auto vars with context != cfun) can be missing from referenced_vars. In the case where we hit bugs with this it's the non-local variables generated for profile counters. I can add the respective add_referenced_var calls for that, but I'm not sure I see the point. That of course comes back to our old discussion for what purposes referenced_vars actually is used. I looked at all users and I think that the global counters missing from referenced_vars is harmless. OTOH it's a nice invariant that can actually be checked for (that all reachable vars whatsoever have to be in referenced_vars), so I'm going to do that. > That they are not is the bug. So - can you investigate > which var doesn't have an annotation an why it isn't in referenced vars? Ciao, Michael.
On Sat, Sep 3, 2011 at 5:31 PM, Michael Matz <matz@suse.de> wrote: > Hi, > > On Sat, 3 Sep 2011, Richard Guenther wrote: > >> >> As I feared the call to get_var_ann in set_is_used right now really >> >> is still needed, privatizing it hence isn't that straight forward. >> > >> > After pondering I concluded that it's not necessary to call >> > set_is_used for variables without var annotation. Those won't be in >> > referenced_vars (or local_decls) and therefore also won't be removed >> > from those lists no matter how hard we try. >> > >> > Regstrapped on x86_64-linux (without Ada). Okay for trunk? >> >> No. We call mark_all_vars_used on trees contained in GIMPLE non-debug >> statements. All vars we can reach that way _have_ to be in the list of >> referenced vars. > > Sometimes global variables (non-auto vars with context != cfun) can be > missing from referenced_vars. In the case where we hit bugs with this > it's the non-local variables generated for profile counters. I can add > the respective add_referenced_var calls for that, but I'm not sure I see > the point. That of course comes back to our old discussion for what > purposes referenced_vars actually is used. I looked at all users and I > think that the global counters missing from referenced_vars is harmless. > > OTOH it's a nice invariant that can actually be checked for (that all > reachable vars whatsoever have to be in referenced_vars), so I'm going to > do that. Yes, until we get rid of referenced_vars (which we still should do at some point...) that's the best. IIRC we have some verification code even, and wonder why it doesn't trigger. Richard. >> That they are not is the bug. So - can you investigate >> which var doesn't have an annotation an why it isn't in referenced vars? > > > Ciao, > Michael.
Index: tree-flow-inline.h =================================================================== --- tree-flow-inline.h (Revision 178488) +++ tree-flow-inline.h (Arbeitskopie) @@ -145,16 +145,6 @@ var_ann (const_tree t) return p ? *p : NULL; } -/* Return the variable annotation for T, which must be a _DECL node. - Create the variable annotation if it doesn't exist. */ -static inline var_ann_t -get_var_ann (tree var) -{ - var_ann_t *p = DECL_VAR_ANN_PTR (var); - gcc_checking_assert (p); - return *p ? *p : create_var_ann (var); -} - /* Get the number of the next statement uid to be allocated. */ static inline unsigned int gimple_stmt_max_uid (struct function *fn) @@ -568,7 +558,7 @@ phi_arg_index_from_use (use_operand_p us static inline void set_is_used (tree var) { - var_ann_t ann = get_var_ann (var); + var_ann_t ann = var_ann (var); ann->used = true; } Index: tree-flow.h =================================================================== --- tree-flow.h (Revision 178488) +++ tree-flow.h (Arbeitskopie) @@ -278,7 +278,6 @@ typedef struct immediate_use_iterator_d typedef struct var_ann_d *var_ann_t; static inline var_ann_t var_ann (const_tree); -static inline var_ann_t get_var_ann (tree); static inline void update_stmt (gimple); static inline int get_lineno (const_gimple); Index: tree-dfa.c =================================================================== --- tree-dfa.c (Revision 178488) +++ tree-dfa.c (Arbeitskopie) @@ -580,8 +580,9 @@ set_default_def (tree var, tree def) bool add_referenced_var (tree var) { - get_var_ann (var); gcc_assert (DECL_P (var)); + if (!*DECL_VAR_ANN_PTR (var)) + create_var_ann (var); /* Insert VAR into the referenced_vars hash table if it isn't present. */ if (referenced_var_check_and_insert (var)) Index: tree-ssa-live.c =================================================================== --- tree-ssa-live.c (Revision 178408) +++ tree-ssa-live.c (Arbeitskopie) @@ -376,7 +376,10 @@ mark_all_vars_used_1 (tree *tp, int *wal { if (data != NULL && bitmap_clear_bit ((bitmap) data, DECL_UID (t))) mark_all_vars_used (&DECL_INITIAL (t), data); - set_is_used (t); + /* If something has no annotation it's neither in referenced_vars, + nor in local_decls. No use in marking it as used. */ + if (var_ann (t)) + set_is_used (t); } /* remove_unused_scope_block_p requires information about labels which are not DECL_IGNORED_P to tell if they might be used in the IL. */