public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [RFC][SSA] Iterator to visit SSA
@ 2016-09-05  5:39 Kugan Vivekanandarajah
  2016-09-05  7:58 ` Richard Biener
  0 siblings, 1 reply; 14+ messages in thread
From: Kugan Vivekanandarajah @ 2016-09-05  5:39 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1787 bytes --]

Hi All,

While looking at gcc source, I noticed that we are iterating over SSA
variable from 0 to num_ssa_names in some cases and 1 to num_ssa_names
in others. It seems that variable 0 is always NULL TREE. But it can
confuse people who are looking for the first time. Therefore It might
be good to follow some consistent usage here.

It might be also good to gave a FOR_EACH_SSAVAR iterator as we do in
other case. Here is attempt to do this based on what is done in other
places. Bootstrapped and regression tested on X86_64-linux-gnu with no
new regressions. is this OK?

Thanks,
Kugan


gcc/ChangeLog:

2016-09-05  Kugan Vivekanandarajah  <kuganv@linaro.org>

    * tree-ssanames.h (ssa_iterator::ssa_iterator): New.
    (ssa_iterator::get): Likewise.
    (ssa_iterator::next): Likewise.
    (FOR_EACH_SSAVAR): Likewise.
    * cfgexpand.c (update_alias_info_with_stack_vars): Use
    FOR_EACH_SSAVAR to iterate over SSA variables.
    (pass_expand::execute): Likewise.
    * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
    * tree-cfg.c (dump_function_to_file): Likewise.
    * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
    (update_ssa): Likewise.
    * tree-ssa-alias.c (dump_alias_info): Likewise.
    * tree-ssa-ccp.c (ccp_finalize): Likewise.
    * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
    (create_outofssa_var_map): Likewise.
    (coalesce_ssa_name): Likewise.
    * tree-ssa-operands.c (dump_immediate_uses): Likewise.
    * tree-ssa-pre.c (compute_avail): Likewise.
    * tree-ssa-sccvn.c (init_scc_vn): Likewise.
    (scc_vn_restore_ssa_info): Likewise.
    (free_scc_vn): Likwise.
    (run_scc_vn): Likewise.
    * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
    * tree-ssa-ter.c (new_temp_expr_table): Likewise.

[-- Attachment #2: ssa_iterator.txt --]
[-- Type: text/plain, Size: 13974 bytes --]

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 130a16b..66d8ba5 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -814,13 +814,12 @@ update_alias_info_with_stack_vars (void)
      contain all members of the partition.  */
   if (decls_to_partitions)
     {
-      unsigned i;
+      tree name;
       hash_set<bitmap> visited;
       bitmap temp = BITMAP_ALLOC (&stack_var_bitmap_obstack);
 
-      for (i = 1; i < num_ssa_names; i++)
+      FOR_EACH_SSAVAR (name)
 	{
-	  tree name = ssa_name (i);
 	  struct ptr_info_def *pi;
 
 	  if (name
@@ -6162,7 +6161,7 @@ pass_expand::execute (function *fun)
   edge_iterator ei;
   edge e;
   rtx_insn *var_seq, *var_ret_seq;
-  unsigned i;
+  tree name;
 
   timevar_push (TV_OUT_OF_SSA);
   rewrite_out_of_ssa (&SA);
@@ -6270,10 +6269,8 @@ pass_expand::execute (function *fun)
 
   /* Now propagate the RTL assignment of each partition to the
      underlying var of each SSA_NAME.  */
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSAVAR (name)
     {
-      tree name = ssa_name (i);
-
       if (!name
 	  /* We might have generated new SSA names in
 	     update_alias_info_with_stack_vars.  They will have a NULL
@@ -6288,9 +6285,8 @@ pass_expand::execute (function *fun)
   /* Clean up RTL of variables that straddle across multiple
      partitions, and check that the rtl of any PARM_DECLs that are not
      cleaned up is that of their default defs.  */
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSAVAR (name)
     {
-      tree name = ssa_name (i);
       int part;
 
       if (!name
diff --git a/gcc/omp-simd-clone.c b/gcc/omp-simd-clone.c
index 467d872..cedd60d 100644
--- a/gcc/omp-simd-clone.c
+++ b/gcc/omp-simd-clone.c
@@ -888,6 +888,7 @@ ipa_simd_modify_function_body (struct cgraph_node *node,
 {
   basic_block bb;
   unsigned int i, j, l;
+  tree name;
 
   /* Re-use the adjustments array, but this time use it to replace
      every function argument use to an offset into the corresponding
@@ -911,9 +912,8 @@ ipa_simd_modify_function_body (struct cgraph_node *node,
     }
 
   l = adjustments.length ();
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSAVAR (name)
     {
-      tree name = ssa_name (i);
       if (name
 	  && SSA_NAME_VAR (name)
 	  && TREE_CODE (SSA_NAME_VAR (name)) == PARM_DECL)
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 57c8410..09f9f7e 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -7539,6 +7539,7 @@ dump_function_to_file (tree fndecl, FILE *file, int flags)
   if (fun && fun->decl == fndecl && (fun->curr_properties & PROP_gimple_lcf))
     {
       unsigned ix;
+      tree name;
       ignore_topmost_bind = true;
 
       fprintf (file, "{\n");
@@ -7582,9 +7583,8 @@ dump_function_to_file (tree fndecl, FILE *file, int flags)
 	    any_var = true;
 	  }
       if (gimple_in_ssa_p (cfun))
-	for (ix = 1; ix < num_ssa_names; ++ix)
+	FOR_EACH_SSAVAR (name)
 	  {
-	    tree name = ssa_name (ix);
 	    if (name && !SSA_NAME_VAR (name))
 	      {
 		fprintf (file, "  ");
diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
index ceafa68..56214b6 100644
--- a/gcc/tree-into-ssa.c
+++ b/gcc/tree-into-ssa.c
@@ -2341,7 +2341,7 @@ pass_build_ssa::execute (function *fun)
 {
   bitmap_head *dfs;
   basic_block bb;
-  unsigned i;
+  tree decl, name;
 
   /* Initialize operand data structures.  */
   init_ssa_operands (fun);
@@ -2385,9 +2385,8 @@ pass_build_ssa::execute (function *fun)
   /* Try to get rid of all gimplifier generated temporaries by making
      its SSA names anonymous.  This way we can garbage collect them
      all after removing unused locals which we do in our TODO.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  FOR_EACH_SSAVAR (name)
     {
-      tree decl, name = ssa_name (i);
       if (!name
 	  || SSA_NAME_IS_DEFAULT_DEF (name))
 	continue;
@@ -3165,7 +3164,7 @@ update_ssa (unsigned update_flags)
   unsigned i = 0;
   bool insert_phi_p;
   sbitmap_iterator sbi;
-  tree sym;
+  tree sym, name;
 
   /* Only one update flag should be set.  */
   gcc_assert (update_flags == TODO_update_ssa
@@ -3284,9 +3283,8 @@ update_ssa (unsigned update_flags)
       prepare_block_for_update (start_bb, insert_phi_p);
 
       if (flag_checking)
-	for (i = 1; i < num_ssa_names; ++i)
+	FOR_EACH_SSAVAR (name)
 	  {
-	    tree name = ssa_name (i);
 	    if (!name
 		|| virtual_operand_p (name))
 	      continue;
diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 8051a66..0a55865 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -450,6 +450,7 @@ void
 dump_alias_info (FILE *file)
 {
   unsigned i;
+  tree ptr;
   const char *funcname
     = lang_hooks.decl_printable_name (current_function_decl, 2);
   tree var;
@@ -471,9 +472,8 @@ dump_alias_info (FILE *file)
 
   fprintf (file, "\n\nFlow-insensitive points-to information\n\n");
 
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSAVAR (ptr)
     {
-      tree ptr = ssa_name (i);
       struct ptr_info_def *pi;
 
       if (ptr == NULL_TREE
diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c
index 9871304..681da85 100644
--- a/gcc/tree-ssa-ccp.c
+++ b/gcc/tree-ssa-ccp.c
@@ -897,16 +897,15 @@ static bool
 ccp_finalize (bool nonzero_p) 
 {
   bool something_changed;
-  unsigned i;
+  tree name;
 
   do_dbg_cnt ();
 
   /* Derive alignment and misalignment information from partially
      constant pointers in the lattice or nonzero bits from partially
      constant integers.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  FOR_EACH_SSAVAR (name)
     {
-      tree name = ssa_name (i);
       ccp_prop_value_t *val;
       unsigned int tem, align;
 
diff --git a/gcc/tree-ssa-coalesce.c b/gcc/tree-ssa-coalesce.c
index 34c3fa1..9907d55 100644
--- a/gcc/tree-ssa-coalesce.c
+++ b/gcc/tree-ssa-coalesce.c
@@ -954,11 +954,10 @@ build_ssa_conflict_graph (tree_live_info_p liveinfo)
 	 RTL to the same partition.  */
       if (bb == entry)
 	{
-	  unsigned i;
-	  for (i = 1; i < num_ssa_names; i++)
-	    {
-	      tree var = ssa_name (i);
+	  tree var;
 
+	  FOR_EACH_SSAVAR (var)
+	    {
 	      if (!var
 		  || !SSA_NAME_IS_DEFAULT_DEF (var)
 		  || !SSA_NAME_VAR (var)
@@ -1090,7 +1089,6 @@ create_outofssa_var_map (coalesce_list *cl, bitmap used_in_copy)
   var_map map;
   ssa_op_iter iter;
   int v1, v2, cost;
-  unsigned i;
 
   for_all_parms (create_default_def, NULL);
 
@@ -1261,9 +1259,8 @@ create_outofssa_var_map (coalesce_list *cl, bitmap used_in_copy)
   /* Now process result decls and live on entry variables for entry into
      the coalesce list.  */
   first = NULL_TREE;
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSAVAR (var)
     {
-      var = ssa_name (i);
       if (var != NULL_TREE && !virtual_operand_p (var))
         {
 	  coalesce_with_default (var, cl, used_in_copy);
@@ -1805,7 +1802,7 @@ coalesce_ssa_name (void)
   coalesce_list *cl;
   bitmap used_in_copies = BITMAP_ALLOC (NULL);
   var_map map;
-  unsigned int i;
+  tree a;
 
   cl = create_coalesce_list ();
   map = create_outofssa_var_map (cl, used_in_copies);
@@ -1817,10 +1814,8 @@ coalesce_ssa_name (void)
     {
       hash_table<ssa_name_var_hash> ssa_name_hash (10);
 
-      for (i = 1; i < num_ssa_names; i++)
+      FOR_EACH_SSAVAR (a)
 	{
-	  tree a = ssa_name (i);
-
 	  if (a
 	      && SSA_NAME_VAR (a)
 	      && !DECL_IGNORED_P (SSA_NAME_VAR (a))
diff --git a/gcc/tree-ssa-operands.c b/gcc/tree-ssa-operands.c
index eccea2f..3480639 100644
--- a/gcc/tree-ssa-operands.c
+++ b/gcc/tree-ssa-operands.c
@@ -1244,12 +1244,10 @@ void
 dump_immediate_uses (FILE *file)
 {
   tree var;
-  unsigned int x;
 
   fprintf (file, "Immediate_uses: \n\n");
-  for (x = 1; x < num_ssa_names; x++)
+  FOR_EACH_SSAVAR (var)
     {
-      var = ssa_name (x);
       if (!var)
         continue;
       dump_immediate_uses_for (file, var);
diff --git a/gcc/tree-ssa-pre.c b/gcc/tree-ssa-pre.c
index fdb1c2c..3c1d95f 100644
--- a/gcc/tree-ssa-pre.c
+++ b/gcc/tree-ssa-pre.c
@@ -3669,13 +3669,12 @@ compute_avail (void)
   basic_block block, son;
   basic_block *worklist;
   size_t sp = 0;
-  unsigned i;
+  tree name;
 
   /* We pretend that default definitions are defined in the entry block.
      This includes function arguments and the static chain decl.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  FOR_EACH_SSAVAR (name)
     {
-      tree name = ssa_name (i);
       pre_expr e;
       if (!name
 	  || !SSA_NAME_IS_DEFAULT_DEF (name)
diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c
index 21b3d56..d9cb2e7 100644
--- a/gcc/tree-ssa-sccvn.c
+++ b/gcc/tree-ssa-sccvn.c
@@ -4290,7 +4290,7 @@ free_vn_table (vn_tables_t table)
 static void
 init_scc_vn (void)
 {
-  size_t i;
+  tree name;
   int j;
   int *rpo_numbers_temp;
 
@@ -4339,9 +4339,8 @@ init_scc_vn (void)
 
   /* Create the VN_INFO structures, and initialize value numbers to
      TOP or VARYING for parameters.  */
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSAVAR (name)
     {
-      tree name = ssa_name (i);
       if (!name)
 	continue;
 
@@ -4402,9 +4401,10 @@ init_scc_vn (void)
 void
 scc_vn_restore_ssa_info (void)
 {
-  for (unsigned i = 0; i < num_ssa_names; i++)
+  tree name;
+
+  FOR_EACH_SSAVAR (name)
     {
-      tree name = ssa_name (i);
       if (name
 	  && has_VN_INFO (name))
 	{
@@ -4427,7 +4427,7 @@ scc_vn_restore_ssa_info (void)
 void
 free_scc_vn (void)
 {
-  size_t i;
+  tree name;
 
   delete constant_to_value_id;
   constant_to_value_id = NULL;
@@ -4436,9 +4436,8 @@ free_scc_vn (void)
   shared_lookup_references.release ();
   XDELETEVEC (rpo_numbers);
 
-  for (i = 0; i < num_ssa_names; i++)
+  FOR_EACH_SSAVAR (name)
     {
-      tree name = ssa_name (i);
       if (name
 	  && has_VN_INFO (name)
 	  && VN_INFO (name)->needs_insertion)
@@ -4757,6 +4756,7 @@ bool
 run_scc_vn (vn_lookup_kind default_vn_walk_kind_)
 {
   size_t i;
+  tree name;
 
   default_vn_walk_kind = default_vn_walk_kind_;
 
@@ -4797,9 +4797,8 @@ run_scc_vn (vn_lookup_kind default_vn_walk_kind_)
 
   /* Initialize the value ids and prune out remaining VN_TOPs
      from dead code.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  FOR_EACH_SSAVAR (name)
     {
-      tree name = ssa_name (i);
       vn_ssa_aux_t info;
       if (!name)
 	continue;
@@ -4814,9 +4813,8 @@ run_scc_vn (vn_lookup_kind default_vn_walk_kind_)
     }
 
   /* Propagate.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  FOR_EACH_SSAVAR (name)
     {
-      tree name = ssa_name (i);
       vn_ssa_aux_t info;
       if (!name)
 	continue;
@@ -4832,9 +4830,8 @@ run_scc_vn (vn_lookup_kind default_vn_walk_kind_)
   if (dump_file && (dump_flags & TDF_DETAILS))
     {
       fprintf (dump_file, "Value numbers:\n");
-      for (i = 0; i < num_ssa_names; i++)
+      FOR_EACH_SSAVAR (name)
 	{
-	  tree name = ssa_name (i);
 	  if (name
 	      && VN_INFO (name)->visited
 	      && SSA_VAL (name) != name)
diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index fd96c3a..c300650 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -7028,7 +7028,7 @@ static void
 compute_points_to_sets (void)
 {
   basic_block bb;
-  unsigned i;
+  tree ptr;
   varinfo_t vi;
 
   timevar_push (TV_TREE_PTA);
@@ -7077,9 +7077,8 @@ compute_points_to_sets (void)
   cfun->gimple_df->escaped.escaped = 0;
 
   /* Compute the points-to sets for pointer SSA_NAMEs.  */
-  for (i = 0; i < num_ssa_names; ++i)
+  FOR_EACH_SSAVAR (ptr)
     {
-      tree ptr = ssa_name (i);
       if (ptr
 	  && POINTER_TYPE_P (TREE_TYPE (ptr)))
 	find_what_p_points_to (cfun->decl, ptr);
diff --git a/gcc/tree-ssa-ter.c b/gcc/tree-ssa-ter.c
index 2a772b2..50faf81 100644
--- a/gcc/tree-ssa-ter.c
+++ b/gcc/tree-ssa-ter.c
@@ -185,7 +185,7 @@ extern void debug_ter (FILE *, temp_expr_table *);
 static temp_expr_table *
 new_temp_expr_table (var_map map)
 {
-  unsigned x;
+  tree name;
 
   temp_expr_table *t = XNEW (struct temp_expr_table);
   t->map = map;
@@ -201,10 +201,9 @@ new_temp_expr_table (var_map map)
 
   t->replaceable_expressions = NULL;
   t->num_in_part = XCNEWVEC (int, num_var_partitions (map));
-  for (x = 1; x < num_ssa_names; x++)
+  FOR_EACH_SSAVAR (name)
     {
       int p;
-      tree name = ssa_name (x);
       if (!name)
         continue;
       p = var_to_partition (map, name);
diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
index 8e66ce6..8b8c07e 100644
--- a/gcc/tree-ssanames.h
+++ b/gcc/tree-ssanames.h
@@ -62,6 +62,64 @@ struct GTY ((variable_size)) range_info_def {
 #define num_ssa_names (vec_safe_length (cfun->gimple_df->ssa_names))
 #define ssa_name(i) ((*cfun->gimple_df->ssa_names)[(i)])
 
+/* The iterator for ssa names in a function.  */
+class ssa_iterator
+{
+public:
+  ssa_iterator (unsigned p_ssa_names_no,
+		vec<tree_node*, va_gc> *p_ssanames,
+		tree *p_var);
+  ~ssa_iterator () {}
+
+  inline void next ();
+  inline bool get (tree *);
+
+private:
+  /* The index of the ssa visiting.  */
+  unsigned m_idx;
+  /* The number of ssa in the function.  */
+  unsigned m_ssa_names_count;
+  /* Vector containing the ssa names in the function.  */
+  vec<tree_node*, va_gc> *m_ssanames;
+};
+
+inline
+ssa_iterator::ssa_iterator (unsigned p_ssa_names_count,
+			    vec<tree_node*, va_gc> *p_ssanames,
+			    tree *p_var) : m_idx (0),
+  m_ssa_names_count (p_ssa_names_count),
+  m_ssanames (p_ssanames)
+{
+  this->get (p_var);
+}
+
+inline bool
+ssa_iterator::get (tree *p_var)
+{
+  if (this->m_idx < this->m_ssa_names_count)
+    {
+      *p_var = (*m_ssanames)[this->m_idx];
+      return true;
+    }
+  else
+    {
+      *p_var = NULL_TREE;
+      return false;
+    }
+}
+
+inline void
+ssa_iterator::next ()
+{
+  this->m_idx += 1;
+}
+
+#define FOR_EACH_SSAVAR(VAR)					\
+  for (ssa_iterator si (num_ssa_names, cfun->gimple_df->ssa_names,	\
+		       &VAR);						\
+       si.get (&VAR);							\
+       si.next ())
+
 /* Sets the value range to SSA.  */
 extern void set_range_info (tree, enum value_range_type, const wide_int_ref &,
 			    const wide_int_ref &);

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][SSA] Iterator to visit SSA
  2016-09-05  5:39 [RFC][SSA] Iterator to visit SSA Kugan Vivekanandarajah
@ 2016-09-05  7:58 ` Richard Biener
  2016-09-06  6:00   ` Kugan Vivekanandarajah
  2016-09-06 21:58   ` Jeff Law
  0 siblings, 2 replies; 14+ messages in thread
