Re: rfa: remove get_var_ann (was: Fix PR50260)

2011-10-06 Thread Michael Matz 
Hi,

On Sat, 3 Sep 2011, Richard Guenther wrote:

  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.

Okay, like so then.  Regstrapped on x86_64-linux.  (Note that sometimes I 
use add_referenced_vars, and sometimes find_referenced_vars_in, the latter 
when I would have to add several add_referenced_vars for one statement).

 IIRC we have some verification code even, and wonder why it doesn't 
 trigger.

Nope, we don't.  But with the patch we segfault in case this happens 
again, which is good enough checking for me.


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-profile.c (gimple_gen_edge_profiler): Call
find_referenced_var_in.
(gimple_gen_interval_profiler): Ditto.
(gimple_gen_pow2_profiler): Ditto.
(gimple_gen_one_value_profiler): Ditto.
(gimple_gen_average_profiler): Ditto.
(gimple_gen_ior_profiler): Ditto.
(gimple_gen_ic_profiler): Ditto plus call add_referenced_var.
(gimple_gen_ic_func_profiler): Call add_referenced_var.
* tree-mudflap.c (execute_mudflap_function_ops): Call
add_referenced_var.

Index: tree-flow.h
===
--- tree-flow.h (revision 178488)
+++ tree-flow.h (working copy)
@@ -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-flow-inline.h
===
--- tree-flow-inline.h  (revision 178488)
+++ tree-flow-inline.h  (working copy)
@@ -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-dfa.c
===
--- tree-dfa.c  (revision 178488)
+++ tree-dfa.c  (working copy)
@@ -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-profile.c
===
--- tree-profile.c  (revision 178408)
+++ tree-profile.c  (working copy)
@@ -224,6 +224,7 @@ gimple_gen_edge_profiler (int edgeno, ed
   one = build_int_cst (gcov_type_node, 1);
   stmt1 = gimple_build_assign (gcov_type_tmp_var, ref);
   gimple_assign_set_lhs (stmt1, make_ssa_name (gcov_type_tmp_var, stmt1));
+  find_referenced_vars_in (stmt1);
   stmt2 = gimple_build_assign_with_ops (PLUS_EXPR, gcov_type_tmp_var,
gimple_assign_lhs (stmt1), one);
   gimple_assign_set_lhs (stmt2, make_ssa_name (gcov_type_tmp_var, stmt2));
@@ -270,6 +271,7 @@ gimple_gen_interval_profiler (histogram_
   val = prepare_instrumented_value (gsi, value);
   call = gimple_build_call (tree_interval_profiler_fn, 4,
ref_ptr, val, start, steps);
+  find_referenced_vars_in (call);
   gsi_insert_before (gsi, call, GSI_NEW_STMT);
 }
 
@@ -290,6 +292,7 @@ gimple_gen_pow2_profiler (histogram_valu
  true, NULL_TREE, true, GSI_SAME_STMT);
   val = prepare_instrumented_value (gsi, value);
   call = gimple_build_call (tree_pow2_profiler_fn, 2, ref_ptr, val);
+  find_referenced_vars_in (call);
   gsi_insert_before (gsi, call, GSI_NEW_STMT);
 }
 
@@ -310,6 +313,7 @@ gimple_gen_one_value_profiler (histogram
  true, NULL_TREE, true, GSI_SAME_STMT);
   val = prepare_instrumented_value (gsi, value);
   call = gimple_build_call (tree_one_value_profiler_fn, 2, ref_ptr, val);
+  find_referenced_vars_in (call);
   gsi_insert_before

Re: rfa: remove get_var_ann (was: Fix PR50260)

