Patchwork Ping: RFA: Improve doloop_begin support

login
register
mail settings
Submitter Joern Rennecke
Date Oct. 16, 2012, 2:30 p.m.
Message ID <20121016103011.8w1hwfa73ms0c8sw-nzlynne@webmail.spamcop.net>
Download mbox | patch
Permalink /patch/191805/
State New
Headers show

Comments

Joern Rennecke - Oct. 16, 2012, 2:30 p.m.
Quoting Zdenek Dvorak <rakdver@iuuk.mff.cuni.cz>:

> no -- you should also test that latch contains no active insns.    
> I.e., factorize
> out whatever forwarder_block_p does except for the test   
> "(dest->loop_father->header == dest)" test,

Like this?

         * basic-block.h (forwarder_block_p_1): Declare.
         * cfgrtl.c (orwarder_block_p_1): New function, factored out of ...
         (orwarder_block_p): ... here.
Zdenek Dvorak - Oct. 16, 2012, 2:33 p.m.
> Quoting Zdenek Dvorak <rakdver@iuuk.mff.cuni.cz>:
> 
> >no -- you should also test that latch contains no active insns.
> >I.e., factorize
> >out whatever forwarder_block_p does except for the test
> >"(dest->loop_father->header == dest)" test,
> 
> Like this?
> 
>         * basic-block.h (forwarder_block_p_1): Declare.
>         * cfgrtl.c (orwarder_block_p_1): New function, factored out of ...
>         (orwarder_block_p): ... here.

yes (except maybe giving it some more descriptive name than forwarder_block_p_1,
say "contains_no_active_insn_p" or something),

Zdenek
Richard Guenther - Oct. 16, 2012, 2:44 p.m.
On Tue, Oct 16, 2012 at 4:30 PM, Joern Rennecke
<joern.rennecke@embecosm.com> wrote:
> Quoting Zdenek Dvorak <rakdver@iuuk.mff.cuni.cz>:
>
>> no -- you should also test that latch contains no active insns.   I.e.,
>> factorize
>> out whatever forwarder_block_p does except for the test
>> "(dest->loop_father->header == dest)" test,
>
>
> Like this?

ISTR the tree level has a predicate like can_remove_forwarder_block
and forwarder_block_p.  I suppose on the RTL level they are merged
for cfgcleanup.

>         * basic-block.h (forwarder_block_p_1): Declare.
>         * cfgrtl.c (orwarder_block_p_1): New function, factored out of ...
>         (orwarder_block_p): ... here.
>
>
>
>
>
>
> --- cfgrtl.c-old        2012-10-16 15:21:06.025532573 +0100
> +++ cfgrtl.c    2012-10-16 15:23:23.135529867 +0100
> @@ -541,10 +541,9 @@ flow_active_insn_p (const_rtx insn)
>
>  /* Return true if the block has no effect and only forwards control flow to
>     its single destination.  */
> -/* FIXME: Make this a cfg hook.  */
>
>  bool
> -forwarder_block_p (const_basic_block bb)
> +forwarder_block_p_1 (const_basic_block bb)
>  {
>    rtx insn;
>
> @@ -552,6 +551,24 @@ forwarder_block_p (const_basic_block bb)
>        || !single_succ_p (bb))
>      return false;
>
> +  for (insn = BB_HEAD (bb); insn != BB_END (bb); insn = NEXT_INSN (insn))
> +    if (INSN_P (insn) && flow_active_insn_p (insn))
> +      return false;
> +
> +  return (!INSN_P (insn)
> +         || (JUMP_P (insn) && simplejump_p (insn))
> +         || !flow_active_insn_p (insn));
> +}
> +
> +/* Likewise, but protect loop latches, headers and preheaders.  */
> +/* FIXME: Make this a cfg hook.  */
> +
> +bool
> +forwarder_block_p (const_basic_block bb)
> +{
> +  if (!forwarder_block_p_1 (bb))
> +    return false;
> +
>    /* Protect loop latches, headers and preheaders.  */
>    if (current_loops)
>      {
> @@ -563,13 +580,7 @@ forwarder_block_p (const_basic_block bb)
>         return false;
>      }
>
> -  for (insn = BB_HEAD (bb); insn != BB_END (bb); insn = NEXT_INSN (insn))
> -    if (INSN_P (insn) && flow_active_insn_p (insn))
> -      return false;
> -
> -  return (!INSN_P (insn)
> -         || (JUMP_P (insn) && simplejump_p (insn))
> -         || !flow_active_insn_p (insn));
> +  return true;
>  }
>
>  /* Return nonzero if we can reach target from src by falling through.  */
>

Patch

--- cfgrtl.c-old	2012-10-16 15:21:06.025532573 +0100
+++ cfgrtl.c	2012-10-16 15:23:23.135529867 +0100
@@ -541,10 +541,9 @@  flow_active_insn_p (const_rtx insn)
 
 /* Return true if the block has no effect and only forwards control flow to
    its single destination.  */
-/* FIXME: Make this a cfg hook.  */
 
 bool
-forwarder_block_p (const_basic_block bb)
+forwarder_block_p_1 (const_basic_block bb)
 {
   rtx insn;
 
@@ -552,6 +551,24 @@  forwarder_block_p (const_basic_block bb)
       || !single_succ_p (bb))
     return false;
 
+  for (insn = BB_HEAD (bb); insn != BB_END (bb); insn = NEXT_INSN (insn))
+    if (INSN_P (insn) && flow_active_insn_p (insn))
+      return false;
+
+  return (!INSN_P (insn)
+	  || (JUMP_P (insn) && simplejump_p (insn))
+	  || !flow_active_insn_p (insn));
+}
+
+/* Likewise, but protect loop latches, headers and preheaders.  */
+/* FIXME: Make this a cfg hook.  */
+
+bool
+forwarder_block_p (const_basic_block bb)
+{
+  if (!forwarder_block_p_1 (bb))
+    return false;
+
   /* Protect loop latches, headers and preheaders.  */
   if (current_loops)
     {
@@ -563,13 +580,7 @@  forwarder_block_p (const_basic_block bb)
 	return false;
     }
 
-  for (insn = BB_HEAD (bb); insn != BB_END (bb); insn = NEXT_INSN (insn))
-    if (INSN_P (insn) && flow_active_insn_p (insn))
-      return false;
-
-  return (!INSN_P (insn)
-	  || (JUMP_P (insn) && simplejump_p (insn))
-	  || !flow_active_insn_p (insn));
+  return true;
 }
 
 /* Return nonzero if we can reach target from src by falling through.  */