This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Do not set BB_SUPERBLOCK in loop-unroll.c:split_edge_and_insert


On 3/4/07, Zdenek Dvorak <rakdver@atrey.karlin.mff.cuni.cz> wrote:
> Therefore, Zdenek, can you live with a patch that removes the line
> that sets BB_SUPERBLOCK in split_edge_and_insert, and adds a comment
> with your analysis of why it might be needed but we don't know for
> sure?

Yes, I am now persuaded that this might be the best option.

OK, here's my proposed patch. Bootstrapped and tested and all that on i686. I'm going to install this later this week unless I hear objections. Thanks,

Gr.
Steven
	* loop-unroll.c (split_edge_and_insert): Don't set BB_SUPERBLOCK
	on the new basic block.  Add a lengthy comment explaining why we
	thought this was necessary.
	* cfglayout.c (cfg_layout_finalize): Don't break superblocks.

Index: loop-unroll.c
===================================================================
--- loop-unroll.c	(revision 122599)
+++ loop-unroll.c	(working copy)
@@ -879,7 +879,37 @@ split_edge_and_insert (edge e, rtx insns
     return NULL;
   bb = split_edge (e); 
   emit_insn_after (insns, BB_END (bb));
-  bb->flags |= BB_SUPERBLOCK;
+
+  /* ??? We used to assume that INSNS can contain control flow insns, and
+     that we had to try to find sub basic blocks in BB to maintain a valid
+     CFG.  For this purpose we used to set the BB_SUPERBLOCK flag on BB
+     and call break_superblocks when going out of cfglayout mode.  But it
+     turns out that this never happens; and that if it had ever happens,
+     the verify_flow_info call in loop_optimizer_finalize would fail.
+
+     There are two reasons why we expected we could have control flow insns
+     in INSNS.  The first is when a comparison has to be done in parts, and
+     the second is when the number of iterations is computed for loops with
+     the number of iterations known at runtime.  In both cases, test cases
+     to get control flow in INSNS appear to be impossible to construct:
+
+      * If do_compare_rtx_and_jump needs several branches to do comparison
+	in a mode that needs comparison by parts, we cannot analyze the
+	number of iterations of the loop, and we never get to unrolling it.
+
+      * The code in expand_divmod that was suspected to cause creation of
+	branching code seems to be only accessed for signed division.  The
+	divisions used by # of iterations analysis are always unsigned.
+	Problems might arise on architectures that emits branching code
+	for some operations that may appear in the unroller (especially
+	for division), but we have no such architectures.
+
+     Considering all this, it was decided that we should for now assume
+     that INSNS can in theory contain control flow insns, but in practice
+     it never does.  So we don't handle the theoretical case, and should
+     a real failure ever show up, we have a pretty good clue for how to
+     fix it.  */
+
   return bb;
 }
 
Index: cfglayout.c
===================================================================
--- cfglayout.c	(revision 122599)
+++ cfglayout.c	(working copy)
@@ -1149,8 +1149,6 @@ cfg_layout_finalize (void)
     bb->il.rtl->visited = 0;
   }
 
-  break_superblocks ();
-
 #ifdef ENABLE_CHECKING
   verify_flow_info ();
 #endif

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]