From: Richard Biener @ 2016-09-05  7:58 UTC (permalink / raw)
  To: Kugan Vivekanandarajah; +Cc: gcc-patches

On Mon, Sep 5, 2016 at 7:26 AM, Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi All,
>
> While looking at gcc source, I noticed that we are iterating over SSA
> variable from 0 to num_ssa_names in some cases and 1 to num_ssa_names
> in others. It seems that variable 0 is always NULL TREE.

Yeah, that's artificial because we don't assign SSA name version 0 (for some
unknown reason).

> But it can
> confuse people who are looking for the first time. Therefore It might
> be good to follow some consistent usage here.
>
> It might be also good to gave a FOR_EACH_SSAVAR iterator as we do in
> other case. Here is attempt to do this based on what is done in other
> places. Bootstrapped and regression tested on X86_64-linux-gnu with no
> new regressions. is this OK?

First of all some bikeshedding - FOR_EACH_SSA_NAME would be better
as SSAVAR might be confused with iterating over SSA_NAME_VAR.

Then, if you add an iterator why leave the name == NULL handling to consumers?
That looks odd.

Then, SSA names are just in a vector thus why not re-use the vector iterator?

That is,

#define FOR_EACH_SSA_NAME (name) \
  for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i)

would be equivalent to your patch?

Please also don't add new iterators that implicitely use 'cfun' but always use
one that passes it as explicit arg.

Thanks,
Richard.


> Thanks,
> Kugan
>
>
> gcc/ChangeLog:
>
> 2016-09-05  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
>     * tree-ssanames.h (ssa_iterator::ssa_iterator): New.
>     (ssa_iterator::get): Likewise.
>     (ssa_iterator::next): Likewise.
>     (FOR_EACH_SSAVAR): Likewise.
>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>     FOR_EACH_SSAVAR to iterate over SSA variables.
>     (pass_expand::execute): Likewise.
>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>     * tree-cfg.c (dump_function_to_file): Likewise.
>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>     (update_ssa): Likewise.
>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>     (create_outofssa_var_map): Likewise.
>     (coalesce_ssa_name): Likewise.
>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>     * tree-ssa-pre.c (compute_avail): Likewise.
>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>     (scc_vn_restore_ssa_info): Likewise.
>     (free_scc_vn): Likwise.
>     (run_scc_vn): Likewise.
>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][SSA] Iterator to visit SSA
  2016-09-05  7:58 ` Richard Biener
@ 2016-09-06  6:00   ` Kugan Vivekanandarajah
  2016-09-06  9:08     ` Richard Biener
  2016-09-06 21:58   ` Jeff Law
  1 sibling, 1 reply; 14+ messages in thread
From: Kugan Vivekanandarajah @ 2016-09-06  6:00 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 4407 bytes --]

Hi Richard,

On 5 September 2016 at 17:57, Richard Biener <richard.guenther@gmail.com> wrote:
> On Mon, Sep 5, 2016 at 7:26 AM, Kugan Vivekanandarajah
> <kugan.vivekanandarajah@linaro.org> wrote:
>> Hi All,
>>
>> While looking at gcc source, I noticed that we are iterating over SSA
>> variable from 0 to num_ssa_names in some cases and 1 to num_ssa_names
>> in others. It seems that variable 0 is always NULL TREE.
>
> Yeah, that's artificial because we don't assign SSA name version 0 (for some
> unknown reason).
>
>> But it can
>> confuse people who are looking for the first time. Therefore It might
>> be good to follow some consistent usage here.
>>
>> It might be also good to gave a FOR_EACH_SSAVAR iterator as we do in
>> other case. Here is attempt to do this based on what is done in other
>> places. Bootstrapped and regression tested on X86_64-linux-gnu with no
>> new regressions. is this OK?
>
> First of all some bikeshedding - FOR_EACH_SSA_NAME would be better
> as SSAVAR might be confused with iterating over SSA_NAME_VAR.
>
> Then, if you add an iterator why leave the name == NULL handling to consumers?
> That looks odd.
>
> Then, SSA names are just in a vector thus why not re-use the vector iterator?
>
> That is,
>
> #define FOR_EACH_SSA_NAME (name) \
>   for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i)
>
> would be equivalent to your patch?
>
> Please also don't add new iterators that implicitely use 'cfun' but always use
> one that passes it as explicit arg.

I think defining FOR_EACH_SSA_NAME with vector iterator is better. But
we will not be able to skill NULL ssa_names with that. I also added
index variable to the macro so that there want be any conflicts if the
index variable "i" (or whatever) is also defined in the loop.

Bootstrapped and regression tested on x86_64-linux-gnu with no new
regressions. Is this OK for trunk?

Thanks,
Kugan


gcc/ChangeLog:

2016-09-06  Kugan Vivekanandarajah  <kuganv@linaro.org>

    * tree-ssanames.h (FOR_EACH_SSA_NAME): New.
    * cfgexpand.c (update_alias_info_with_stack_vars): Use
    FOR_EACH_SSA_NAME to iterate over SSA variables.
    (pass_expand::execute): Likewise.
    * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
    * tree-cfg.c (dump_function_to_file): Likewise.
    * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
    (update_ssa): Likewise.
    * tree-ssa-alias.c (dump_alias_info): Likewise.
    * tree-ssa-ccp.c (ccp_finalize): Likewise.
    * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
    (create_outofssa_var_map): Likewise.
    (coalesce_ssa_name): Likewise.
    * tree-ssa-operands.c (dump_immediate_uses): Likewise.
    * tree-ssa-pre.c (compute_avail): Likewise.
    * tree-ssa-sccvn.c (init_scc_vn): Likewise.
    (scc_vn_restore_ssa_info): Likewise.
    (free_scc_vn): Likwise.
    (run_scc_vn): Likewise.
    * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
    * tree-ssa-ter.c (new_temp_expr_table): Likewise.
    * tree-ssa-copy.c (fini_copy_prop): Likewise.
    * tree-ssa.c (verify_ssa): Likewise.

>
> Thanks,
> Richard.
>
>
>> Thanks,
>> Kugan
>>
>>
>> gcc/ChangeLog:
>>
>> 2016-09-05  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>
>>     * tree-ssanames.h (ssa_iterator::ssa_iterator): New.
>>     (ssa_iterator::get): Likewise.
>>     (ssa_iterator::next): Likewise.
>>     (FOR_EACH_SSAVAR): Likewise.
>>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>     FOR_EACH_SSAVAR to iterate over SSA variables.
>>     (pass_expand::execute): Likewise.
>>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>     * tree-cfg.c (dump_function_to_file): Likewise.
>>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>     (update_ssa): Likewise.
>>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>     (create_outofssa_var_map): Likewise.
>>     (coalesce_ssa_name): Likewise.
>>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>     * tree-ssa-pre.c (compute_avail): Likewise.
>>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>     (scc_vn_restore_ssa_info): Likewise.
>>     (free_scc_vn): Likwise.
>>     (run_scc_vn): Likewise.
>>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.

