Re: [rfc] and [autovect patch] Vectorize reduction.

[Dorit Naishlos <DORIT at il dot ibm dot com>]

Diego Novillo <dnovillo@redhat.com> wrote on 23/02/2005 22:54:16:
> Richard Henderson wrote:
>
> >>(f) I'm using replace_immediate_uses to replaces uses of a_3 in
[Example1]
> >>with uses of a_5 [Example2]. Is there a better way around this? (is
there a
> >>way to replace the defining stmt of an ssa-name with a new defining
stmt
> >>(for the same ssa-name), removing the original defining stmt?
> >
> >
> > I'm not sure, but wouldn't it just be modifying SSA_NAME_DEF_STMT?
Diego?
> >
> In theory, yes.  IIRC, we don't cache the defining statement of SSA
> names anywhere, so things shouldn't break.  But this sounds hacky.
>
> Dorit, what are you trying to do?  Replace some SSA names with newly
> created names?

I have:

===>
loop_exit_bb:
      a_3 = phi <a_3>
somewhere:
      use <a_3>
      use <a_3>

and I create:

===>
loop_exit_bb:
      a_3 = phi <a_2>
+     a_5 = new_defining_stmt
somewhere:
      use <a_3>
      use <a_3>

and want to replace uses of a_3 with uses of a_5, for which I'm currently
using replace_immediate_uses, so the result is:

===>
loop_exit_bb:
      a_3 = phi <a_2>
+     a_5 = new_defining_stmt
somewhere:
!     use <a_5>
!     use <a_5>

I was trying to instead directly replace the definition of a_3 with a new
definition; i.e.:

===>
-     a_3 = phi <a_2>
      va_3 = phi <va_2>
+     a_3 = new_defining_stmt
      use <a_3>
      use <a_3>

The problem is I can't call remove_phi_node because it releases the
ssa_name defined by the phi_node. To work around this I created my own
remove_phi_node that doesn't release the ssa_name, and now I get the
desired result. Would it be acceptable to add this additional version of
remove_phi_node, as follows?:

Index: tree-phinodes.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-phinodes.c,v
retrieving revision 2.21.2.1
diff -c -3 -p -r2.21.2.1 tree-phinodes.c
*** tree-phinodes.c     14 Dec 2004 08:45:48 -0000      2.21.2.1
--- tree-phinodes.c     27 Feb 2005 15:10:50 -0000
*************** remove_phi_args (edge e)
*** 398,403 ****
--- 398,435 ----
      remove_phi_arg_num (phi, e->dest_idx);
  }

+
+ /* Like remove_phi_node but without releasing the ssa-name of the
+    phi-result.  */
+
+ void
+ remove_phi_node_keeping_name (tree phi, tree prev, basic_block bb)
+ {
+   if (prev)
+     {
+       /* Rewire the list if we are given a PREV pointer.  */
+       PHI_CHAIN (prev) = PHI_CHAIN (phi);
+       release_phi_node (phi);
+     }
+   else if (phi == phi_nodes (bb))
+     {
+       /* Update the list head if removing the first element.  */
+       bb_ann (bb)->phi_nodes = PHI_CHAIN (phi);
+       release_phi_node (phi);
+     }
+   else
+     {
+       /* Traverse the list looking for the node to remove.  */
+       tree prev, t;
+       prev = NULL_TREE;
+       for (t = phi_nodes (bb); t && t != phi; t = PHI_CHAIN (t))
+         prev = t;
+       if (t)
+         remove_phi_node_keeping_name (t, prev, bb);
+     }
+ }
+
+
  /* Remove PHI node PHI from basic block BB.  If PREV is non-NULL, it is
     used as the node immediately before PHI in the linked list.  */


(By the way, I was able to redefine a_3 without removing the phi_node that
defines it:

===>
      a_3 = phi <a_2>
      va_3 = phi <va_2>
+     a_3 = new_defining_stmt
      use <a_3>
      use <a_3>

and this did not upset the ssa verifiers, despite the two definitions of
a_3.
This is the actual code I generated:

  # vect_var_.19_38 = PHI <vect_var_.19_37(1)>;
  # udiff_18 = PHI <udiff_15(1)>;
<L2>:;
  vect_var_.21_39 =  REDUC_PLUS_EXPR < vect_var_.19_38 > ;
  stmp_var_.22_40 = BIT_FIELD_REF <vect_var_.21_39, 32, 0>;
  udiff_18 = stmp_var_.22_40;
  if (udiff_18 != 240) goto <L3>; else goto <L4>;

and somehow compilation passed successfully, although udiff_18 is defined
twice. The phi that defines it is dead, but is this an expected behavior?)

thanks,

dorit


> There is an update_ssa call in tree-cleanup-branch and a
> similar mechanism in ssaupdate-branch.
>
> Zdenek and I are working on an SSA update mechanism along those lines.
> (http://gcc.gnu.org/ml/gcc-patches/2005-02/msg00187.html)
>
>
> Diego.