2011-10-06 Thread Richard Guenther 
On Thu, Oct 6, 2011 at 4:59 PM, Michael Matz m...@suse.de wrote:
 Hi,

 On Sat, 3 Sep 2011, Richard Guenther wrote:

  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.

 Okay, like so then.  Regstrapped on x86_64-linux.  (Note that sometimes I
 use add_referenced_vars, and sometimes find_referenced_vars_in, the latter
 when I would have to add several add_referenced_vars for one statement).

 IIRC we have some verification code even, and wonder why it doesn't
 trigger.

 Nope, we don't.  But with the patch we segfault in case this happens
 again, which is good enough checking for me.

Ok.

Thanks,
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-profile.c (gimple_gen_edge_profiler): Call
        find_referenced_var_in.
        (gimple_gen_interval_profiler): Ditto.
        (gimple_gen_pow2_profiler): Ditto.
        (gimple_gen_one_value_profiler): Ditto.
        (gimple_gen_average_profiler): Ditto.
        (gimple_gen_ior_profiler): Ditto.
        (gimple_gen_ic_profiler): Ditto plus call add_referenced_var.
        (gimple_gen_ic_func_profiler): Call add_referenced_var.
        * tree-mudflap.c (execute_mudflap_function_ops): Call
        add_referenced_var.

 Index: tree-flow.h
 ===
 --- tree-flow.h (revision 178488)
 +++ tree-flow.h (working copy)
 @@ -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-flow-inline.h
 ===
 --- tree-flow-inline.h  (revision 178488)
 +++ tree-flow-inline.h  (working copy)
 @@ -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-dfa.c
 ===
 --- tree-dfa.c  (revision 178488)
 +++ tree-dfa.c  (working copy)
 @@ -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-profile.c
 ===
 --- tree-profile.c      (revision 178408)
 +++ tree-profile.c      (working copy)
 @@ -224,6 +224,7 @@ gimple_gen_edge_profiler (int edgeno, ed
   one = build_int_cst (gcov_type_node, 1);
   stmt1 = gimple_build_assign (gcov_type_tmp_var, ref);
   gimple_assign_set_lhs (stmt1, make_ssa_name (gcov_type_tmp_var, stmt1));
 +  find_referenced_vars_in (stmt1);
   stmt2 = gimple_build_assign_with_ops (PLUS_EXPR, gcov_type_tmp_var,
                                        gimple_assign_lhs (stmt1), one);
   gimple_assign_set_lhs (stmt2, make_ssa_name (gcov_type_tmp_var, stmt2));
 @@ -270,6 +271,7 @@ gimple_gen_interval_profiler (histogram_
   val = prepare_instrumented_value (gsi, value);
   call = gimple_build_call (tree_interval_profiler_fn, 4,
                            ref_ptr, val, start, steps);
 +  find_referenced_vars_in (call);
   gsi_insert_before (gsi, call, GSI_NEW_STMT);
  }

 @@ -290,6 +292,7 @@ gimple_gen_pow2_profiler (histogram_valu
                                      true, NULL_TREE, true, GSI_SAME_STMT);
   val = prepare_instrumented_value (gsi, value);
   call = gimple_build_call (tree_pow2_profiler_fn, 2, ref_ptr, val);
 +  find_referenced_vars_in (call);
   gsi_insert_before (gsi, call, GSI_NEW_STMT);
  }

 @@ -310,6 +313,7 @@ gimple_gen_one_value_profiler (histogram
                                      true, NULL_TREE, true, GSI_SAME_STMT);
   val = prepare_instrumented_value

Re: rfa: remove get_var_ann (was: Fix PR50260)

2011-09-03 Thread Richard Guenther 
On Sat, Sep 3, 2011 at 3:51 AM, Michael Matz m...@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.  */

Re: rfa: remove get_var_ann (was: Fix PR50260)

2011-09-03 Thread Michael Matz 
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.

Re: rfa: remove get_var_ann (was: Fix PR50260)

2011-09-03 Thread Richard Guenther 
On Sat, Sep 3, 2011 at 5:31 PM, Michael Matz m...@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.

rfa: remove get_var_ann (was: Fix PR50260)

2011-09-02 Thread Michael Matz 
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?


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.  */