[-- Attachment #2: ssa_iterator.txt --]
[-- Type: text/plain, Size: 13689 bytes --]

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 130a16b..6f94335 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -815,12 +815,12 @@ update_alias_info_with_stack_vars (void)
   if (decls_to_partitions)
     {
       unsigned i;
+      tree name;
       hash_set<bitmap> visited;
       bitmap temp = BITMAP_ALLOC (&stack_var_bitmap_obstack);
 
-      for (i = 1; i < num_ssa_names; i++)
+      FOR_EACH_SSA_NAME (i, name)
 	{
-	  tree name = ssa_name (i);
 	  struct ptr_info_def *pi;
 
 	  if (name
@@ -6163,6 +6163,7 @@ pass_expand::execute (function *fun)
   edge e;
   rtx_insn *var_seq, *var_ret_seq;
   unsigned i;
+  tree name;
 
   timevar_push (TV_OUT_OF_SSA);
   rewrite_out_of_ssa (&SA);
@@ -6270,10 +6271,8 @@ pass_expand::execute (function *fun)
 
   /* Now propagate the RTL assignment of each partition to the
      underlying var of each SSA_NAME.  */
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
-
       if (!name
 	  /* We might have generated new SSA names in
 	     update_alias_info_with_stack_vars.  They will have a NULL
@@ -6288,9 +6287,8 @@ pass_expand::execute (function *fun)
   /* Clean up RTL of variables that straddle across multiple
      partitions, and check that the rtl of any PARM_DECLs that are not
      cleaned up is that of their default defs.  */
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
       int part;
 
       if (!name
diff --git a/gcc/omp-simd-clone.c b/gcc/omp-simd-clone.c
index 467d872..210dea5 100644
--- a/gcc/omp-simd-clone.c
+++ b/gcc/omp-simd-clone.c
@@ -888,6 +888,7 @@ ipa_simd_modify_function_body (struct cgraph_node *node,
 {
   basic_block bb;
   unsigned int i, j, l;
+  tree name;
 
   /* Re-use the adjustments array, but this time use it to replace
      every function argument use to an offset into the corresponding
@@ -911,9 +912,8 @@ ipa_simd_modify_function_body (struct cgraph_node *node,
     }
 
   l = adjustments.length ();
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
       if (name
 	  && SSA_NAME_VAR (name)
 	  && TREE_CODE (SSA_NAME_VAR (name)) == PARM_DECL)
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 57c8410..a786965 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -7539,6 +7539,7 @@ dump_function_to_file (tree fndecl, FILE *file, int flags)
   if (fun && fun->decl == fndecl && (fun->curr_properties & PROP_gimple_lcf))
     {
       unsigned ix;
+      tree name;
       ignore_topmost_bind = true;
 
       fprintf (file, "{\n");
@@ -7582,9 +7583,8 @@ dump_function_to_file (tree fndecl, FILE *file, int flags)
 	    any_var = true;
 	  }
       if (gimple_in_ssa_p (cfun))
-	for (ix = 1; ix < num_ssa_names; ++ix)
+	FOR_EACH_SSA_NAME (ix, name)
 	  {
-	    tree name = ssa_name (ix);
 	    if (name && !SSA_NAME_VAR (name))
 	      {
 		fprintf (file, "  ");
diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
index ceafa68..042934f 100644
--- a/gcc/tree-into-ssa.c
+++ b/gcc/tree-into-ssa.c
@@ -2342,6 +2342,7 @@ pass_build_ssa::execute (function *fun)
   bitmap_head *dfs;
   basic_block bb;
   unsigned i;
+  tree decl, name;
 
   /* Initialize operand data structures.  */
   init_ssa_operands (fun);
@@ -2385,9 +2386,8 @@ pass_build_ssa::execute (function *fun)
   /* Try to get rid of all gimplifier generated temporaries by making
      its SSA names anonymous.  This way we can garbage collect them
      all after removing unused locals which we do in our TODO.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree decl, name = ssa_name (i);
       if (!name
 	  || SSA_NAME_IS_DEFAULT_DEF (name))
 	continue;
@@ -3165,7 +3165,7 @@ update_ssa (unsigned update_flags)
   unsigned i = 0;
   bool insert_phi_p;
   sbitmap_iterator sbi;
-  tree sym;
+  tree sym, name;
 
   /* Only one update flag should be set.  */
   gcc_assert (update_flags == TODO_update_ssa
@@ -3284,9 +3284,8 @@ update_ssa (unsigned update_flags)
       prepare_block_for_update (start_bb, insert_phi_p);
 
       if (flag_checking)
-	for (i = 1; i < num_ssa_names; ++i)
+	FOR_EACH_SSA_NAME (i, name)
 	  {
-	    tree name = ssa_name (i);
 	    if (!name
 		|| virtual_operand_p (name))
 	      continue;
diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 8051a66..d3bf719 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -450,6 +450,7 @@ void
 dump_alias_info (FILE *file)
 {
   unsigned i;
+  tree ptr;
   const char *funcname
     = lang_hooks.decl_printable_name (current_function_decl, 2);
   tree var;
@@ -471,9 +472,8 @@ dump_alias_info (FILE *file)
 
   fprintf (file, "\n\nFlow-insensitive points-to information\n\n");
 
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, ptr)
     {
-      tree ptr = ssa_name (i);
       struct ptr_info_def *pi;
 
       if (ptr == NULL_TREE
diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c
index 9871304..702fbfb 100644
--- a/gcc/tree-ssa-ccp.c
+++ b/gcc/tree-ssa-ccp.c
@@ -898,15 +898,15 @@ ccp_finalize (bool nonzero_p)
 {
   bool something_changed;
   unsigned i;
+  tree name;
 
   do_dbg_cnt ();
 
   /* Derive alignment and misalignment information from partially
      constant pointers in the lattice or nonzero bits from partially
      constant integers.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
       ccp_prop_value_t *val;
       unsigned int tem, align;
 
diff --git a/gcc/tree-ssa-coalesce.c b/gcc/tree-ssa-coalesce.c
index 34c3fa1..90bd7c6 100644
--- a/gcc/tree-ssa-coalesce.c
+++ b/gcc/tree-ssa-coalesce.c
@@ -955,10 +955,10 @@ build_ssa_conflict_graph (tree_live_info_p liveinfo)
       if (bb == entry)
 	{
 	  unsigned i;
-	  for (i = 1; i < num_ssa_names; i++)
-	    {
-	      tree var = ssa_name (i);
+	  tree var;
 
+	  FOR_EACH_SSA_NAME (i, var)
+	    {
 	      if (!var
 		  || !SSA_NAME_IS_DEFAULT_DEF (var)
 		  || !SSA_NAME_VAR (var)
@@ -1261,9 +1261,8 @@ create_outofssa_var_map (coalesce_list *cl, bitmap used_in_copy)
   /* Now process result decls and live on entry variables for entry into
      the coalesce list.  */
   first = NULL_TREE;
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, var)
     {
-      var = ssa_name (i);
       if (var != NULL_TREE && !virtual_operand_p (var))
         {
 	  coalesce_with_default (var, cl, used_in_copy);
@@ -1806,6 +1805,7 @@ coalesce_ssa_name (void)
   bitmap used_in_copies = BITMAP_ALLOC (NULL);
   var_map map;
   unsigned int i;
+  tree a;
 
   cl = create_coalesce_list ();
   map = create_outofssa_var_map (cl, used_in_copies);
@@ -1817,10 +1817,8 @@ coalesce_ssa_name (void)
     {
       hash_table<ssa_name_var_hash> ssa_name_hash (10);
 
-      for (i = 1; i < num_ssa_names; i++)
+      FOR_EACH_SSA_NAME (i, a)
 	{
-	  tree a = ssa_name (i);
-
 	  if (a
 	      && SSA_NAME_VAR (a)
 	      && !DECL_IGNORED_P (SSA_NAME_VAR (a))
diff --git a/gcc/tree-ssa-copy.c b/gcc/tree-ssa-copy.c
index 3d17926..01c09c8 100644
--- a/gcc/tree-ssa-copy.c
+++ b/gcc/tree-ssa-copy.c
@@ -503,12 +503,12 @@ static bool
 fini_copy_prop (void)
 {
   unsigned i;
+  tree var;
 
   /* Set the final copy-of value for each variable by traversing the
      copy-of chains.  */
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, var)
     {
-      tree var = ssa_name (i);
       if (!var
 	  || !copy_of[i].value
 	  || copy_of[i].value == var)
diff --git a/gcc/tree-ssa-operands.c b/gcc/tree-ssa-operands.c
index eccea2f..7a22dea 100644
--- a/gcc/tree-ssa-operands.c
+++ b/gcc/tree-ssa-operands.c
@@ -1247,9 +1247,8 @@ dump_immediate_uses (FILE *file)
   unsigned int x;
 
   fprintf (file, "Immediate_uses: \n\n");
-  for (x = 1; x < num_ssa_names; x++)
+  FOR_EACH_SSA_NAME (x, var)
     {
-      var = ssa_name (x);
       if (!var)
         continue;
       dump_immediate_uses_for (file, var);
diff --git a/gcc/tree-ssa-pre.c b/gcc/tree-ssa-pre.c
index fdb1c2c..47f7908 100644
--- a/gcc/tree-ssa-pre.c
+++ b/gcc/tree-ssa-pre.c
@@ -3670,12 +3670,12 @@ compute_avail (void)
   basic_block *worklist;
   size_t sp = 0;
   unsigned i;
+  tree name;
 
   /* We pretend that default definitions are defined in the entry block.
      This includes function arguments and the static chain decl.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
       pre_expr e;
       if (!name
 	  || !SSA_NAME_IS_DEFAULT_DEF (name)
diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c
index 21b3d56..0323d7f 100644
--- a/gcc/tree-ssa-sccvn.c
+++ b/gcc/tree-ssa-sccvn.c
@@ -4291,6 +4291,7 @@ static void
 init_scc_vn (void)
 {
   size_t i;
+  tree name;
   int j;
   int *rpo_numbers_temp;
 
@@ -4339,9 +4340,8 @@ init_scc_vn (void)
 
   /* Create the VN_INFO structures, and initialize value numbers to
      TOP or VARYING for parameters.  */
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
       if (!name)
 	continue;
 
@@ -4402,9 +4402,11 @@ init_scc_vn (void)
 void
 scc_vn_restore_ssa_info (void)
 {
-  for (unsigned i = 0; i < num_ssa_names; i++)
+  unsigned i;
+  tree name;
+
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
       if (name
 	  && has_VN_INFO (name))
 	{
@@ -4428,6 +4430,7 @@ void
 free_scc_vn (void)
 {
   size_t i;
+  tree name;
 
   delete constant_to_value_id;
   constant_to_value_id = NULL;
@@ -4436,9 +4439,8 @@ free_scc_vn (void)
   shared_lookup_references.release ();
   XDELETEVEC (rpo_numbers);
 
-  for (i = 0; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
       if (name
 	  && has_VN_INFO (name)
 	  && VN_INFO (name)->needs_insertion)
@@ -4757,6 +4759,7 @@ bool
 run_scc_vn (vn_lookup_kind default_vn_walk_kind_)
 {
   size_t i;
+  tree name;
 
   default_vn_walk_kind = default_vn_walk_kind_;
 
@@ -4797,9 +4800,8 @@ run_scc_vn (vn_lookup_kind default_vn_walk_kind_)
 
   /* Initialize the value ids and prune out remaining VN_TOPs
      from dead code.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
       vn_ssa_aux_t info;
       if (!name)
 	continue;
@@ -4814,9 +4816,8 @@ run_scc_vn (vn_lookup_kind default_vn_walk_kind_)
     }
 
   /* Propagate.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
       vn_ssa_aux_t info;
       if (!name)
 	continue;
@@ -4832,9 +4833,8 @@ run_scc_vn (vn_lookup_kind default_vn_walk_kind_)
   if (dump_file && (dump_flags & TDF_DETAILS))
     {
       fprintf (dump_file, "Value numbers:\n");
-      for (i = 0; i < num_ssa_names; i++)
+      FOR_EACH_SSA_NAME (i, name)
 	{
-	  tree name = ssa_name (i);
 	  if (name
 	      && VN_INFO (name)->visited
 	      && SSA_VAL (name) != name)
diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index fd96c3a..3b13387 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -7029,6 +7029,7 @@ compute_points_to_sets (void)
 {
   basic_block bb;
   unsigned i;
+  tree ptr;
   varinfo_t vi;
 
   timevar_push (TV_TREE_PTA);
@@ -7077,9 +7078,8 @@ compute_points_to_sets (void)
   cfun->gimple_df->escaped.escaped = 0;
 
   /* Compute the points-to sets for pointer SSA_NAMEs.  */
-  for (i = 0; i < num_ssa_names; ++i)
+  FOR_EACH_SSA_NAME (i, ptr)
     {
-      tree ptr = ssa_name (i);
       if (ptr
 	  && POINTER_TYPE_P (TREE_TYPE (ptr)))
 	find_what_p_points_to (cfun->decl, ptr);
diff --git a/gcc/tree-ssa-ter.c b/gcc/tree-ssa-ter.c
index 2a772b2..0efa407 100644
--- a/gcc/tree-ssa-ter.c
+++ b/gcc/tree-ssa-ter.c
@@ -186,6 +186,7 @@ static temp_expr_table *
 new_temp_expr_table (var_map map)
 {
   unsigned x;
+  tree name;
 
   temp_expr_table *t = XNEW (struct temp_expr_table);
   t->map = map;
@@ -201,10 +202,9 @@ new_temp_expr_table (var_map map)
 
   t->replaceable_expressions = NULL;
   t->num_in_part = XCNEWVEC (int, num_var_partitions (map));
-  for (x = 1; x < num_ssa_names; x++)
+  FOR_EACH_SSA_NAME (x, name)
     {
       int p;
-      tree name = ssa_name (x);
       if (!name)
         continue;
       p = var_to_partition (map, name);
diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c
index aae383d..3389e2e 100644
--- a/gcc/tree-ssa.c
+++ b/gcc/tree-ssa.c
@@ -1010,6 +1010,7 @@ verify_ssa (bool check_modified_stmt, bool check_ssa_operands)
   basic_block *definition_block = XCNEWVEC (basic_block, num_ssa_names);
   ssa_op_iter iter;
   tree op;
+  tree name;
   enum dom_state orig_dom_state = dom_info_state (CDI_DOMINATORS);
   bitmap names_defined_in_bb = BITMAP_ALLOC (NULL);
 
@@ -1018,9 +1019,8 @@ verify_ssa (bool check_modified_stmt, bool check_ssa_operands)
   timevar_push (TV_TREE_SSA_VERIFY);
 
   /* Keep track of SSA names present in the IL.  */
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
       if (name)
 	{
 	  gimple *stmt;
diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
index 8e66ce6..8e9ce00 100644
--- a/gcc/tree-ssanames.h
+++ b/gcc/tree-ssanames.h
@@ -62,6 +62,9 @@ struct GTY ((variable_size)) range_info_def {
 #define num_ssa_names (vec_safe_length (cfun->gimple_df->ssa_names))
 #define ssa_name(i) ((*cfun->gimple_df->ssa_names)[(i)])
 
+#define FOR_EACH_SSA_NAME(i, VAR) \
+  for (i = 1; SSANAMES (cfun)->iterate (i, &VAR); ++i)
+
 /* Sets the value range to SSA.  */
 extern void set_range_info (tree, enum value_range_type, const wide_int_ref &,
 			    const wide_int_ref &);

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][SSA] Iterator to visit SSA
  2016-09-06  6:00   ` Kugan Vivekanandarajah
@ 2016-09-06  9:08     ` Richard Biener
  2016-09-06  9:58       ` Kugan Vivekanandarajah
  2016-09-07  4:46       ` Kugan Vivekanandarajah
  0 siblings, 2 replies; 14+ messages in thread
From: Richard Biener @ 2016-09-06  9:08 UTC (permalink / raw)
  To: Kugan Vivekanandarajah; +Cc: gcc-patches

On Tue, Sep 6, 2016 at 2:24 AM, Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi Richard,
>
> On 5 September 2016 at 17:57, Richard Biener <richard.guenther@gmail.com> wrote:
>> On Mon, Sep 5, 2016 at 7:26 AM, Kugan Vivekanandarajah
>> <kugan.vivekanandarajah@linaro.org> wrote:
>>> Hi All,
>>>
>>> While looking at gcc source, I noticed that we are iterating over SSA
>>> variable from 0 to num_ssa_names in some cases and 1 to num_ssa_names
>>> in others. It seems that variable 0 is always NULL TREE.
>>
>> Yeah, that's artificial because we don't assign SSA name version 0 (for some
>> unknown reason).
>>
>>> But it can
>>> confuse people who are looking for the first time. Therefore It might
>>> be good to follow some consistent usage here.
>>>
>>> It might be also good to gave a FOR_EACH_SSAVAR iterator as we do in
>>> other case. Here is attempt to do this based on what is done in other
>>> places. Bootstrapped and regression tested on X86_64-linux-gnu with no
>>> new regressions. is this OK?
>>
>> First of all some bikeshedding - FOR_EACH_SSA_NAME would be better
>> as SSAVAR might be confused with iterating over SSA_NAME_VAR.
>>
>> Then, if you add an iterator why leave the name == NULL handling to consumers?
>> That looks odd.
>>
>> Then, SSA names are just in a vector thus why not re-use the vector iterator?
>>
>> That is,
>>
>> #define FOR_EACH_SSA_NAME (name) \
>>   for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i)
>>
>> would be equivalent to your patch?
>>
>> Please also don't add new iterators that implicitely use 'cfun' but always use
>> one that passes it as explicit arg.
>
> I think defining FOR_EACH_SSA_NAME with vector iterator is better. But
> we will not be able to skill NULL ssa_names with that.

Why?  Can't you simply do

  #define FOR_EACH_SSA_NAME (name) \
    for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i) \
       if (name)

?

> I also added
> index variable to the macro so that there want be any conflicts if the
> index variable "i" (or whatever) is also defined in the loop.
>
> Bootstrapped and regression tested on x86_64-linux-gnu with no new
> regressions. Is this OK for trunk?
>
> Thanks,
> Kugan
>
>
> gcc/ChangeLog:
>
> 2016-09-06  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
>     * tree-ssanames.h (FOR_EACH_SSA_NAME): New.
>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>     FOR_EACH_SSA_NAME to iterate over SSA variables.
>     (pass_expand::execute): Likewise.
>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>     * tree-cfg.c (dump_function_to_file): Likewise.
>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>     (update_ssa): Likewise.
>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>     (create_outofssa_var_map): Likewise.
>     (coalesce_ssa_name): Likewise.
>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>     * tree-ssa-pre.c (compute_avail): Likewise.
>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>     (scc_vn_restore_ssa_info): Likewise.
>     (free_scc_vn): Likwise.
>     (run_scc_vn): Likewise.
>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.
>     * tree-ssa-copy.c (fini_copy_prop): Likewise.
>     * tree-ssa.c (verify_ssa): Likewise.
>
>>
>> Thanks,
>> Richard.
>>
>>
>>> Thanks,
>>> Kugan
>>>
>>>
>>> gcc/ChangeLog:
>>>
>>> 2016-09-05  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>>
>>>     * tree-ssanames.h (ssa_iterator::ssa_iterator): New.
>>>     (ssa_iterator::get): Likewise.
>>>     (ssa_iterator::next): Likewise.
>>>     (FOR_EACH_SSAVAR): Likewise.
>>>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>>     FOR_EACH_SSAVAR to iterate over SSA variables.
>>>     (pass_expand::execute): Likewise.
>>>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>>     * tree-cfg.c (dump_function_to_file): Likewise.
>>>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>>     (update_ssa): Likewise.
>>>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>>>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>>     (create_outofssa_var_map): Likewise.
>>>     (coalesce_ssa_name): Likewise.
>>>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>>     * tree-ssa-pre.c (compute_avail): Likewise.
>>>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>>     (scc_vn_restore_ssa_info): Likewise.
>>>     (free_scc_vn): Likwise.
>>>     (run_scc_vn): Likewise.
>>>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][SSA] Iterator to visit SSA
  2016-09-06  9:08     ` Richard Biener
@ 2016-09-06  9:58       ` Kugan Vivekanandarajah
  2016-09-06 10:07         ` Richard Biener
  2016-09-07  4:46       ` Kugan Vivekanandarajah
  1 sibling, 1 reply; 14+ messages in thread
From: Kugan Vivekanandarajah @ 2016-09-06  9:58 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 6001 bytes --]

Hi Richard,

On 6 September 2016 at 19:08, Richard Biener <richard.guenther@gmail.com> wrote:
> On Tue, Sep 6, 2016 at 2:24 AM, Kugan Vivekanandarajah
> <kugan.vivekanandarajah@linaro.org> wrote:
>> Hi Richard,
>>
>> On 5 September 2016 at 17:57, Richard Biener <richard.guenther@gmail.com> wrote:
>>> On Mon, Sep 5, 2016 at 7:26 AM, Kugan Vivekanandarajah
>>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>> Hi All,
>>>>
>>>> While looking at gcc source, I noticed that we are iterating over SSA
>>>> variable from 0 to num_ssa_names in some cases and 1 to num_ssa_names
>>>> in others. It seems that variable 0 is always NULL TREE.
>>>
>>> Yeah, that's artificial because we don't assign SSA name version 0 (for some
>>> unknown reason).
>>>
>>>> But it can
>>>> confuse people who are looking for the first time. Therefore It might
>>>> be good to follow some consistent usage here.
>>>>
>>>> It might be also good to gave a FOR_EACH_SSAVAR iterator as we do in
>>>> other case. Here is attempt to do this based on what is done in other
>>>> places. Bootstrapped and regression tested on X86_64-linux-gnu with no
>>>> new regressions. is this OK?
>>>
>>> First of all some bikeshedding - FOR_EACH_SSA_NAME would be better
>>> as SSAVAR might be confused with iterating over SSA_NAME_VAR.
>>>
>>> Then, if you add an iterator why leave the name == NULL handling to consumers?
>>> That looks odd.
>>>
>>> Then, SSA names are just in a vector thus why not re-use the vector iterator?
>>>
>>> That is,
>>>
>>> #define FOR_EACH_SSA_NAME (name) \
>>>   for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i)
>>>
>>> would be equivalent to your patch?
>>>
>>> Please also don't add new iterators that implicitely use 'cfun' but always use
>>> one that passes it as explicit arg.
>>
>> I think defining FOR_EACH_SSA_NAME with vector iterator is better. But
>> we will not be able to skill NULL ssa_names with that.
>
> Why?  Can't you simply do
>
>   #define FOR_EACH_SSA_NAME (name) \
>     for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i) \
>        if (name)
>
> ?

For example, with the test patch where loop body also defines i which
is giving error:

../../trunk/gcc/tree-ssa-ter.c: In function ‘temp_expr_table*
new_temp_expr_table(var_map)’:
../../trunk/gcc/tree-ssa-ter.c:206:11: error: redeclaration of ‘int i’
       int i;
           ^
In file included from ../../trunk/gcc/ssa.h:29:0,
                 from ../../trunk/gcc/tree-ssa-ter.c:28:
../../trunk/gcc/tree-ssanames.h:66:17: note: ‘unsigned int i’
previously declared here
   for (unsigned i = 1; SSANAMES (cfun)->iterate (i, &VAR); ++i)
                 ^
../../trunk/gcc/tree-ssa-ter.c:204:3: note: in expansion of macro
‘FOR_EACH_SSA_NAME’
   FOR_EACH_SSA_NAME (name)
   ^
Makefile:1097: recipe for target 'tree-ssa-ter.o' failed

Am I missing something here ?

Thanks,
Kugan

>
>> I also added
>> index variable to the macro so that there want be any conflicts if the
>> index variable "i" (or whatever) is also defined in the loop.
>>
>> Bootstrapped and regression tested on x86_64-linux-gnu with no new
>> regressions. Is this OK for trunk?
>>
>> Thanks,
>> Kugan
>>
>>
>> gcc/ChangeLog:
>>
>> 2016-09-06  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>
>>     * tree-ssanames.h (FOR_EACH_SSA_NAME): New.
>>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>     FOR_EACH_SSA_NAME to iterate over SSA variables.
>>     (pass_expand::execute): Likewise.
>>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>     * tree-cfg.c (dump_function_to_file): Likewise.
>>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>     (update_ssa): Likewise.
>>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>     (create_outofssa_var_map): Likewise.
>>     (coalesce_ssa_name): Likewise.
>>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>     * tree-ssa-pre.c (compute_avail): Likewise.
>>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>     (scc_vn_restore_ssa_info): Likewise.
>>     (free_scc_vn): Likwise.
>>     (run_scc_vn): Likewise.
>>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.
>>     * tree-ssa-copy.c (fini_copy_prop): Likewise.
>>     * tree-ssa.c (verify_ssa): Likewise.
>>
>>>
>>> Thanks,
>>> Richard.
>>>
>>>
>>>> Thanks,
>>>> Kugan
>>>>
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2016-09-05  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>>>
>>>>     * tree-ssanames.h (ssa_iterator::ssa_iterator): New.
>>>>     (ssa_iterator::get): Likewise.
>>>>     (ssa_iterator::next): Likewise.
>>>>     (FOR_EACH_SSAVAR): Likewise.
>>>>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>>>     FOR_EACH_SSAVAR to iterate over SSA variables.
>>>>     (pass_expand::execute): Likewise.
>>>>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>>>     * tree-cfg.c (dump_function_to_file): Likewise.
>>>>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>>>     (update_ssa): Likewise.
>>>>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>>>>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>>>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>>>     (create_outofssa_var_map): Likewise.
>>>>     (coalesce_ssa_name): Likewise.
>>>>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>>>     * tree-ssa-pre.c (compute_avail): Likewise.
>>>>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>>>     (scc_vn_restore_ssa_info): Likewise.
>>>>     (free_scc_vn): Likwise.
>>>>     (run_scc_vn): Likewise.
>>>>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>>>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.

[-- Attachment #2: t.txt --]
[-- Type: text/plain, Size: 1544 bytes --]

diff --git a/gcc/tree-ssa-ter.c b/gcc/tree-ssa-ter.c
index 2a772b2..15f30ee 100644
--- a/gcc/tree-ssa-ter.c
+++ b/gcc/tree-ssa-ter.c
@@ -185,7 +185,7 @@ extern void debug_ter (FILE *, temp_expr_table *);
 static temp_expr_table *
 new_temp_expr_table (var_map map)
 {
-  unsigned x;
+  tree name;
 
   temp_expr_table *t = XNEW (struct temp_expr_table);
   t->map = map;
@@ -201,15 +201,14 @@ new_temp_expr_table (var_map map)
 
   t->replaceable_expressions = NULL;
   t->num_in_part = XCNEWVEC (int, num_var_partitions (map));
-  for (x = 1; x < num_ssa_names; x++)
+  FOR_EACH_SSA_NAME (name)
     {
-      int p;
-      tree name = ssa_name (x);
+      int i;
       if (!name)
         continue;
-      p = var_to_partition (map, name);
-      if (p != NO_PARTITION)
-        t->num_in_part[p]++;
+      i = var_to_partition (map, name);
+      if (i != NO_PARTITION)
+        t->num_in_part[i]++;
     }
   t->call_cnt = XCNEWVEC (int, num_ssa_names + 1);
 
diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
index 8e66ce6..538e35f 100644
--- a/gcc/tree-ssanames.h
+++ b/gcc/tree-ssanames.h
@@ -62,6 +62,9 @@ struct GTY ((variable_size)) range_info_def {
 #define num_ssa_names (vec_safe_length (cfun->gimple_df->ssa_names))
 #define ssa_name(i) ((*cfun->gimple_df->ssa_names)[(i)])
 
+#define FOR_EACH_SSA_NAME(VAR) \
+  for (unsigned i = 1; SSANAMES (cfun)->iterate (i, &VAR); ++i)
+
 /* Sets the value range to SSA.  */
 extern void set_range_info (tree, enum value_range_type, const wide_int_ref &,
 			    const wide_int_ref &);

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][SSA] Iterator to visit SSA
  2016-09-06 10:07         ` Richard Biener
@ 2016-09-06 10:07           ` Kugan Vivekanandarajah
  2016-09-06 10:41             ` Richard Biener
  0 siblings, 1 reply; 14+ messages in thread
From: Kugan Vivekanandarajah @ 2016-09-06 10:07 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

Hi Richard,


On 6 September 2016 at 19:57, Richard Biener <richard.guenther@gmail.com> wrote:
> On Tue, Sep 6, 2016 at 11:33 AM, Kugan Vivekanandarajah
> <kugan.vivekanandarajah@linaro.org> wrote:
>> Hi Richard,
>>
>> On 6 September 2016 at 19:08, Richard Biener <richard.guenther@gmail.com> wrote:
>>> On Tue, Sep 6, 2016 at 2:24 AM, Kugan Vivekanandarajah
>>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>> Hi Richard,
>>>>
>>>> On 5 September 2016 at 17:57, Richard Biener <richard.guenther@gmail.com> wrote:
>>>>> On Mon, Sep 5, 2016 at 7:26 AM, Kugan Vivekanandarajah
>>>>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>>>> Hi All,
>>>>>>
>>>>>> While looking at gcc source, I noticed that we are iterating over SSA
>>>>>> variable from 0 to num_ssa_names in some cases and 1 to num_ssa_names
>>>>>> in others. It seems that variable 0 is always NULL TREE.
>>>>>
>>>>> Yeah, that's artificial because we don't assign SSA name version 0 (for some
>>>>> unknown reason).
>>>>>
>>>>>> But it can
>>>>>> confuse people who are looking for the first time. Therefore It might
>>>>>> be good to follow some consistent usage here.
>>>>>>
>>>>>> It might be also good to gave a FOR_EACH_SSAVAR iterator as we do in
>>>>>> other case. Here is attempt to do this based on what is done in other
>>>>>> places. Bootstrapped and regression tested on X86_64-linux-gnu with no
>>>>>> new regressions. is this OK?
>>>>>
>>>>> First of all some bikeshedding - FOR_EACH_SSA_NAME would be better
>>>>> as SSAVAR might be confused with iterating over SSA_NAME_VAR.
>>>>>
>>>>> Then, if you add an iterator why leave the name == NULL handling to consumers?
>>>>> That looks odd.
>>>>>
>>>>> Then, SSA names are just in a vector thus why not re-use the vector iterator?
>>>>>
>>>>> That is,
>>>>>
>>>>> #define FOR_EACH_SSA_NAME (name) \
>>>>>   for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i)
>>>>>
>>>>> would be equivalent to your patch?
>>>>>
>>>>> Please also don't add new iterators that implicitely use 'cfun' but always use
>>>>> one that passes it as explicit arg.
>>>>
>>>> I think defining FOR_EACH_SSA_NAME with vector iterator is better. But
>>>> we will not be able to skill NULL ssa_names with that.
>>>
>>> Why?  Can't you simply do
>>>
>>>   #define FOR_EACH_SSA_NAME (name) \
>>>     for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i) \
>>>        if (name)
>>>
>>> ?
>>
>> For example, with the test patch where loop body also defines i which
>> is giving error:
>>
>> ../../trunk/gcc/tree-ssa-ter.c: In function ‘temp_expr_table*
>> new_temp_expr_table(var_map)’:
>> ../../trunk/gcc/tree-ssa-ter.c:206:11: error: redeclaration of ‘int i’
>>        int i;
>>            ^
>> In file included from ../../trunk/gcc/ssa.h:29:0,
>>                  from ../../trunk/gcc/tree-ssa-ter.c:28:
>> ../../trunk/gcc/tree-ssanames.h:66:17: note: ‘unsigned int i’
>> previously declared here
>>    for (unsigned i = 1; SSANAMES (cfun)->iterate (i, &VAR); ++i)
>>                  ^
>> ../../trunk/gcc/tree-ssa-ter.c:204:3: note: in expansion of macro
>> ‘FOR_EACH_SSA_NAME’
>>    FOR_EACH_SSA_NAME (name)
>>    ^
>> Makefile:1097: recipe for target 'tree-ssa-ter.o' failed
>>
>> Am I missing something here ?
>
> Well, my comment was not about passing 'i' to the macro but about
> not being able to skip NULL names.  I just copied & pasted my
> earlier macro.

Sorry, I misunderstood your comment.

I have:

+#define FOR_EACH_SSA_NAME(i, VAR) \
+  for (i = 1; SSANAMES (cfun)->iterate (i, &VAR); ++i)

We will skip the first (0 index) if that is what you are referring here.

I also thought that you wanted to skip any NULL_TREEs after that too.
After looking at your previous your comment, I don’t you think you
wanted that anyway.

Therefore I think I am doing what you wanted in my last patch.

Thanks,
Kugan


> Richard.
>
>> Thanks,
>> Kugan
>>
>>>
>>>> I also added
>>>> index variable to the macro so that there want be any conflicts if the
>>>> index variable "i" (or whatever) is also defined in the loop.
>>>>
>>>> Bootstrapped and regression tested on x86_64-linux-gnu with no new
>>>> regressions. Is this OK for trunk?
>>>>
>>>> Thanks,
>>>> Kugan
>>>>
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2016-09-06  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>>>
>>>>     * tree-ssanames.h (FOR_EACH_SSA_NAME): New.
>>>>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>>>     FOR_EACH_SSA_NAME to iterate over SSA variables.
>>>>     (pass_expand::execute): Likewise.
>>>>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>>>     * tree-cfg.c (dump_function_to_file): Likewise.
>>>>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>>>     (update_ssa): Likewise.
>>>>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>>>>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>>>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>>>     (create_outofssa_var_map): Likewise.
>>>>     (coalesce_ssa_name): Likewise.
>>>>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>>>     * tree-ssa-pre.c (compute_avail): Likewise.
>>>>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>>>     (scc_vn_restore_ssa_info): Likewise.
>>>>     (free_scc_vn): Likwise.
>>>>     (run_scc_vn): Likewise.
>>>>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>>>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.
>>>>     * tree-ssa-copy.c (fini_copy_prop): Likewise.
>>>>     * tree-ssa.c (verify_ssa): Likewise.
>>>>
>>>>>
>>>>> Thanks,
>>>>> Richard.
>>>>>
>>>>>
>>>>>> Thanks,
>>>>>> Kugan
>>>>>>
>>>>>>
>>>>>> gcc/ChangeLog:
>>>>>>
>>>>>> 2016-09-05  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>>>>>
>>>>>>     * tree-ssanames.h (ssa_iterator::ssa_iterator): New.
>>>>>>     (ssa_iterator::get): Likewise.
>>>>>>     (ssa_iterator::next): Likewise.
>>>>>>     (FOR_EACH_SSAVAR): Likewise.
>>>>>>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>>>>>     FOR_EACH_SSAVAR to iterate over SSA variables.
>>>>>>     (pass_expand::execute): Likewise.
>>>>>>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>>>>>     * tree-cfg.c (dump_function_to_file): Likewise.
>>>>>>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>>>>>     (update_ssa): Likewise.
>>>>>>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>>>>>>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>>>>>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>>>>>     (create_outofssa_var_map): Likewise.
>>>>>>     (coalesce_ssa_name): Likewise.
>>>>>>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>>>>>     * tree-ssa-pre.c (compute_avail): Likewise.
>>>>>>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>>>>>     (scc_vn_restore_ssa_info): Likewise.
>>>>>>     (free_scc_vn): Likwise.
>>>>>>     (run_scc_vn): Likewise.
>>>>>>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>>>>>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][SSA] Iterator to visit SSA
  2016-09-06  9:58       ` Kugan Vivekanandarajah
@ 2016-09-06 10:07         ` Richard Biener
  2016-09-06 10:07           ` Kugan Vivekanandarajah
  0 siblings, 1 reply; 14+ messages in thread
From: Richard Biener @ 2016-09-06 10:07 UTC (permalink / raw)
  To: Kugan Vivekanandarajah; +Cc: gcc-patches

On Tue, Sep 6, 2016 at 11:33 AM, Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi Richard,
>
> On 6 September 2016 at 19:08, Richard Biener <richard.guenther@gmail.com> wrote:
>> On Tue, Sep 6, 2016 at 2:24 AM, Kugan Vivekanandarajah
>> <kugan.vivekanandarajah@linaro.org> wrote:
>>> Hi Richard,
>>>
>>> On 5 September 2016 at 17:57, Richard Biener <richard.guenther@gmail.com> wrote:
>>>> On Mon, Sep 5, 2016 at 7:26 AM, Kugan Vivekanandarajah
>>>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>>> Hi All,
>>>>>
>>>>> While looking at gcc source, I noticed that we are iterating over SSA
>>>>> variable from 0 to num_ssa_names in some cases and 1 to num_ssa_names
>>>>> in others. It seems that variable 0 is always NULL TREE.
>>>>
>>>> Yeah, that's artificial because we don't assign SSA name version 0 (for some
>>>> unknown reason).
>>>>
>>>>> But it can
>>>>> confuse people who are looking for the first time. Therefore It might
>>>>> be good to follow some consistent usage here.
>>>>>
>>>>> It might be also good to gave a FOR_EACH_SSAVAR iterator as we do in
>>>>> other case. Here is attempt to do this based on what is done in other
>>>>> places. Bootstrapped and regression tested on X86_64-linux-gnu with no
>>>>> new regressions. is this OK?
>>>>
>>>> First of all some bikeshedding - FOR_EACH_SSA_NAME would be better
>>>> as SSAVAR might be confused with iterating over SSA_NAME_VAR.
>>>>
>>>> Then, if you add an iterator why leave the name == NULL handling to consumers?
>>>> That looks odd.
>>>>
>>>> Then, SSA names are just in a vector thus why not re-use the vector iterator?
>>>>
>>>> That is,
>>>>
>>>> #define FOR_EACH_SSA_NAME (name) \
>>>>   for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i)
>>>>
>>>> would be equivalent to your patch?
>>>>
>>>> Please also don't add new iterators that implicitely use 'cfun' but always use
>>>> one that passes it as explicit arg.
>>>
>>> I think defining FOR_EACH_SSA_NAME with vector iterator is better. But
>>> we will not be able to skill NULL ssa_names with that.
>>
>> Why?  Can't you simply do
>>
>>   #define FOR_EACH_SSA_NAME (name) \
>>     for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i) \
>>        if (name)
>>
>> ?
>
> For example, with the test patch where loop body also defines i which
> is giving error:
>
> ../../trunk/gcc/tree-ssa-ter.c: In function ‘temp_expr_table*
> new_temp_expr_table(var_map)’:
> ../../trunk/gcc/tree-ssa-ter.c:206:11: error: redeclaration of ‘int i’
>        int i;
>            ^
> In file included from ../../trunk/gcc/ssa.h:29:0,
>                  from ../../trunk/gcc/tree-ssa-ter.c:28:
> ../../trunk/gcc/tree-ssanames.h:66:17: note: ‘unsigned int i’
> previously declared here
>    for (unsigned i = 1; SSANAMES (cfun)->iterate (i, &VAR); ++i)
>                  ^
> ../../trunk/gcc/tree-ssa-ter.c:204:3: note: in expansion of macro
> ‘FOR_EACH_SSA_NAME’
>    FOR_EACH_SSA_NAME (name)
>    ^
> Makefile:1097: recipe for target 'tree-ssa-ter.o' failed
>
> Am I missing something here ?

Well, my comment was not about passing 'i' to the macro but about
not being able to skip NULL names.  I just copied & pasted my
earlier macro.

Richard.

> Thanks,
> Kugan
>
>>
>>> I also added
>>> index variable to the macro so that there want be any conflicts if the
>>> index variable "i" (or whatever) is also defined in the loop.
>>>
>>> Bootstrapped and regression tested on x86_64-linux-gnu with no new
>>> regressions. Is this OK for trunk?
>>>
>>> Thanks,
>>> Kugan
>>>
>>>
>>> gcc/ChangeLog:
>>>
>>> 2016-09-06  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>>
>>>     * tree-ssanames.h (FOR_EACH_SSA_NAME): New.
>>>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>>     FOR_EACH_SSA_NAME to iterate over SSA variables.
>>>     (pass_expand::execute): Likewise.
>>>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>>     * tree-cfg.c (dump_function_to_file): Likewise.
>>>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>>     (update_ssa): Likewise.
>>>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>>>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>>     (create_outofssa_var_map): Likewise.
>>>     (coalesce_ssa_name): Likewise.
>>>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>>     * tree-ssa-pre.c (compute_avail): Likewise.
>>>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>>     (scc_vn_restore_ssa_info): Likewise.
>>>     (free_scc_vn): Likwise.
>>>     (run_scc_vn): Likewise.
>>>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.
>>>     * tree-ssa-copy.c (fini_copy_prop): Likewise.
>>>     * tree-ssa.c (verify_ssa): Likewise.
>>>
>>>>
>>>> Thanks,
>>>> Richard.
>>>>
>>>>
>>>>> Thanks,
>>>>> Kugan
>>>>>
>>>>>
>>>>> gcc/ChangeLog:
>>>>>
>>>>> 2016-09-05  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>>>>
>>>>>     * tree-ssanames.h (ssa_iterator::ssa_iterator): New.
>>>>>     (ssa_iterator::get): Likewise.
>>>>>     (ssa_iterator::next): Likewise.
>>>>>     (FOR_EACH_SSAVAR): Likewise.
>>>>>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>>>>     FOR_EACH_SSAVAR to iterate over SSA variables.
>>>>>     (pass_expand::execute): Likewise.
>>>>>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>>>>     * tree-cfg.c (dump_function_to_file): Likewise.
>>>>>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>>>>     (update_ssa): Likewise.
>>>>>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>>>>>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>>>>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>>>>     (create_outofssa_var_map): Likewise.
>>>>>     (coalesce_ssa_name): Likewise.
>>>>>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>>>>     * tree-ssa-pre.c (compute_avail): Likewise.
>>>>>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>>>>     (scc_vn_restore_ssa_info): Likewise.
>>>>>     (free_scc_vn): Likwise.
>>>>>     (run_scc_vn): Likewise.
>>>>>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>>>>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][SSA] Iterator to visit SSA
  2016-09-06 10:07           ` Kugan Vivekanandarajah
@ 2016-09-06 10:41             ` Richard Biener
  0 siblings, 0 replies; 14+ messages in thread
From: Richard Biener @ 2016-09-06 10:41 UTC (permalink / raw)
  To: Kugan Vivekanandarajah; +Cc: gcc-patches

On Tue, Sep 6, 2016 at 12:07 PM, Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi Richard,
>
>
> On 6 September 2016 at 19:57, Richard Biener <richard.guenther@gmail.com> wrote:
>> On Tue, Sep 6, 2016 at 11:33 AM, Kugan Vivekanandarajah
>> <kugan.vivekanandarajah@linaro.org> wrote:
>>> Hi Richard,
>>>
>>> On 6 September 2016 at 19:08, Richard Biener <richard.guenther@gmail.com> wrote:
>>>> On Tue, Sep 6, 2016 at 2:24 AM, Kugan Vivekanandarajah
>>>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>>> Hi Richard,
>>>>>
>>>>> On 5 September 2016 at 17:57, Richard Biener <richard.guenther@gmail.com> wrote:
>>>>>> On Mon, Sep 5, 2016 at 7:26 AM, Kugan Vivekanandarajah
>>>>>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>>>>> Hi All,
>>>>>>>
>>>>>>> While looking at gcc source, I noticed that we are iterating over SSA
>>>>>>> variable from 0 to num_ssa_names in some cases and 1 to num_ssa_names
>>>>>>> in others. It seems that variable 0 is always NULL TREE.
>>>>>>
>>>>>> Yeah, that's artificial because we don't assign SSA name version 0 (for some
>>>>>> unknown reason).
>>>>>>
>>>>>>> But it can
>>>>>>> confuse people who are looking for the first time. Therefore It might
>>>>>>> be good to follow some consistent usage here.
>>>>>>>
>>>>>>> It might be also good to gave a FOR_EACH_SSAVAR iterator as we do in
>>>>>>> other case. Here is attempt to do this based on what is done in other
>>>>>>> places. Bootstrapped and regression tested on X86_64-linux-gnu with no
>>>>>>> new regressions. is this OK?
>>>>>>
>>>>>> First of all some bikeshedding - FOR_EACH_SSA_NAME would be better
>>>>>> as SSAVAR might be confused with iterating over SSA_NAME_VAR.
>>>>>>
>>>>>> Then, if you add an iterator why leave the name == NULL handling to consumers?
>>>>>> That looks odd.
>>>>>>
>>>>>> Then, SSA names are just in a vector thus why not re-use the vector iterator?
>>>>>>
>>>>>> That is,
>>>>>>
>>>>>> #define FOR_EACH_SSA_NAME (name) \
>>>>>>   for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i)
>>>>>>
>>>>>> would be equivalent to your patch?
>>>>>>
>>>>>> Please also don't add new iterators that implicitely use 'cfun' but always use
>>>>>> one that passes it as explicit arg.
>>>>>
>>>>> I think defining FOR_EACH_SSA_NAME with vector iterator is better. But
>>>>> we will not be able to skill NULL ssa_names with that.
>>>>
>>>> Why?  Can't you simply do
>>>>
>>>>   #define FOR_EACH_SSA_NAME (name) \
>>>>     for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i) \
>>>>        if (name)
>>>>
>>>> ?
>>>
>>> For example, with the test patch where loop body also defines i which
>>> is giving error:
>>>
>>> ../../trunk/gcc/tree-ssa-ter.c: In function ‘temp_expr_table*
>>> new_temp_expr_table(var_map)’:
>>> ../../trunk/gcc/tree-ssa-ter.c:206:11: error: redeclaration of ‘int i’
>>>        int i;
>>>            ^
>>> In file included from ../../trunk/gcc/ssa.h:29:0,
>>>                  from ../../trunk/gcc/tree-ssa-ter.c:28:
>>> ../../trunk/gcc/tree-ssanames.h:66:17: note: ‘unsigned int i’
>>> previously declared here
>>>    for (unsigned i = 1; SSANAMES (cfun)->iterate (i, &VAR); ++i)
>>>                  ^
>>> ../../trunk/gcc/tree-ssa-ter.c:204:3: note: in expansion of macro
>>> ‘FOR_EACH_SSA_NAME’
>>>    FOR_EACH_SSA_NAME (name)
>>>    ^
>>> Makefile:1097: recipe for target 'tree-ssa-ter.o' failed
>>>
>>> Am I missing something here ?
>>
>> Well, my comment was not about passing 'i' to the macro but about
>> not being able to skip NULL names.  I just copied & pasted my
>> earlier macro.
>
> Sorry, I misunderstood your comment.
>
> I have:
>
> +#define FOR_EACH_SSA_NAME(i, VAR) \
> +  for (i = 1; SSANAMES (cfun)->iterate (i, &VAR); ++i)
>
> We will skip the first (0 index) if that is what you are referring here.
>
> I also thought that you wanted to skip any NULL_TREEs after that too.
> After looking at your previous your comment, I don’t you think you
> wanted that anyway.

I _did_ want that, otherwise my suggestion to use

 #define FOR_EACH_SSA_NAME(i, VAR) \
   for (i = 1; SSANAMES (cfun)->iterate (i, &VAR); ++i) \
      if (VAR)

would have made no sense.  Btw, please use capital i and put uses
in parens when not lvalues (see FOR_EACH_VEC_ELT).  As I said
I'd like 'cfun' to be explicit, thus

#define FOR_EACH_SSA_NAME(I, VAR, FN) \
  for (I = 1; SSANAMES (FN)->iterate ((I), &(VAR)); ++(I)) \
    if (VAR)

Thanks,
Richard.

> Therefore I think I am doing what you wanted in my last patch.
>
> Thanks,
> Kugan
>
>
>> Richard.
>>
>>> Thanks,
>>> Kugan
>>>
>>>>
>>>>> I also added
>>>>> index variable to the macro so that there want be any conflicts if the
>>>>> index variable "i" (or whatever) is also defined in the loop.
>>>>>
>>>>> Bootstrapped and regression tested on x86_64-linux-gnu with no new
>>>>> regressions. Is this OK for trunk?
>>>>>
>>>>> Thanks,
>>>>> Kugan
>>>>>
>>>>>
>>>>> gcc/ChangeLog:
>>>>>
>>>>> 2016-09-06  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>>>>
>>>>>     * tree-ssanames.h (FOR_EACH_SSA_NAME): New.
>>>>>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>>>>     FOR_EACH_SSA_NAME to iterate over SSA variables.
>>>>>     (pass_expand::execute): Likewise.
>>>>>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>>>>     * tree-cfg.c (dump_function_to_file): Likewise.
>>>>>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>>>>     (update_ssa): Likewise.
>>>>>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>>>>>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>>>>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>>>>     (create_outofssa_var_map): Likewise.
>>>>>     (coalesce_ssa_name): Likewise.
>>>>>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>>>>     * tree-ssa-pre.c (compute_avail): Likewise.
>>>>>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>>>>     (scc_vn_restore_ssa_info): Likewise.
>>>>>     (free_scc_vn): Likwise.
>>>>>     (run_scc_vn): Likewise.
>>>>>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>>>>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.
>>>>>     * tree-ssa-copy.c (fini_copy_prop): Likewise.
>>>>>     * tree-ssa.c (verify_ssa): Likewise.
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Richard.
>>>>>>
>>>>>>
>>>>>>> Thanks,
>>>>>>> Kugan
>>>>>>>
>>>>>>>
>>>>>>> gcc/ChangeLog:
>>>>>>>
>>>>>>> 2016-09-05  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>>>>>>
>>>>>>>     * tree-ssanames.h (ssa_iterator::ssa_iterator): New.
>>>>>>>     (ssa_iterator::get): Likewise.
>>>>>>>     (ssa_iterator::next): Likewise.
>>>>>>>     (FOR_EACH_SSAVAR): Likewise.
>>>>>>>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>>>>>>     FOR_EACH_SSAVAR to iterate over SSA variables.
>>>>>>>     (pass_expand::execute): Likewise.
>>>>>>>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>>>>>>     * tree-cfg.c (dump_function_to_file): Likewise.
>>>>>>>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>>>>>>     (update_ssa): Likewise.
>>>>>>>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>>>>>>>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>>>>>>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>>>>>>     (create_outofssa_var_map): Likewise.
>>>>>>>     (coalesce_ssa_name): Likewise.
>>>>>>>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>>>>>>     * tree-ssa-pre.c (compute_avail): Likewise.
>>>>>>>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>>>>>>     (scc_vn_restore_ssa_info): Likewise.
>>>>>>>     (free_scc_vn): Likwise.
>>>>>>>     (run_scc_vn): Likewise.
>>>>>>>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>>>>>>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][SSA] Iterator to visit SSA
  2016-09-05  7:58 ` Richard Biener
  2016-09-06  6:00   ` Kugan Vivekanandarajah
@ 2016-09-06 21:58   ` Jeff Law
  1 sibling, 0 replies; 14+ messages in thread
From: Jeff Law @ 2016-09-06 21:58 UTC (permalink / raw)
  To: Richard Biener, Kugan Vivekanandarajah; +Cc: gcc-patches

On 09/05/2016 01:57 AM, Richard Biener wrote:
> On Mon, Sep 5, 2016 at 7:26 AM, Kugan Vivekanandarajah
> <kugan.vivekanandarajah@linaro.org> wrote:
>> Hi All,
>>
>> While looking at gcc source, I noticed that we are iterating over SSA
>> variable from 0 to num_ssa_names in some cases and 1 to num_ssa_names
>> in others. It seems that variable 0 is always NULL TREE.
>
> Yeah, that's artificial because we don't assign SSA name version 0 (for some
> unknown reason).
That might have been the global for aliasing purposes eons ago.  Or it 
might have been used for default defs early in the SSA development work. 
  I wouldn't be surprised if it doesn't need to be special anymore.

Jeff

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][SSA] Iterator to visit SSA
  2016-09-06  9:08     ` Richard Biener
  2016-09-06  9:58       ` Kugan Vivekanandarajah
@ 2016-09-07  4:46       ` Kugan Vivekanandarajah
  2016-09-07  8:03         ` Dominik Inführ
  2016-09-07 11:37         ` Richard Biener
  1 sibling, 2 replies; 14+ messages in thread
From: Kugan Vivekanandarajah @ 2016-09-07  4:46 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 5208 bytes --]

Hi Richard,

On 6 September 2016 at 19:08, Richard Biener <richard.guenther@gmail.com> wrote:
> On Tue, Sep 6, 2016 at 2:24 AM, Kugan Vivekanandarajah
> <kugan.vivekanandarajah@linaro.org> wrote:
>> Hi Richard,
>>
>> On 5 September 2016 at 17:57, Richard Biener <richard.guenther@gmail.com> wrote:
>>> On Mon, Sep 5, 2016 at 7:26 AM, Kugan Vivekanandarajah
>>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>> Hi All,
>>>>
>>>> While looking at gcc source, I noticed that we are iterating over SSA
>>>> variable from 0 to num_ssa_names in some cases and 1 to num_ssa_names
>>>> in others. It seems that variable 0 is always NULL TREE.
>>>
>>> Yeah, that's artificial because we don't assign SSA name version 0 (for some
>>> unknown reason).
>>>
>>>> But it can
>>>> confuse people who are looking for the first time. Therefore It might
>>>> be good to follow some consistent usage here.
>>>>
>>>> It might be also good to gave a FOR_EACH_SSAVAR iterator as we do in
>>>> other case. Here is attempt to do this based on what is done in other
>>>> places. Bootstrapped and regression tested on X86_64-linux-gnu with no
>>>> new regressions. is this OK?
>>>
>>> First of all some bikeshedding - FOR_EACH_SSA_NAME would be better
>>> as SSAVAR might be confused with iterating over SSA_NAME_VAR.
>>>
>>> Then, if you add an iterator why leave the name == NULL handling to consumers?
>>> That looks odd.
>>>
>>> Then, SSA names are just in a vector thus why not re-use the vector iterator?
>>>
>>> That is,
>>>
>>> #define FOR_EACH_SSA_NAME (name) \
>>>   for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i)
>>>
>>> would be equivalent to your patch?
>>>
>>> Please also don't add new iterators that implicitely use 'cfun' but always use
>>> one that passes it as explicit arg.
>>
>> I think defining FOR_EACH_SSA_NAME with vector iterator is better. But
>> we will not be able to skill NULL ssa_names with that.
>
> Why?  Can't you simply do
>
>   #define FOR_EACH_SSA_NAME (name) \
>     for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i) \
>        if (name)
>
> ?

Indeed.  I missed the if at the end :(.  Here is an updated patch.
Bootstrapped and regression tested on x86_64-linux-gnu with no new
regressions.

Thanks,
Kugan
>
>> I also added
>> index variable to the macro so that there want be any conflicts if the
>> index variable "i" (or whatever) is also defined in the loop.
>>
>> Bootstrapped and regression tested on x86_64-linux-gnu with no new
>> regressions. Is this OK for trunk?
>>
>> Thanks,
>> Kugan
>>
>>
>> gcc/ChangeLog:
>>
>> 2016-09-06  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>
>>     * tree-ssanames.h (FOR_EACH_SSA_NAME): New.
>>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>     FOR_EACH_SSA_NAME to iterate over SSA variables.
>>     (pass_expand::execute): Likewise.
>>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>     * tree-cfg.c (dump_function_to_file): Likewise.
>>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>     (update_ssa): Likewise.
>>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>     (create_outofssa_var_map): Likewise.
>>     (coalesce_ssa_name): Likewise.
>>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>     * tree-ssa-pre.c (compute_avail): Likewise.
>>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>     (scc_vn_restore_ssa_info): Likewise.
>>     (free_scc_vn): Likwise.
>>     (run_scc_vn): Likewise.
>>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.
>>     * tree-ssa-copy.c (fini_copy_prop): Likewise.
>>     * tree-ssa.c (verify_ssa): Likewise.
>>
>>>
>>> Thanks,
>>> Richard.
>>>
>>>
>>>> Thanks,
>>>> Kugan
>>>>
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2016-09-05  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>>>
>>>>     * tree-ssanames.h (ssa_iterator::ssa_iterator): New.
>>>>     (ssa_iterator::get): Likewise.
>>>>     (ssa_iterator::next): Likewise.
>>>>     (FOR_EACH_SSAVAR): Likewise.
>>>>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>>>     FOR_EACH_SSAVAR to iterate over SSA variables.
>>>>     (pass_expand::execute): Likewise.
>>>>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>>>     * tree-cfg.c (dump_function_to_file): Likewise.
>>>>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>>>     (update_ssa): Likewise.
>>>>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>>>>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>>>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>>>     (create_outofssa_var_map): Likewise.
>>>>     (coalesce_ssa_name): Likewise.
>>>>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>>>     * tree-ssa-pre.c (compute_avail): Likewise.
>>>>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>>>     (scc_vn_restore_ssa_info): Likewise.
>>>>     (free_scc_vn): Likwise.
>>>>     (run_scc_vn): Likewise.
>>>>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>>>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.

[-- Attachment #2: ssa_iterator.txt --]
[-- Type: text/plain, Size: 17579 bytes --]

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 130a16b..0611e0f 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -815,16 +815,15 @@ update_alias_info_with_stack_vars (void)
   if (decls_to_partitions)
     {
       unsigned i;
+      tree name;
       hash_set<bitmap> visited;
       bitmap temp = BITMAP_ALLOC (&stack_var_bitmap_obstack);
 
-      for (i = 1; i < num_ssa_names; i++)
+      FOR_EACH_SSA_NAME (i, name)
 	{
-	  tree name = ssa_name (i);
 	  struct ptr_info_def *pi;
 
-	  if (name
-	      && POINTER_TYPE_P (TREE_TYPE (name))
+	  if (POINTER_TYPE_P (TREE_TYPE (name))
 	      && ((pi = SSA_NAME_PTR_INFO (name)) != NULL))
 	    add_partitioned_vars_to_ptset (&pi->pt, decls_to_partitions,
 					   &visited, temp);
@@ -6163,6 +6162,7 @@ pass_expand::execute (function *fun)
   edge e;
   rtx_insn *var_seq, *var_ret_seq;
   unsigned i;
+  tree name;
 
   timevar_push (TV_OUT_OF_SSA);
   rewrite_out_of_ssa (&SA);
@@ -6270,16 +6270,13 @@ pass_expand::execute (function *fun)
 
   /* Now propagate the RTL assignment of each partition to the
      underlying var of each SSA_NAME.  */
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
-
-      if (!name
-	  /* We might have generated new SSA names in
-	     update_alias_info_with_stack_vars.  They will have a NULL
-	     defining statements, and won't be part of the partitioning,
-	     so ignore those.  */
-	  || !SSA_NAME_DEF_STMT (name))
+      /* We might have generated new SSA names in
+	 update_alias_info_with_stack_vars.  They will have a NULL
+	 defining statements, and won't be part of the partitioning,
+	 so ignore those.  */
+      if (!SSA_NAME_DEF_STMT (name))
 	continue;
 
       adjust_one_expanded_partition_var (name);
@@ -6288,17 +6285,15 @@ pass_expand::execute (function *fun)
   /* Clean up RTL of variables that straddle across multiple
      partitions, and check that the rtl of any PARM_DECLs that are not
      cleaned up is that of their default defs.  */
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
       int part;
 
-      if (!name
-	  /* We might have generated new SSA names in
-	     update_alias_info_with_stack_vars.  They will have a NULL
-	     defining statements, and won't be part of the partitioning,
-	     so ignore those.  */
-	  || !SSA_NAME_DEF_STMT (name))
+      /* We might have generated new SSA names in
+	 update_alias_info_with_stack_vars.  They will have a NULL
+	 defining statements, and won't be part of the partitioning,
+	 so ignore those.  */
+      if (!SSA_NAME_DEF_STMT (name))
 	continue;
       part = var_to_partition (SA.map, name);
       if (part == NO_PARTITION)
diff --git a/gcc/omp-simd-clone.c b/gcc/omp-simd-clone.c
index c418440c..e6589bb 100644
--- a/gcc/omp-simd-clone.c
+++ b/gcc/omp-simd-clone.c
@@ -908,6 +908,7 @@ ipa_simd_modify_function_body (struct cgraph_node *node,
 {
   basic_block bb;
   unsigned int i, j, l;
+  tree name;
 
   /* Re-use the adjustments array, but this time use it to replace
      every function argument use to an offset into the corresponding
@@ -931,11 +932,9 @@ ipa_simd_modify_function_body (struct cgraph_node *node,
     }
 
   l = adjustments.length ();
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
-      if (name
-	  && SSA_NAME_VAR (name)
+      if (SSA_NAME_VAR (name)
 	  && TREE_CODE (SSA_NAME_VAR (name)) == PARM_DECL)
 	{
 	  for (j = 0; j < l; j++)
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 57c8410..72e645d 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -7539,6 +7539,7 @@ dump_function_to_file (tree fndecl, FILE *file, int flags)
   if (fun && fun->decl == fndecl && (fun->curr_properties & PROP_gimple_lcf))
     {
       unsigned ix;
+      tree name;
       ignore_topmost_bind = true;
 
       fprintf (file, "{\n");
@@ -7582,10 +7583,9 @@ dump_function_to_file (tree fndecl, FILE *file, int flags)
 	    any_var = true;
 	  }
       if (gimple_in_ssa_p (cfun))
-	for (ix = 1; ix < num_ssa_names; ++ix)
+	FOR_EACH_SSA_NAME (ix, name)
 	  {
-	    tree name = ssa_name (ix);
-	    if (name && !SSA_NAME_VAR (name))
+	    if (!SSA_NAME_VAR (name))
 	      {
 		fprintf (file, "  ");
 		print_generic_expr (file, TREE_TYPE (name), flags);
diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
index ceafa68..d73dad7 100644
--- a/gcc/tree-into-ssa.c
+++ b/gcc/tree-into-ssa.c
@@ -2342,6 +2342,7 @@ pass_build_ssa::execute (function *fun)
   bitmap_head *dfs;
   basic_block bb;
   unsigned i;
+  tree decl, name;
 
   /* Initialize operand data structures.  */
   init_ssa_operands (fun);
@@ -2385,11 +2386,9 @@ pass_build_ssa::execute (function *fun)
   /* Try to get rid of all gimplifier generated temporaries by making
      its SSA names anonymous.  This way we can garbage collect them
      all after removing unused locals which we do in our TODO.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree decl, name = ssa_name (i);
-      if (!name
-	  || SSA_NAME_IS_DEFAULT_DEF (name))
+      if (SSA_NAME_IS_DEFAULT_DEF (name))
 	continue;
       decl = SSA_NAME_VAR (name);
       if (decl
@@ -3165,7 +3164,7 @@ update_ssa (unsigned update_flags)
   unsigned i = 0;
   bool insert_phi_p;
   sbitmap_iterator sbi;
-  tree sym;
+  tree sym, name;
 
   /* Only one update flag should be set.  */
   gcc_assert (update_flags == TODO_update_ssa
@@ -3284,11 +3283,9 @@ update_ssa (unsigned update_flags)
       prepare_block_for_update (start_bb, insert_phi_p);
 
       if (flag_checking)
-	for (i = 1; i < num_ssa_names; ++i)
+	FOR_EACH_SSA_NAME (i, name)
 	  {
-	    tree name = ssa_name (i);
-	    if (!name
-		|| virtual_operand_p (name))
+	    if (virtual_operand_p (name))
 	      continue;
 
 	    /* For all but virtual operands, which do not have SSA names
diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 8051a66..de0b1f9 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -450,6 +450,7 @@ void
 dump_alias_info (FILE *file)
 {
   unsigned i;
+  tree ptr;
   const char *funcname
     = lang_hooks.decl_printable_name (current_function_decl, 2);
   tree var;
@@ -471,13 +472,11 @@ dump_alias_info (FILE *file)
 
   fprintf (file, "\n\nFlow-insensitive points-to information\n\n");
 
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, ptr)
     {
-      tree ptr = ssa_name (i);
       struct ptr_info_def *pi;
 
-      if (ptr == NULL_TREE
-	  || !POINTER_TYPE_P (TREE_TYPE (ptr))
+      if (!POINTER_TYPE_P (TREE_TYPE (ptr))
 	  || SSA_NAME_IN_FREE_LIST (ptr))
 	continue;
 
diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c
index 9871304..cbded7b 100644
--- a/gcc/tree-ssa-ccp.c
+++ b/gcc/tree-ssa-ccp.c
@@ -898,24 +898,23 @@ ccp_finalize (bool nonzero_p)
 {
   bool something_changed;
   unsigned i;
+  tree name;
 
   do_dbg_cnt ();
 
   /* Derive alignment and misalignment information from partially
      constant pointers in the lattice or nonzero bits from partially
      constant integers.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
       ccp_prop_value_t *val;
       unsigned int tem, align;
 
-      if (!name
-	  || (!POINTER_TYPE_P (TREE_TYPE (name))
-	      && (!INTEGRAL_TYPE_P (TREE_TYPE (name))
-		  /* Don't record nonzero bits before IPA to avoid
-		     using too much memory.  */
-		  || !nonzero_p)))
+      if (!POINTER_TYPE_P (TREE_TYPE (name))
+	  && (!INTEGRAL_TYPE_P (TREE_TYPE (name))
+	      /* Don't record nonzero bits before IPA to avoid
+		 using too much memory.  */
+	      || !nonzero_p))
 	continue;
 
       val = get_value (name);
diff --git a/gcc/tree-ssa-coalesce.c b/gcc/tree-ssa-coalesce.c
index 34c3fa1..d9a27ee 100644
--- a/gcc/tree-ssa-coalesce.c
+++ b/gcc/tree-ssa-coalesce.c
@@ -955,12 +955,11 @@ build_ssa_conflict_graph (tree_live_info_p liveinfo)
       if (bb == entry)
 	{
 	  unsigned i;
-	  for (i = 1; i < num_ssa_names; i++)
-	    {
-	      tree var = ssa_name (i);
+	  tree var;
 
-	      if (!var
-		  || !SSA_NAME_IS_DEFAULT_DEF (var)
+	  FOR_EACH_SSA_NAME (i, var)
+	    {
+	      if (!SSA_NAME_IS_DEFAULT_DEF (var)
 		  || !SSA_NAME_VAR (var)
 		  || VAR_P (SSA_NAME_VAR (var)))
 		continue;
@@ -1261,10 +1260,9 @@ create_outofssa_var_map (coalesce_list *cl, bitmap used_in_copy)
   /* Now process result decls and live on entry variables for entry into
      the coalesce list.  */
   first = NULL_TREE;
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, var)
     {
-      var = ssa_name (i);
-      if (var != NULL_TREE && !virtual_operand_p (var))
+      if (!virtual_operand_p (var))
         {
 	  coalesce_with_default (var, cl, used_in_copy);
 
@@ -1806,6 +1804,7 @@ coalesce_ssa_name (void)
   bitmap used_in_copies = BITMAP_ALLOC (NULL);
   var_map map;
   unsigned int i;
+  tree a;
 
   cl = create_coalesce_list ();
   map = create_outofssa_var_map (cl, used_in_copies);
@@ -1817,12 +1816,9 @@ coalesce_ssa_name (void)
     {
       hash_table<ssa_name_var_hash> ssa_name_hash (10);
 
-      for (i = 1; i < num_ssa_names; i++)
+      FOR_EACH_SSA_NAME (i, a)
 	{
-	  tree a = ssa_name (i);
-
-	  if (a
-	      && SSA_NAME_VAR (a)
+	  if (SSA_NAME_VAR (a)
 	      && !DECL_IGNORED_P (SSA_NAME_VAR (a))
 	      && (!has_zero_uses (a) || !SSA_NAME_IS_DEFAULT_DEF (a)
 		  || !VAR_P (SSA_NAME_VAR (a))))
diff --git a/gcc/tree-ssa-copy.c b/gcc/tree-ssa-copy.c
index 3d17926..1a3c69f 100644
--- a/gcc/tree-ssa-copy.c
+++ b/gcc/tree-ssa-copy.c
@@ -503,14 +503,13 @@ static bool
 fini_copy_prop (void)
 {
   unsigned i;
+  tree var;
 
   /* Set the final copy-of value for each variable by traversing the
      copy-of chains.  */
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, var)
     {
-      tree var = ssa_name (i);
-      if (!var
-	  || !copy_of[i].value
+      if (!copy_of[i].value
 	  || copy_of[i].value == var)
 	continue;
 
diff --git a/gcc/tree-ssa-operands.c b/gcc/tree-ssa-operands.c
index eccea2f..140e1c5 100644
--- a/gcc/tree-ssa-operands.c
+++ b/gcc/tree-ssa-operands.c
@@ -1247,11 +1247,8 @@ dump_immediate_uses (FILE *file)
   unsigned int x;
 
   fprintf (file, "Immediate_uses: \n\n");
-  for (x = 1; x < num_ssa_names; x++)
+  FOR_EACH_SSA_NAME (x, var)
     {
-      var = ssa_name (x);
-      if (!var)
-        continue;
       dump_immediate_uses_for (file, var);
     }
 }
diff --git a/gcc/tree-ssa-pre.c b/gcc/tree-ssa-pre.c
index fdb1c2c..b608a8d 100644
--- a/gcc/tree-ssa-pre.c
+++ b/gcc/tree-ssa-pre.c
@@ -3670,15 +3670,14 @@ compute_avail (void)
   basic_block *worklist;
   size_t sp = 0;
   unsigned i;
+  tree name;
 
   /* We pretend that default definitions are defined in the entry block.
      This includes function arguments and the static chain decl.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
       pre_expr e;
-      if (!name
-	  || !SSA_NAME_IS_DEFAULT_DEF (name)
+      if (!SSA_NAME_IS_DEFAULT_DEF (name)
 	  || has_zero_uses (name)
 	  || virtual_operand_p (name))
 	continue;
diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c
index 21b3d56..898a7c6 100644
--- a/gcc/tree-ssa-sccvn.c
+++ b/gcc/tree-ssa-sccvn.c
@@ -4291,6 +4291,7 @@ static void
 init_scc_vn (void)
 {
   size_t i;
+  tree name;
   int j;
   int *rpo_numbers_temp;
 
@@ -4339,12 +4340,8 @@ init_scc_vn (void)
 
   /* Create the VN_INFO structures, and initialize value numbers to
      TOP or VARYING for parameters.  */
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
-      if (!name)
-	continue;
-
       VN_INFO_GET (name)->valnum = VN_TOP;
       VN_INFO (name)->needs_insertion = false;
       VN_INFO (name)->expr = NULL;
@@ -4402,11 +4399,12 @@ init_scc_vn (void)
 void
 scc_vn_restore_ssa_info (void)
 {
-  for (unsigned i = 0; i < num_ssa_names; i++)
+  unsigned i;
+  tree name;
+
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
-      if (name
-	  && has_VN_INFO (name))
+      if (has_VN_INFO (name))
 	{
 	  if (VN_INFO (name)->needs_insertion)
 	    ;
@@ -4428,6 +4426,7 @@ void
 free_scc_vn (void)
 {
   size_t i;
+  tree name;
 
   delete constant_to_value_id;
   constant_to_value_id = NULL;
@@ -4436,11 +4435,9 @@ free_scc_vn (void)
   shared_lookup_references.release ();
   XDELETEVEC (rpo_numbers);
 
-  for (i = 0; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
-      if (name
-	  && has_VN_INFO (name)
+      if (has_VN_INFO (name)
 	  && VN_INFO (name)->needs_insertion)
 	release_ssa_name (name);
     }
@@ -4757,6 +4754,7 @@ bool
 run_scc_vn (vn_lookup_kind default_vn_walk_kind_)
 {
   size_t i;
+  tree name;
 
   default_vn_walk_kind = default_vn_walk_kind_;
 
@@ -4797,9 +4795,8 @@ run_scc_vn (vn_lookup_kind default_vn_walk_kind_)
 
   /* Initialize the value ids and prune out remaining VN_TOPs
      from dead code.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
       vn_ssa_aux_t info;
       if (!name)
 	continue;
@@ -4814,12 +4811,9 @@ run_scc_vn (vn_lookup_kind default_vn_walk_kind_)
     }
 
   /* Propagate.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
       vn_ssa_aux_t info;
-      if (!name)
-	continue;
       info = VN_INFO (name);
       if (TREE_CODE (info->valnum) == SSA_NAME
 	  && info->valnum != name
@@ -4832,11 +4826,9 @@ run_scc_vn (vn_lookup_kind default_vn_walk_kind_)
   if (dump_file && (dump_flags & TDF_DETAILS))
     {
       fprintf (dump_file, "Value numbers:\n");
-      for (i = 0; i < num_ssa_names; i++)
+      FOR_EACH_SSA_NAME (i, name)
 	{
-	  tree name = ssa_name (i);
-	  if (name
-	      && VN_INFO (name)->visited
+	  if (VN_INFO (name)->visited
 	      && SSA_VAL (name) != name)
 	    {
 	      print_generic_expr (dump_file, name, 0);
diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index fd96c3a..0149bec 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -7029,6 +7029,7 @@ compute_points_to_sets (void)
 {
   basic_block bb;
   unsigned i;
+  tree ptr;
   varinfo_t vi;
 
   timevar_push (TV_TREE_PTA);
@@ -7077,11 +7078,9 @@ compute_points_to_sets (void)
   cfun->gimple_df->escaped.escaped = 0;
 
   /* Compute the points-to sets for pointer SSA_NAMEs.  */
-  for (i = 0; i < num_ssa_names; ++i)
+  FOR_EACH_SSA_NAME (i, ptr)
     {
-      tree ptr = ssa_name (i);
-      if (ptr
-	  && POINTER_TYPE_P (TREE_TYPE (ptr)))
+      if (POINTER_TYPE_P (TREE_TYPE (ptr)))
 	find_what_p_points_to (cfun->decl, ptr);
     }
 
diff --git a/gcc/tree-ssa-ter.c b/gcc/tree-ssa-ter.c
index 2a772b2..16ccd9d 100644
--- a/gcc/tree-ssa-ter.c
+++ b/gcc/tree-ssa-ter.c
@@ -186,6 +186,7 @@ static temp_expr_table *
 new_temp_expr_table (var_map map)
 {
   unsigned x;
+  tree name;
 
   temp_expr_table *t = XNEW (struct temp_expr_table);
   t->map = map;
@@ -201,12 +202,9 @@ new_temp_expr_table (var_map map)
 
   t->replaceable_expressions = NULL;
   t->num_in_part = XCNEWVEC (int, num_var_partitions (map));
-  for (x = 1; x < num_ssa_names; x++)
+  FOR_EACH_SSA_NAME (x, name)
     {
       int p;
-      tree name = ssa_name (x);
-      if (!name)
-        continue;
       p = var_to_partition (map, name);
       if (p != NO_PARTITION)
         t->num_in_part[p]++;
diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c
index aae383d..b243f5c 100644
--- a/gcc/tree-ssa.c
+++ b/gcc/tree-ssa.c
@@ -1010,6 +1010,7 @@ verify_ssa (bool check_modified_stmt, bool check_ssa_operands)
   basic_block *definition_block = XCNEWVEC (basic_block, num_ssa_names);
   ssa_op_iter iter;
   tree op;
+  tree name;
   enum dom_state orig_dom_state = dom_info_state (CDI_DOMINATORS);
   bitmap names_defined_in_bb = BITMAP_ALLOC (NULL);
 
@@ -1018,24 +1019,20 @@ verify_ssa (bool check_modified_stmt, bool check_ssa_operands)
   timevar_push (TV_TREE_SSA_VERIFY);
 
   /* Keep track of SSA names present in the IL.  */
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, name)
     {
-      tree name = ssa_name (i);
-      if (name)
-	{
-	  gimple *stmt;
-	  TREE_VISITED (name) = 0;
+      gimple *stmt;
+      TREE_VISITED (name) = 0;
 
-	  verify_ssa_name (name, virtual_operand_p (name));
+      verify_ssa_name (name, virtual_operand_p (name));
 
-	  stmt = SSA_NAME_DEF_STMT (name);
-	  if (!gimple_nop_p (stmt))
-	    {
-	      basic_block bb = gimple_bb (stmt);
-	      if (verify_def (bb, definition_block,
-			      name, stmt, virtual_operand_p (name)))
-		goto err;
-	    }
+      stmt = SSA_NAME_DEF_STMT (name);
+      if (!gimple_nop_p (stmt))
+	{
+	  basic_block bb = gimple_bb (stmt);
+	  if (verify_def (bb, definition_block,
+			  name, stmt, virtual_operand_p (name)))
+	    goto err;
 	}
     }
 
diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
index 8e66ce6..1d53774 100644
--- a/gcc/tree-ssanames.h
+++ b/gcc/tree-ssanames.h
@@ -62,6 +62,10 @@ struct GTY ((variable_size)) range_info_def {
 #define num_ssa_names (vec_safe_length (cfun->gimple_df->ssa_names))
 #define ssa_name(i) ((*cfun->gimple_df->ssa_names)[(i)])
 
+#define FOR_EACH_SSA_NAME(I, VAR) \
+  for (I = 1; SSANAMES (cfun)->iterate (I, &VAR); ++I)	\
+    if (VAR)
+
 /* Sets the value range to SSA.  */
 extern void set_range_info (tree, enum value_range_type, const wide_int_ref &,
 			    const wide_int_ref &);

[-- Attachment #3: ssa_iterator.log --]
[-- Type: text/x-log, Size: 1110 bytes --]

gcc/ChangeLog:

2016-09-07  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* tree-ssanames.h (FOR_EACH_SSA_NAME): New.
	* cfgexpand.c (update_alias_info_with_stack_vars): Use
	FOR_EACH_SSA_NAME to iterate over SSA variables.
	(pass_expand::execute): Likewise.
	* omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
	* tree-cfg.c (dump_function_to_file): Likewise.
	* tree-into-ssa.c (pass_build_ssa::execute): Likewise.
	(update_ssa): Likewise.
	* tree-ssa-alias.c (dump_alias_info): Likewise.
	* tree-ssa-ccp.c (ccp_finalize): Likewise.
	* tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
	(create_outofssa_var_map): Likewise.
	(coalesce_ssa_name): Likewise.
	* tree-ssa-operands.c (dump_immediate_uses): Likewise.
	* tree-ssa-pre.c (compute_avail): Likewise.
	* tree-ssa-sccvn.c (init_scc_vn): Likewise.
	(scc_vn_restore_ssa_info): Likewise.
	(free_scc_vn): Likwise.
	(run_scc_vn): Likewise.
	* tree-ssa-structalias.c (compute_points_to_sets): Likewise.
	* tree-ssa-ter.c (new_temp_expr_table): Likewise.
	* tree-ssa-copy.c (fini_copy_prop): Likewise.
	* tree-ssa.c (verify_ssa): Likewise.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][SSA] Iterator to visit SSA
  2016-09-07  4:46       ` Kugan Vivekanandarajah
@ 2016-09-07  8:03         ` Dominik Inführ
  2016-09-07 11:37         ` Richard Biener
  1 sibling, 0 replies; 14+ messages in thread
From: Dominik Inführ @ 2016-09-07  8:03 UTC (permalink / raw)
  To: Kugan Vivekanandarajah; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 5761 bytes --]

I am not sure about the process, but it may also be nice/useful to add your new macro to ForEachMacros in contrib/clang-format.

Dominik

> On 07 Sep 2016, at 02:21, Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org> wrote:
> 
> Hi Richard,
> 
> On 6 September 2016 at 19:08, Richard Biener <richard.guenther@gmail.com> wrote:
>> On Tue, Sep 6, 2016 at 2:24 AM, Kugan Vivekanandarajah
>> <kugan.vivekanandarajah@linaro.org> wrote:
>>> Hi Richard,
>>> 
>>> On 5 September 2016 at 17:57, Richard Biener <richard.guenther@gmail.com> wrote:
>>>> On Mon, Sep 5, 2016 at 7:26 AM, Kugan Vivekanandarajah
>>>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>>> Hi All,
>>>>> 
>>>>> While looking at gcc source, I noticed that we are iterating over SSA
>>>>> variable from 0 to num_ssa_names in some cases and 1 to num_ssa_names
>>>>> in others. It seems that variable 0 is always NULL TREE.
>>>> 
>>>> Yeah, that's artificial because we don't assign SSA name version 0 (for some
>>>> unknown reason).
>>>> 
>>>>> But it can
>>>>> confuse people who are looking for the first time. Therefore It might
>>>>> be good to follow some consistent usage here.
>>>>> 
>>>>> It might be also good to gave a FOR_EACH_SSAVAR iterator as we do in
>>>>> other case. Here is attempt to do this based on what is done in other
>>>>> places. Bootstrapped and regression tested on X86_64-linux-gnu with no
>>>>> new regressions. is this OK?
>>>> 
>>>> First of all some bikeshedding - FOR_EACH_SSA_NAME would be better
>>>> as SSAVAR might be confused with iterating over SSA_NAME_VAR.
>>>> 
>>>> Then, if you add an iterator why leave the name == NULL handling to consumers?
>>>> That looks odd.
>>>> 
>>>> Then, SSA names are just in a vector thus why not re-use the vector iterator?
>>>> 
>>>> That is,
>>>> 
>>>> #define FOR_EACH_SSA_NAME (name) \
>>>>  for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i)
>>>> 
>>>> would be equivalent to your patch?
>>>> 
>>>> Please also don't add new iterators that implicitely use 'cfun' but always use
>>>> one that passes it as explicit arg.
>>> 
>>> I think defining FOR_EACH_SSA_NAME with vector iterator is better. But
>>> we will not be able to skill NULL ssa_names with that.
>> 
>> Why?  Can't you simply do
>> 
>>  #define FOR_EACH_SSA_NAME (name) \
>>    for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i) \
>>       if (name)
>> 
>> ?
> 
> Indeed.  I missed the if at the end :(.  Here is an updated patch.
> Bootstrapped and regression tested on x86_64-linux-gnu with no new
> regressions.
> 
> Thanks,
> Kugan
>> 
>>> I also added
>>> index variable to the macro so that there want be any conflicts if the
>>> index variable "i" (or whatever) is also defined in the loop.
>>> 
>>> Bootstrapped and regression tested on x86_64-linux-gnu with no new
>>> regressions. Is this OK for trunk?
>>> 
>>> Thanks,
>>> Kugan
>>> 
>>> 
>>> gcc/ChangeLog:
>>> 
>>> 2016-09-06  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>> 
>>>    * tree-ssanames.h (FOR_EACH_SSA_NAME): New.
>>>    * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>>    FOR_EACH_SSA_NAME to iterate over SSA variables.
>>>    (pass_expand::execute): Likewise.
>>>    * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>>    * tree-cfg.c (dump_function_to_file): Likewise.
>>>    * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>>    (update_ssa): Likewise.
>>>    * tree-ssa-alias.c (dump_alias_info): Likewise.
>>>    * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>>    * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>>    (create_outofssa_var_map): Likewise.
>>>    (coalesce_ssa_name): Likewise.
>>>    * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>>    * tree-ssa-pre.c (compute_avail): Likewise.
>>>    * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>>    (scc_vn_restore_ssa_info): Likewise.
>>>    (free_scc_vn): Likwise.
>>>    (run_scc_vn): Likewise.
>>>    * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>>    * tree-ssa-ter.c (new_temp_expr_table): Likewise.
>>>    * tree-ssa-copy.c (fini_copy_prop): Likewise.
>>>    * tree-ssa.c (verify_ssa): Likewise.
>>> 
>>>> 
>>>> Thanks,
>>>> Richard.
>>>> 
>>>> 
>>>>> Thanks,
>>>>> Kugan
>>>>> 
>>>>> 
>>>>> gcc/ChangeLog:
>>>>> 
>>>>> 2016-09-05  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>>>> 
>>>>>    * tree-ssanames.h (ssa_iterator::ssa_iterator): New.
>>>>>    (ssa_iterator::get): Likewise.
>>>>>    (ssa_iterator::next): Likewise.
>>>>>    (FOR_EACH_SSAVAR): Likewise.
>>>>>    * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>>>>    FOR_EACH_SSAVAR to iterate over SSA variables.
>>>>>    (pass_expand::execute): Likewise.
>>>>>    * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>>>>    * tree-cfg.c (dump_function_to_file): Likewise.
>>>>>    * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>>>>    (update_ssa): Likewise.
>>>>>    * tree-ssa-alias.c (dump_alias_info): Likewise.
>>>>>    * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>>>>    * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>>>>    (create_outofssa_var_map): Likewise.
>>>>>    (coalesce_ssa_name): Likewise.
>>>>>    * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>>>>    * tree-ssa-pre.c (compute_avail): Likewise.
>>>>>    * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>>>>    (scc_vn_restore_ssa_info): Likewise.
>>>>>    (free_scc_vn): Likwise.
>>>>>    (run_scc_vn): Likewise.
>>>>>    * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>>>>    * tree-ssa-ter.c (new_temp_expr_table): Likewise.
> <ssa_iterator.txt><ssa_iterator.log>


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 842 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][SSA] Iterator to visit SSA
  2016-09-07  4:46       ` Kugan Vivekanandarajah
  2016-09-07  8:03         ` Dominik Inführ
@ 2016-09-07 11:37         ` Richard Biener
  2016-09-08  5:57           ` Kugan Vivekanandarajah
  1 sibling, 1 reply; 14+ messages in thread
From: Richard Biener @ 2016-09-07 11:37 UTC (permalink / raw)
  To: Kugan Vivekanandarajah; +Cc: gcc-patches

On Wed, Sep 7, 2016 at 2:21 AM, Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi Richard,
>
> On 6 September 2016 at 19:08, Richard Biener <richard.guenther@gmail.com> wrote:
>> On Tue, Sep 6, 2016 at 2:24 AM, Kugan Vivekanandarajah
>> <kugan.vivekanandarajah@linaro.org> wrote:
>>> Hi Richard,
>>>
>>> On 5 September 2016 at 17:57, Richard Biener <richard.guenther@gmail.com> wrote:
>>>> On Mon, Sep 5, 2016 at 7:26 AM, Kugan Vivekanandarajah
>>>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>>> Hi All,
>>>>>
>>>>> While looking at gcc source, I noticed that we are iterating over SSA
>>>>> variable from 0 to num_ssa_names in some cases and 1 to num_ssa_names
>>>>> in others. It seems that variable 0 is always NULL TREE.
>>>>
>>>> Yeah, that's artificial because we don't assign SSA name version 0 (for some
>>>> unknown reason).
>>>>
>>>>> But it can
>>>>> confuse people who are looking for the first time. Therefore It might
>>>>> be good to follow some consistent usage here.
>>>>>
>>>>> It might be also good to gave a FOR_EACH_SSAVAR iterator as we do in
>>>>> other case. Here is attempt to do this based on what is done in other
>>>>> places. Bootstrapped and regression tested on X86_64-linux-gnu with no
>>>>> new regressions. is this OK?
>>>>
>>>> First of all some bikeshedding - FOR_EACH_SSA_NAME would be better
>>>> as SSAVAR might be confused with iterating over SSA_NAME_VAR.
>>>>
>>>> Then, if you add an iterator why leave the name == NULL handling to consumers?
>>>> That looks odd.
>>>>
>>>> Then, SSA names are just in a vector thus why not re-use the vector iterator?
>>>>
>>>> That is,
>>>>
>>>> #define FOR_EACH_SSA_NAME (name) \
>>>>   for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i)
>>>>
>>>> would be equivalent to your patch?
>>>>
>>>> Please also don't add new iterators that implicitely use 'cfun' but always use
>>>> one that passes it as explicit arg.
>>>
>>> I think defining FOR_EACH_SSA_NAME with vector iterator is better. But
>>> we will not be able to skill NULL ssa_names with that.
>>
>> Why?  Can't you simply do
>>
>>   #define FOR_EACH_SSA_NAME (name) \
>>     for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i) \
>>        if (name)
>>
>> ?
>
> Indeed.  I missed the if at the end :(.  Here is an updated patch.
> Bootstrapped and regression tested on x86_64-linux-gnu with no new
> regressions.

Quoting myself:

> As I said
> I'd like 'cfun' to be explicit, thus
>
> #define FOR_EACH_SSA_NAME(I, VAR, FN) \
>  for (I = 1; SSANAMES (FN)->iterate ((I), &(VAR)); ++(I)) \
>   if (VAR)

the patch doesn't seem to contain that FN part.  Please also put declarations
of 'name' you need to add right before the FOR_EACH_SSA_NAME rather
than far away.

Thanks,
Richard.

> Thanks,
> Kugan
>>
>>> I also added
>>> index variable to the macro so that there want be any conflicts if the
>>> index variable "i" (or whatever) is also defined in the loop.
>>>
>>> Bootstrapped and regression tested on x86_64-linux-gnu with no new
>>> regressions. Is this OK for trunk?
>>>
>>> Thanks,
>>> Kugan
>>>
>>>
>>> gcc/ChangeLog:
>>>
>>> 2016-09-06  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>>
>>>     * tree-ssanames.h (FOR_EACH_SSA_NAME): New.
>>>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>>     FOR_EACH_SSA_NAME to iterate over SSA variables.
>>>     (pass_expand::execute): Likewise.
>>>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>>     * tree-cfg.c (dump_function_to_file): Likewise.
>>>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>>     (update_ssa): Likewise.
>>>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>>>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>>     (create_outofssa_var_map): Likewise.
>>>     (coalesce_ssa_name): Likewise.
>>>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>>     * tree-ssa-pre.c (compute_avail): Likewise.
>>>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>>     (scc_vn_restore_ssa_info): Likewise.
>>>     (free_scc_vn): Likwise.
>>>     (run_scc_vn): Likewise.
>>>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.
>>>     * tree-ssa-copy.c (fini_copy_prop): Likewise.
>>>     * tree-ssa.c (verify_ssa): Likewise.
>>>
>>>>
>>>> Thanks,
>>>> Richard.
>>>>
>>>>
>>>>> Thanks,
>>>>> Kugan
>>>>>
>>>>>
>>>>> gcc/ChangeLog:
>>>>>
>>>>> 2016-09-05  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>>>>
>>>>>     * tree-ssanames.h (ssa_iterator::ssa_iterator): New.
>>>>>     (ssa_iterator::get): Likewise.
>>>>>     (ssa_iterator::next): Likewise.
>>>>>     (FOR_EACH_SSAVAR): Likewise.
>>>>>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>>>>     FOR_EACH_SSAVAR to iterate over SSA variables.
>>>>>     (pass_expand::execute): Likewise.
>>>>>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>>>>     * tree-cfg.c (dump_function_to_file): Likewise.
>>>>>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>>>>     (update_ssa): Likewise.
>>>>>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>>>>>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>>>>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>>>>     (create_outofssa_var_map): Likewise.
>>>>>     (coalesce_ssa_name): Likewise.
>>>>>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>>>>     * tree-ssa-pre.c (compute_avail): Likewise.
>>>>>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>>>>     (scc_vn_restore_ssa_info): Likewise.
>>>>>     (free_scc_vn): Likwise.
>>>>>     (run_scc_vn): Likewise.
>>>>>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>>>>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][SSA] Iterator to visit SSA
  2016-09-07 11:37         ` Richard Biener
@ 2016-09-08  5:57           ` Kugan Vivekanandarajah
  2016-09-14 10:21             ` Richard Biener
  0 siblings, 1 reply; 14+ messages in thread
From: Kugan Vivekanandarajah @ 2016-09-08  5:57 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 6232 bytes --]

Hi Richard,

On 7 September 2016 at 19:35, Richard Biener <richard.guenther@gmail.com> wrote:
> On Wed, Sep 7, 2016 at 2:21 AM, Kugan Vivekanandarajah
> <kugan.vivekanandarajah@linaro.org> wrote:
>> Hi Richard,
>>
>> On 6 September 2016 at 19:08, Richard Biener <richard.guenther@gmail.com> wrote:
>>> On Tue, Sep 6, 2016 at 2:24 AM, Kugan Vivekanandarajah
>>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>> Hi Richard,
>>>>
>>>> On 5 September 2016 at 17:57, Richard Biener <richard.guenther@gmail.com> wrote:
>>>>> On Mon, Sep 5, 2016 at 7:26 AM, Kugan Vivekanandarajah
>>>>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>>>> Hi All,
>>>>>>
>>>>>> While looking at gcc source, I noticed that we are iterating over SSA
>>>>>> variable from 0 to num_ssa_names in some cases and 1 to num_ssa_names
>>>>>> in others. It seems that variable 0 is always NULL TREE.
>>>>>
>>>>> Yeah, that's artificial because we don't assign SSA name version 0 (for some
>>>>> unknown reason).
>>>>>
>>>>>> But it can
>>>>>> confuse people who are looking for the first time. Therefore It might
>>>>>> be good to follow some consistent usage here.
>>>>>>
>>>>>> It might be also good to gave a FOR_EACH_SSAVAR iterator as we do in
>>>>>> other case. Here is attempt to do this based on what is done in other
>>>>>> places. Bootstrapped and regression tested on X86_64-linux-gnu with no
>>>>>> new regressions. is this OK?
>>>>>
>>>>> First of all some bikeshedding - FOR_EACH_SSA_NAME would be better
>>>>> as SSAVAR might be confused with iterating over SSA_NAME_VAR.
>>>>>
>>>>> Then, if you add an iterator why leave the name == NULL handling to consumers?
>>>>> That looks odd.
>>>>>
>>>>> Then, SSA names are just in a vector thus why not re-use the vector iterator?
>>>>>
>>>>> That is,
>>>>>
>>>>> #define FOR_EACH_SSA_NAME (name) \
>>>>>   for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i)
>>>>>
>>>>> would be equivalent to your patch?
>>>>>
>>>>> Please also don't add new iterators that implicitely use 'cfun' but always use
>>>>> one that passes it as explicit arg.
>>>>
>>>> I think defining FOR_EACH_SSA_NAME with vector iterator is better. But
>>>> we will not be able to skill NULL ssa_names with that.
>>>
>>> Why?  Can't you simply do
>>>
>>>   #define FOR_EACH_SSA_NAME (name) \
>>>     for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i) \
>>>        if (name)
>>>
>>> ?
>>
>> Indeed.  I missed the if at the end :(.  Here is an updated patch.
>> Bootstrapped and regression tested on x86_64-linux-gnu with no new
>> regressions.
>
> Quoting myself:
>
>> As I said
>> I'd like 'cfun' to be explicit, thus
>>
>> #define FOR_EACH_SSA_NAME(I, VAR, FN) \
>>  for (I = 1; SSANAMES (FN)->iterate ((I), &(VAR)); ++(I)) \
>>   if (VAR)
>
> the patch doesn't seem to contain that FN part.  Please also put declarations
> of 'name' you need to add right before the FOR_EACH_SSA_NAME rather
> than far away.

Please find the attached patch does this. Bootstrapped and regression
tested on x86_64-linux-gnu with no new regressions.

Is this OK?

Thanks,
Kugan

>
> Thanks,
> Richard.
>
>> Thanks,
>> Kugan
>>>
>>>> I also added
>>>> index variable to the macro so that there want be any conflicts if the
>>>> index variable "i" (or whatever) is also defined in the loop.
>>>>
>>>> Bootstrapped and regression tested on x86_64-linux-gnu with no new
>>>> regressions. Is this OK for trunk?
>>>>
>>>> Thanks,
>>>> Kugan
>>>>
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2016-09-06  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>>>
>>>>     * tree-ssanames.h (FOR_EACH_SSA_NAME): New.
>>>>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>>>     FOR_EACH_SSA_NAME to iterate over SSA variables.
>>>>     (pass_expand::execute): Likewise.
>>>>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>>>     * tree-cfg.c (dump_function_to_file): Likewise.
>>>>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>>>     (update_ssa): Likewise.
>>>>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>>>>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>>>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>>>     (create_outofssa_var_map): Likewise.
>>>>     (coalesce_ssa_name): Likewise.
>>>>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>>>     * tree-ssa-pre.c (compute_avail): Likewise.
>>>>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>>>     (scc_vn_restore_ssa_info): Likewise.
>>>>     (free_scc_vn): Likwise.
>>>>     (run_scc_vn): Likewise.
>>>>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>>>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.
>>>>     * tree-ssa-copy.c (fini_copy_prop): Likewise.
>>>>     * tree-ssa.c (verify_ssa): Likewise.
>>>>
>>>>>
>>>>> Thanks,
>>>>> Richard.
>>>>>
>>>>>
>>>>>> Thanks,
>>>>>> Kugan
>>>>>>
>>>>>>
>>>>>> gcc/ChangeLog:
>>>>>>
>>>>>> 2016-09-05  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>>>>>
>>>>>>     * tree-ssanames.h (ssa_iterator::ssa_iterator): New.
>>>>>>     (ssa_iterator::get): Likewise.
>>>>>>     (ssa_iterator::next): Likewise.
>>>>>>     (FOR_EACH_SSAVAR): Likewise.
>>>>>>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>>>>>     FOR_EACH_SSAVAR to iterate over SSA variables.
>>>>>>     (pass_expand::execute): Likewise.
>>>>>>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>>>>>     * tree-cfg.c (dump_function_to_file): Likewise.
>>>>>>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>>>>>     (update_ssa): Likewise.
>>>>>>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>>>>>>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>>>>>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>>>>>     (create_outofssa_var_map): Likewise.
>>>>>>     (coalesce_ssa_name): Likewise.
>>>>>>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>>>>>     * tree-ssa-pre.c (compute_avail): Likewise.
>>>>>>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>>>>>     (scc_vn_restore_ssa_info): Likewise.
>>>>>>     (free_scc_vn): Likwise.
>>>>>>     (run_scc_vn): Likewise.
>>>>>>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>>>>>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.

[-- Attachment #2: ssa_iterator.txt --]
[-- Type: text/plain, Size: 17198 bytes --]

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 130a16b..dfa301d 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -815,16 +815,15 @@ update_alias_info_with_stack_vars (void)
   if (decls_to_partitions)
     {
       unsigned i;
+      tree name;
       hash_set<bitmap> visited;
       bitmap temp = BITMAP_ALLOC (&stack_var_bitmap_obstack);
 
-      for (i = 1; i < num_ssa_names; i++)
+      FOR_EACH_SSA_NAME (i, name, cfun)
 	{
-	  tree name = ssa_name (i);
 	  struct ptr_info_def *pi;
 
-	  if (name
-	      && POINTER_TYPE_P (TREE_TYPE (name))
+	  if (POINTER_TYPE_P (TREE_TYPE (name))
 	      && ((pi = SSA_NAME_PTR_INFO (name)) != NULL))
 	    add_partitioned_vars_to_ptset (&pi->pt, decls_to_partitions,
 					   &visited, temp);
@@ -6270,16 +6269,15 @@ pass_expand::execute (function *fun)
 
   /* Now propagate the RTL assignment of each partition to the
      underlying var of each SSA_NAME.  */
-  for (i = 1; i < num_ssa_names; i++)
-    {
-      tree name = ssa_name (i);
+  tree name;
 
-      if (!name
-	  /* We might have generated new SSA names in
-	     update_alias_info_with_stack_vars.  They will have a NULL
-	     defining statements, and won't be part of the partitioning,
-	     so ignore those.  */
-	  || !SSA_NAME_DEF_STMT (name))
+  FOR_EACH_SSA_NAME (i, name, cfun)
+    {
+      /* We might have generated new SSA names in
+	 update_alias_info_with_stack_vars.  They will have a NULL
+	 defining statements, and won't be part of the partitioning,
+	 so ignore those.  */
+      if (!SSA_NAME_DEF_STMT (name))
 	continue;
 
       adjust_one_expanded_partition_var (name);
@@ -6288,17 +6286,15 @@ pass_expand::execute (function *fun)
   /* Clean up RTL of variables that straddle across multiple
      partitions, and check that the rtl of any PARM_DECLs that are not
      cleaned up is that of their default defs.  */
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, name, cfun)
     {
-      tree name = ssa_name (i);
       int part;
 
-      if (!name
-	  /* We might have generated new SSA names in
-	     update_alias_info_with_stack_vars.  They will have a NULL
-	     defining statements, and won't be part of the partitioning,
-	     so ignore those.  */
-	  || !SSA_NAME_DEF_STMT (name))
+      /* We might have generated new SSA names in
+	 update_alias_info_with_stack_vars.  They will have a NULL
+	 defining statements, and won't be part of the partitioning,
+	 so ignore those.  */
+      if (!SSA_NAME_DEF_STMT (name))
 	continue;
       part = var_to_partition (SA.map, name);
       if (part == NO_PARTITION)
diff --git a/gcc/omp-simd-clone.c b/gcc/omp-simd-clone.c
index c418440c..df140d4 100644
--- a/gcc/omp-simd-clone.c
+++ b/gcc/omp-simd-clone.c
@@ -931,11 +931,11 @@ ipa_simd_modify_function_body (struct cgraph_node *node,
     }
 
   l = adjustments.length ();
-  for (i = 1; i < num_ssa_names; i++)
+  tree name;
+
+  FOR_EACH_SSA_NAME (i, name, cfun)
     {
-      tree name = ssa_name (i);
-      if (name
-	  && SSA_NAME_VAR (name)
+      if (SSA_NAME_VAR (name)
 	  && TREE_CODE (SSA_NAME_VAR (name)) == PARM_DECL)
 	{
 	  for (j = 0; j < l; j++)
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 57c8410..badbd96 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -7581,11 +7581,13 @@ dump_function_to_file (tree fndecl, FILE *file, int flags)
 
 	    any_var = true;
 	  }
+
+      tree name;
+
       if (gimple_in_ssa_p (cfun))
-	for (ix = 1; ix < num_ssa_names; ++ix)
+	FOR_EACH_SSA_NAME (ix, name, cfun)
 	  {
-	    tree name = ssa_name (ix);
-	    if (name && !SSA_NAME_VAR (name))
+	    if (!SSA_NAME_VAR (name))
 	      {
 		fprintf (file, "  ");
 		print_generic_expr (file, TREE_TYPE (name), flags);
diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
index ceafa68..a4ff608 100644
--- a/gcc/tree-into-ssa.c
+++ b/gcc/tree-into-ssa.c
@@ -2341,7 +2341,6 @@ pass_build_ssa::execute (function *fun)
 {
   bitmap_head *dfs;
   basic_block bb;
-  unsigned i;
 
   /* Initialize operand data structures.  */
   init_ssa_operands (fun);
@@ -2385,13 +2384,14 @@ pass_build_ssa::execute (function *fun)
   /* Try to get rid of all gimplifier generated temporaries by making
      its SSA names anonymous.  This way we can garbage collect them
      all after removing unused locals which we do in our TODO.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  unsigned i;
+  tree name;
+
+  FOR_EACH_SSA_NAME (i, name, cfun)
     {
-      tree decl, name = ssa_name (i);
-      if (!name
-	  || SSA_NAME_IS_DEFAULT_DEF (name))
+      if (SSA_NAME_IS_DEFAULT_DEF (name))
 	continue;
-      decl = SSA_NAME_VAR (name);
+      tree decl = SSA_NAME_VAR (name);
       if (decl
 	  && TREE_CODE (decl) == VAR_DECL
 	  && !VAR_DECL_IS_VIRTUAL_OPERAND (decl)
@@ -3283,12 +3283,12 @@ update_ssa (unsigned update_flags)
 	 placement heuristics.  */
       prepare_block_for_update (start_bb, insert_phi_p);
 
+      tree name;
+
       if (flag_checking)
-	for (i = 1; i < num_ssa_names; ++i)
+	FOR_EACH_SSA_NAME (i, name, cfun)
 	  {
-	    tree name = ssa_name (i);
-	    if (!name
-		|| virtual_operand_p (name))
+	    if (virtual_operand_p (name))
 	      continue;
 
 	    /* For all but virtual operands, which do not have SSA names
diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 8051a66..30de461 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -450,6 +450,7 @@ void
 dump_alias_info (FILE *file)
 {
   unsigned i;
+  tree ptr;
   const char *funcname
     = lang_hooks.decl_printable_name (current_function_decl, 2);
   tree var;
@@ -471,13 +472,11 @@ dump_alias_info (FILE *file)
 
   fprintf (file, "\n\nFlow-insensitive points-to information\n\n");
 
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, ptr, cfun)
     {
-      tree ptr = ssa_name (i);
       struct ptr_info_def *pi;
 
-      if (ptr == NULL_TREE
-	  || !POINTER_TYPE_P (TREE_TYPE (ptr))
+      if (!POINTER_TYPE_P (TREE_TYPE (ptr))
 	  || SSA_NAME_IN_FREE_LIST (ptr))
 	continue;
 
diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c
index 9871304..d5a0560 100644
--- a/gcc/tree-ssa-ccp.c
+++ b/gcc/tree-ssa-ccp.c
@@ -898,24 +898,23 @@ ccp_finalize (bool nonzero_p)
 {
   bool something_changed;
   unsigned i;
+  tree name;
 
   do_dbg_cnt ();
 
   /* Derive alignment and misalignment information from partially
      constant pointers in the lattice or nonzero bits from partially
      constant integers.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  FOR_EACH_SSA_NAME (i, name, cfun)
     {
-      tree name = ssa_name (i);
       ccp_prop_value_t *val;
       unsigned int tem, align;
 
-      if (!name
-	  || (!POINTER_TYPE_P (TREE_TYPE (name))
-	      && (!INTEGRAL_TYPE_P (TREE_TYPE (name))
-		  /* Don't record nonzero bits before IPA to avoid
-		     using too much memory.  */
-		  || !nonzero_p)))
+      if (!POINTER_TYPE_P (TREE_TYPE (name))
+	  && (!INTEGRAL_TYPE_P (TREE_TYPE (name))
+	      /* Don't record nonzero bits before IPA to avoid
+		 using too much memory.  */
+	      || !nonzero_p))
 	continue;
 
       val = get_value (name);
diff --git a/gcc/tree-ssa-coalesce.c b/gcc/tree-ssa-coalesce.c
index 34c3fa1..01f6c5f 100644
--- a/gcc/tree-ssa-coalesce.c
+++ b/gcc/tree-ssa-coalesce.c
@@ -955,12 +955,11 @@ build_ssa_conflict_graph (tree_live_info_p liveinfo)
       if (bb == entry)
 	{
 	  unsigned i;
-	  for (i = 1; i < num_ssa_names; i++)
-	    {
-	      tree var = ssa_name (i);
+	  tree var;
 
-	      if (!var
-		  || !SSA_NAME_IS_DEFAULT_DEF (var)
+	  FOR_EACH_SSA_NAME (i, var, cfun)
+	    {
+	      if (!SSA_NAME_IS_DEFAULT_DEF (var)
 		  || !SSA_NAME_VAR (var)
 		  || VAR_P (SSA_NAME_VAR (var)))
 		continue;
@@ -1261,10 +1260,9 @@ create_outofssa_var_map (coalesce_list *cl, bitmap used_in_copy)
   /* Now process result decls and live on entry variables for entry into
      the coalesce list.  */
   first = NULL_TREE;
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, var, cfun)
     {
-      var = ssa_name (i);
-      if (var != NULL_TREE && !virtual_operand_p (var))
+      if (!virtual_operand_p (var))
         {
 	  coalesce_with_default (var, cl, used_in_copy);
 
@@ -1806,6 +1804,7 @@ coalesce_ssa_name (void)
   bitmap used_in_copies = BITMAP_ALLOC (NULL);
   var_map map;
   unsigned int i;
+  tree a;
 
   cl = create_coalesce_list ();
   map = create_outofssa_var_map (cl, used_in_copies);
@@ -1817,12 +1816,9 @@ coalesce_ssa_name (void)
     {
       hash_table<ssa_name_var_hash> ssa_name_hash (10);
 
-      for (i = 1; i < num_ssa_names; i++)
+      FOR_EACH_SSA_NAME (i, a, cfun)
 	{
-	  tree a = ssa_name (i);
-
-	  if (a
-	      && SSA_NAME_VAR (a)
+	  if (SSA_NAME_VAR (a)
 	      && !DECL_IGNORED_P (SSA_NAME_VAR (a))
 	      && (!has_zero_uses (a) || !SSA_NAME_IS_DEFAULT_DEF (a)
 		  || !VAR_P (SSA_NAME_VAR (a))))
diff --git a/gcc/tree-ssa-copy.c b/gcc/tree-ssa-copy.c
index 3d17926..fcf4fa9 100644
--- a/gcc/tree-ssa-copy.c
+++ b/gcc/tree-ssa-copy.c
@@ -503,14 +503,13 @@ static bool
 fini_copy_prop (void)
 {
   unsigned i;
+  tree var;
 
   /* Set the final copy-of value for each variable by traversing the
      copy-of chains.  */
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, var, cfun)
     {
-      tree var = ssa_name (i);
-      if (!var
-	  || !copy_of[i].value
+      if (!copy_of[i].value
 	  || copy_of[i].value == var)
 	continue;
 
diff --git a/gcc/tree-ssa-operands.c b/gcc/tree-ssa-operands.c
index eccea2f..a5c3546 100644
--- a/gcc/tree-ssa-operands.c
+++ b/gcc/tree-ssa-operands.c
@@ -1247,11 +1247,8 @@ dump_immediate_uses (FILE *file)
   unsigned int x;
 
   fprintf (file, "Immediate_uses: \n\n");
-  for (x = 1; x < num_ssa_names; x++)
+  FOR_EACH_SSA_NAME (x, var, cfun)
     {
-      var = ssa_name (x);
-      if (!var)
-        continue;
       dump_immediate_uses_for (file, var);
     }
 }
diff --git a/gcc/tree-ssa-pre.c b/gcc/tree-ssa-pre.c
index fdb1c2c..67a0513 100644
--- a/gcc/tree-ssa-pre.c
+++ b/gcc/tree-ssa-pre.c
@@ -3670,15 +3670,14 @@ compute_avail (void)
   basic_block *worklist;
   size_t sp = 0;
   unsigned i;
+  tree name;
 
   /* We pretend that default definitions are defined in the entry block.
      This includes function arguments and the static chain decl.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  FOR_EACH_SSA_NAME (i, name, cfun)
     {
-      tree name = ssa_name (i);
       pre_expr e;
-      if (!name
-	  || !SSA_NAME_IS_DEFAULT_DEF (name)
+      if (!SSA_NAME_IS_DEFAULT_DEF (name)
 	  || has_zero_uses (name)
 	  || virtual_operand_p (name))
 	continue;
diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c
index 21b3d56..e120b4f 100644
--- a/gcc/tree-ssa-sccvn.c
+++ b/gcc/tree-ssa-sccvn.c
@@ -4290,7 +4290,6 @@ free_vn_table (vn_tables_t table)
 static void
 init_scc_vn (void)
 {
-  size_t i;
   int j;
   int *rpo_numbers_temp;
 
@@ -4339,12 +4338,11 @@ init_scc_vn (void)
 
   /* Create the VN_INFO structures, and initialize value numbers to
      TOP or VARYING for parameters.  */
-  for (i = 1; i < num_ssa_names; i++)
-    {
-      tree name = ssa_name (i);
-      if (!name)
-	continue;
+  size_t i;
+  tree name;
 
+  FOR_EACH_SSA_NAME (i, name, cfun)
+    {
       VN_INFO_GET (name)->valnum = VN_TOP;
       VN_INFO (name)->needs_insertion = false;
       VN_INFO (name)->expr = NULL;
@@ -4402,11 +4400,12 @@ init_scc_vn (void)
 void
 scc_vn_restore_ssa_info (void)
 {
-  for (unsigned i = 0; i < num_ssa_names; i++)
+  unsigned i;
+  tree name;
+
+  FOR_EACH_SSA_NAME (i, name, cfun)
     {
-      tree name = ssa_name (i);
-      if (name
-	  && has_VN_INFO (name))
+      if (has_VN_INFO (name))
 	{
 	  if (VN_INFO (name)->needs_insertion)
 	    ;
@@ -4428,6 +4427,7 @@ void
 free_scc_vn (void)
 {
   size_t i;
+  tree name;
 
   delete constant_to_value_id;
   constant_to_value_id = NULL;
@@ -4436,11 +4436,9 @@ free_scc_vn (void)
   shared_lookup_references.release ();
   XDELETEVEC (rpo_numbers);
 
-  for (i = 0; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, name, cfun)
     {
-      tree name = ssa_name (i);
-      if (name
-	  && has_VN_INFO (name)
+      if (has_VN_INFO (name)
 	  && VN_INFO (name)->needs_insertion)
 	release_ssa_name (name);
     }
@@ -4797,13 +4795,11 @@ run_scc_vn (vn_lookup_kind default_vn_walk_kind_)
 
   /* Initialize the value ids and prune out remaining VN_TOPs
      from dead code.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  tree name;
+
+  FOR_EACH_SSA_NAME (i, name, cfun)
     {
-      tree name = ssa_name (i);
-      vn_ssa_aux_t info;
-      if (!name)
-	continue;
-      info = VN_INFO (name);
+      vn_ssa_aux_t info = VN_INFO (name);
       if (!info->visited)
 	info->valnum = name;
       if (info->valnum == name
@@ -4814,13 +4810,9 @@ run_scc_vn (vn_lookup_kind default_vn_walk_kind_)
     }
 
   /* Propagate.  */
-  for (i = 1; i < num_ssa_names; ++i)
+  FOR_EACH_SSA_NAME (i, name, cfun)
     {
-      tree name = ssa_name (i);
-      vn_ssa_aux_t info;
-      if (!name)
-	continue;
-      info = VN_INFO (name);
+      vn_ssa_aux_t info = VN_INFO (name);
       if (TREE_CODE (info->valnum) == SSA_NAME
 	  && info->valnum != name
 	  && info->value_id != VN_INFO (info->valnum)->value_id)
@@ -4832,11 +4824,9 @@ run_scc_vn (vn_lookup_kind default_vn_walk_kind_)
   if (dump_file && (dump_flags & TDF_DETAILS))
     {
       fprintf (dump_file, "Value numbers:\n");
-      for (i = 0; i < num_ssa_names; i++)
+      FOR_EACH_SSA_NAME (i, name, cfun)
 	{
-	  tree name = ssa_name (i);
-	  if (name
-	      && VN_INFO (name)->visited
+	  if (VN_INFO (name)->visited
 	      && SSA_VAL (name) != name)
 	    {
 	      print_generic_expr (dump_file, name, 0);
diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index fd96c3a..cbf509b 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -7028,7 +7028,6 @@ static void
 compute_points_to_sets (void)
 {
   basic_block bb;
-  unsigned i;
   varinfo_t vi;
 
   timevar_push (TV_TREE_PTA);
@@ -7077,11 +7076,12 @@ compute_points_to_sets (void)
   cfun->gimple_df->escaped.escaped = 0;
 
   /* Compute the points-to sets for pointer SSA_NAMEs.  */
-  for (i = 0; i < num_ssa_names; ++i)
+  unsigned i;
+  tree ptr;
+
+  FOR_EACH_SSA_NAME (i, ptr, cfun)
     {
-      tree ptr = ssa_name (i);
-      if (ptr
-	  && POINTER_TYPE_P (TREE_TYPE (ptr)))
+      if (POINTER_TYPE_P (TREE_TYPE (ptr)))
 	find_what_p_points_to (cfun->decl, ptr);
     }
 
diff --git a/gcc/tree-ssa-ter.c b/gcc/tree-ssa-ter.c
index 2a772b2..c7d8b7e 100644
--- a/gcc/tree-ssa-ter.c
+++ b/gcc/tree-ssa-ter.c
@@ -185,8 +185,6 @@ extern void debug_ter (FILE *, temp_expr_table *);
 static temp_expr_table *
 new_temp_expr_table (var_map map)
 {
-  unsigned x;
-
   temp_expr_table *t = XNEW (struct temp_expr_table);
   t->map = map;
 
@@ -201,12 +199,13 @@ new_temp_expr_table (var_map map)
 
   t->replaceable_expressions = NULL;
   t->num_in_part = XCNEWVEC (int, num_var_partitions (map));
-  for (x = 1; x < num_ssa_names; x++)
+
+  unsigned x;
+  tree name;
+
+  FOR_EACH_SSA_NAME (x, name, cfun)
     {
       int p;
-      tree name = ssa_name (x);
-      if (!name)
-        continue;
       p = var_to_partition (map, name);
       if (p != NO_PARTITION)
         t->num_in_part[p]++;
diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c
index aae383d..d442a5f 100644
--- a/gcc/tree-ssa.c
+++ b/gcc/tree-ssa.c
@@ -1005,7 +1005,6 @@ error:
 DEBUG_FUNCTION void
 verify_ssa (bool check_modified_stmt, bool check_ssa_operands)
 {
-  size_t i;
   basic_block bb;
   basic_block *definition_block = XCNEWVEC (basic_block, num_ssa_names);
   ssa_op_iter iter;
@@ -1018,24 +1017,23 @@ verify_ssa (bool check_modified_stmt, bool check_ssa_operands)
   timevar_push (TV_TREE_SSA_VERIFY);
 
   /* Keep track of SSA names present in the IL.  */
-  for (i = 1; i < num_ssa_names; i++)
+  size_t i;
+  tree name;
+
+  FOR_EACH_SSA_NAME (i, name, cfun)
     {
-      tree name = ssa_name (i);
-      if (name)
-	{
-	  gimple *stmt;
-	  TREE_VISITED (name) = 0;
+      gimple *stmt;
+      TREE_VISITED (name) = 0;
 
-	  verify_ssa_name (name, virtual_operand_p (name));
+      verify_ssa_name (name, virtual_operand_p (name));
 
-	  stmt = SSA_NAME_DEF_STMT (name);
-	  if (!gimple_nop_p (stmt))
-	    {
-	      basic_block bb = gimple_bb (stmt);
-	      if (verify_def (bb, definition_block,
-			      name, stmt, virtual_operand_p (name)))
-		goto err;
-	    }
+      stmt = SSA_NAME_DEF_STMT (name);
+      if (!gimple_nop_p (stmt))
+	{
+	  basic_block bb = gimple_bb (stmt);
+	  if (verify_def (bb, definition_block,
+			  name, stmt, virtual_operand_p (name)))
+	    goto err;
 	}
     }
 
diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
index 8e66ce6..4496e1d 100644
--- a/gcc/tree-ssanames.h
+++ b/gcc/tree-ssanames.h
@@ -62,6 +62,10 @@ struct GTY ((variable_size)) range_info_def {
 #define num_ssa_names (vec_safe_length (cfun->gimple_df->ssa_names))
 #define ssa_name(i) ((*cfun->gimple_df->ssa_names)[(i)])
 
+#define FOR_EACH_SSA_NAME(I, VAR, FN)					\
+  for (I = 1; SSANAMES (FN)->iterate (I, &VAR); ++I)			\
+    if (VAR)
+
 /* Sets the value range to SSA.  */
 extern void set_range_info (tree, enum value_range_type, const wide_int_ref &,
 			    const wide_int_ref &);

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][SSA] Iterator to visit SSA
  2016-09-08  5:57           ` Kugan Vivekanandarajah
@ 2016-09-14 10:21             ` Richard Biener
  0 siblings, 0 replies; 14+ messages in thread
From: Richard Biener @ 2016-09-14 10:21 UTC (permalink / raw)
  To: Kugan Vivekanandarajah; +Cc: gcc-patches

On Thu, Sep 8, 2016 at 5:29 AM, Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi Richard,
>
> On 7 September 2016 at 19:35, Richard Biener <richard.guenther@gmail.com> wrote:
>> On Wed, Sep 7, 2016 at 2:21 AM, Kugan Vivekanandarajah
>> <kugan.vivekanandarajah@linaro.org> wrote:
>>> Hi Richard,
>>>
>>> On 6 September 2016 at 19:08, Richard Biener <richard.guenther@gmail.com> wrote:
>>>> On Tue, Sep 6, 2016 at 2:24 AM, Kugan Vivekanandarajah
>>>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>>> Hi Richard,
>>>>>
>>>>> On 5 September 2016 at 17:57, Richard Biener <richard.guenther@gmail.com> wrote:
>>>>>> On Mon, Sep 5, 2016 at 7:26 AM, Kugan Vivekanandarajah
>>>>>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>>>>> Hi All,
>>>>>>>
>>>>>>> While looking at gcc source, I noticed that we are iterating over SSA
>>>>>>> variable from 0 to num_ssa_names in some cases and 1 to num_ssa_names
>>>>>>> in others. It seems that variable 0 is always NULL TREE.
>>>>>>
>>>>>> Yeah, that's artificial because we don't assign SSA name version 0 (for some
>>>>>> unknown reason).
>>>>>>
>>>>>>> But it can
>>>>>>> confuse people who are looking for the first time. Therefore It might
>>>>>>> be good to follow some consistent usage here.
>>>>>>>
>>>>>>> It might be also good to gave a FOR_EACH_SSAVAR iterator as we do in
>>>>>>> other case. Here is attempt to do this based on what is done in other
>>>>>>> places. Bootstrapped and regression tested on X86_64-linux-gnu with no
>>>>>>> new regressions. is this OK?
>>>>>>
>>>>>> First of all some bikeshedding - FOR_EACH_SSA_NAME would be better
>>>>>> as SSAVAR might be confused with iterating over SSA_NAME_VAR.
>>>>>>
>>>>>> Then, if you add an iterator why leave the name == NULL handling to consumers?
>>>>>> That looks odd.
>>>>>>
>>>>>> Then, SSA names are just in a vector thus why not re-use the vector iterator?
>>>>>>
>>>>>> That is,
>>>>>>
>>>>>> #define FOR_EACH_SSA_NAME (name) \
>>>>>>   for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i)
>>>>>>
>>>>>> would be equivalent to your patch?
>>>>>>
>>>>>> Please also don't add new iterators that implicitely use 'cfun' but always use
>>>>>> one that passes it as explicit arg.
>>>>>
>>>>> I think defining FOR_EACH_SSA_NAME with vector iterator is better. But
>>>>> we will not be able to skill NULL ssa_names with that.
>>>>
>>>> Why?  Can't you simply do
>>>>
>>>>   #define FOR_EACH_SSA_NAME (name) \
>>>>     for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i) \
>>>>        if (name)
>>>>
>>>> ?
>>>
>>> Indeed.  I missed the if at the end :(.  Here is an updated patch.
>>> Bootstrapped and regression tested on x86_64-linux-gnu with no new
>>> regressions.
>>
>> Quoting myself:
>>
>>> As I said
>>> I'd like 'cfun' to be explicit, thus
>>>
>>> #define FOR_EACH_SSA_NAME(I, VAR, FN) \
>>>  for (I = 1; SSANAMES (FN)->iterate ((I), &(VAR)); ++(I)) \
>>>   if (VAR)
>>
>> the patch doesn't seem to contain that FN part.  Please also put declarations
>> of 'name' you need to add right before the FOR_EACH_SSA_NAME rather
>> than far away.
>
> Please find the attached patch does this. Bootstrapped and regression
> tested on x86_64-linux-gnu with no new regressions.
>
> Is this OK?

Ok.

Thanks,
Richard.

> Thanks,
> Kugan
>
>>
>> Thanks,
>> Richard.
>>
>>> Thanks,
>>> Kugan
>>>>
>>>>> I also added
>>>>> index variable to the macro so that there want be any conflicts if the
>>>>> index variable "i" (or whatever) is also defined in the loop.
>>>>>
>>>>> Bootstrapped and regression tested on x86_64-linux-gnu with no new
>>>>> regressions. Is this OK for trunk?
>>>>>
>>>>> Thanks,
>>>>> Kugan
>>>>>
>>>>>
>>>>> gcc/ChangeLog:
>>>>>
>>>>> 2016-09-06  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>>>>
>>>>>     * tree-ssanames.h (FOR_EACH_SSA_NAME): New.
>>>>>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>>>>     FOR_EACH_SSA_NAME to iterate over SSA variables.
>>>>>     (pass_expand::execute): Likewise.
>>>>>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>>>>     * tree-cfg.c (dump_function_to_file): Likewise.
>>>>>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>>>>     (update_ssa): Likewise.
>>>>>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>>>>>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>>>>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>>>>     (create_outofssa_var_map): Likewise.
>>>>>     (coalesce_ssa_name): Likewise.
>>>>>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>>>>     * tree-ssa-pre.c (compute_avail): Likewise.
>>>>>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>>>>     (scc_vn_restore_ssa_info): Likewise.
>>>>>     (free_scc_vn): Likwise.
>>>>>     (run_scc_vn): Likewise.
>>>>>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>>>>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.
>>>>>     * tree-ssa-copy.c (fini_copy_prop): Likewise.
>>>>>     * tree-ssa.c (verify_ssa): Likewise.
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Richard.
>>>>>>
>>>>>>
>>>>>>> Thanks,
>>>>>>> Kugan
>>>>>>>
>>>>>>>
>>>>>>> gcc/ChangeLog:
>>>>>>>
>>>>>>> 2016-09-05  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>>>>>>
>>>>>>>     * tree-ssanames.h (ssa_iterator::ssa_iterator): New.
>>>>>>>     (ssa_iterator::get): Likewise.
>>>>>>>     (ssa_iterator::next): Likewise.
>>>>>>>     (FOR_EACH_SSAVAR): Likewise.
>>>>>>>     * cfgexpand.c (update_alias_info_with_stack_vars): Use
>>>>>>>     FOR_EACH_SSAVAR to iterate over SSA variables.
>>>>>>>     (pass_expand::execute): Likewise.
>>>>>>>     * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>>>>>>>     * tree-cfg.c (dump_function_to_file): Likewise.
>>>>>>>     * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>>>>>>>     (update_ssa): Likewise.
>>>>>>>     * tree-ssa-alias.c (dump_alias_info): Likewise.
>>>>>>>     * tree-ssa-ccp.c (ccp_finalize): Likewise.
>>>>>>>     * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>>>>>>>     (create_outofssa_var_map): Likewise.
>>>>>>>     (coalesce_ssa_name): Likewise.
>>>>>>>     * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>>>>>>>     * tree-ssa-pre.c (compute_avail): Likewise.
>>>>>>>     * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>>>>>>>     (scc_vn_restore_ssa_info): Likewise.
>>>>>>>     (free_scc_vn): Likwise.
>>>>>>>     (run_scc_vn): Likewise.
>>>>>>>     * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>>>>>>>     * tree-ssa-ter.c (new_temp_expr_table): Likewise.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-09-14 10:16 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-05  5:39 [RFC][SSA] Iterator to visit SSA Kugan Vivekanandarajah
2016-09-05  7:58 ` Richard Biener
2016-09-06  6:00   ` Kugan Vivekanandarajah
2016-09-06  9:08     ` Richard Biener
2016-09-06  9:58       ` Kugan Vivekanandarajah
2016-09-06 10:07         ` Richard Biener
2016-09-06 10:07           ` Kugan Vivekanandarajah
2016-09-06 10:41             ` Richard Biener
2016-09-07  4:46       ` Kugan Vivekanandarajah
2016-09-07  8:03         ` Dominik Inführ
2016-09-07 11:37         ` Richard Biener
2016-09-08  5:57           ` Kugan Vivekanandarajah
2016-09-14 10:21             ` Richard Biener
2016-09-06 21:58   ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).