diff mbox

[committed,gomp4] Rewrite virtuals into lcssa in transform_to_exit_first_loop_alt

Message ID 5582F88A.5030105@mentor.com
State New
Headers show

Commit Message

Tom de Vries June 18, 2015, 4:57 p.m. UTC 
On 18/06/15 13:38, Richard Biener wrote:
> On Thu, Jun 18, 2015 at 9:25 AM, Tom de Vries <Tom_deVries@mentor.com> wrote:
>> Hi,
>>
>> transform_to_exit_first_loop contains the following comment:
>> ...
>>    /* Make sure that we have phi nodes on exit for all loop header phis
>>       (create_parallel_loop requires that).  */
>> ...
>>
>> I ran into a problem where after transform_to_exit_first_loop_alt this
>> property does not hold for virtuals, and we run into trouble in
>> create_parallel_loop.
>>
>> The patch ensures that the property holds at the start of
>> transform_to_exit_first_loop_alt, which has as effect that:
>> - we simplify transform_to_exit_first_loop_alt a bit, and
>> - since the property is kept by transform_to_exit_first_loop_alt,
>>    the property will be valid after transform_to_exit_first_loop_alt.
>>
>> Bootstrapped and reg-tested on x86_64.
>>
>> Committed to gomp-4_0-branch.
>>
>> OK for trunk?
>
> I once made virtual-operands loop-closed

AFAICT, r190614, ...

> but then somehow I didn't get it
> correct (I suppose I ran into the trap of the existing machinery using
> update_ssa to update out-of-loop uses) and removed that capability again.
>

... and r195609. I guess not PR56150, but PR56113 is the related PR, see 
comment https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56113#c17 ?

AFAICT from that PR, the problem was compilation speed, not a 
correctness problem?

[ confusingly, the rewrite_into_loop_closed_ssa comment still claims to 
update virtuals:
...
    1) Updating it during unrolling/peeling/versioning is trivial, since
       we do not need to care about the uses outside of the loop.
       The same applies to virtual operands which are also rewritten into
       loop closed SSA form.  Note that virtual operands are always live
       until function exit.
...
]

> So instead of adding another limited machinery inside tree-parloops
> can you instead
> make it available from tree-ssa-loop-manip.c, eventually sharing some code?
>
> Other passes would benefit from this as well and it could save some compile-time
> as they wouldn't need to rewrite the virtual operands all the time.
>

Attached untested patch series contains these patches:
1. Move rewrite_virtuals_into_loop_closed_ssa to tree-ssa-loop-manip.c
2. Add bitmap_get_dominated_by
3. Add replace_uses_in_dominated_bbs
4. Add get_virtual_phi

The last three patches split up the implementation of 
rewrite_virtuals_into_loop_closed_ssa in smaller, hopefully reusable bits.

OK for gomp-4_0-branch and trunk if testing succeeds?

I'm not sure what should be the next step. If you can point me to a use 
site in a pass that could benefit from using 
rewrite_virtuals_into_loop_closed_ssa, then I can try to take it from there.

Thanks,
- Tom
diff mbox

Patch

[PATCH 4/4] Add get_virtual_phi

2015-06-18  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-loop-manip.c (get_virtual_phi): Factor out of ...
	(rewrite_virtuals_into_loop_closed_ssa): ... here.
---
 gcc/tree-ssa-loop-manip.c | 39 +++++++++++++++++++--------------------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/gcc/tree-ssa-loop-manip.c b/gcc/tree-ssa-loop-manip.c
index 0d2c972..44c14df 100644
--- a/gcc/tree-ssa-loop-manip.c
+++ b/gcc/tree-ssa-loop-manip.c
@@ -603,44 +603,43 @@  replace_uses_in_dominated_bbs (tree old_val, tree new_val, basic_block bb)
   BITMAP_FREE (dominated);
 }
 
-/* Ensure a virtual phi is present in the exit block, if LOOP contains a vdef.
-   In other words, ensure loop-closed ssa normal form for virtuals.  */
+/* Return the virtual phi in BB.  */
 
-void
-rewrite_virtuals_into_loop_closed_ssa (struct loop *loop)
+static gphi *
+get_virtual_phi (basic_block bb)
 {
   gphi *phi;
-  edge exit = single_dom_exit (loop);
 
   phi = NULL;
-  for (gphi_iterator gsi = gsi_start_phis (loop->header);
+  for (gphi_iterator gsi = gsi_start_phis (bb);
        !gsi_end_p (gsi);
        gsi_next (&gsi))
     {
       if (virtual_operand_p (PHI_RESULT (gsi.phi ())))
 	{
-	  phi = gsi.phi ();
-	  break;
+	  return gsi.phi ();
 	}
     }
 
+  return NULL;
+}
+
+/* Ensure a virtual phi is present in the exit block, if LOOP contains a vdef.
+   In other words, ensure loop-closed ssa normal form for virtuals.  */
+
+void
+rewrite_virtuals_into_loop_closed_ssa (struct loop *loop)
+{
+  gphi *phi;
+  edge exit = single_dom_exit (loop);
+
+  phi = get_virtual_phi (loop->header);
   if (phi == NULL)
     return;
 
   tree final_loop = PHI_ARG_DEF_FROM_EDGE (phi, single_succ_edge (loop->latch));
 
-  phi = NULL;
-  for (gphi_iterator gsi = gsi_start_phis (exit->dest);
-       !gsi_end_p (gsi);
-       gsi_next (&gsi))
-    {
-      if (virtual_operand_p (PHI_RESULT (gsi.phi ())))
-	{
-	  phi = gsi.phi ();
-	  break;
-	}
-    }
-
+  phi = get_virtual_phi (exit->dest);
   if (phi != NULL)
     {
       tree final_exit = PHI_ARG_DEF_FROM_EDGE (phi, exit);
-- 
1.9.1