Re: [Patch, bfin/c6x] Fix ICE for backends that rely on reorder_loops.

[Bernd Schmidt <bernds at codesourcery dot com>]

On 01/05/2014 05:10 PM, Teresa Johnson wrote:
> On Sun, Jan 5, 2014 at 3:39 AM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> > I have a different patch which I'll submit next week after some more
> > testing. The assert in cfgrtl is unnecessarily broad and really only needs
> > to trigger if -freorder-blocks-and-partition; there's nothing wrong with
> > entering cfglayout after normal bb-reorder.
>
> Currently -freorder-blocks-and-partition is the default for x86. I
> assume that hw-doloop is not enabled for any i386 targets, which is
> why we haven't seen this?

Precisely.

> And will this mean that -freorder-blocks-and-partition cannot be used
> for the targets that use hw-doloop? If so, should
> -freorder-blocks-and-partition be prevented with a warning for those
> targets?

If someone explicitly chooses that option we can turn off the reordering 
in hw-doloop. That should happen sufficiently rarely that it isn't a 
problem. That's what the patch below does - bootstraped on x86_64-linux, 
tested there and with bfin-elf. Ok?

> > I've also tested that Blackfin still benefits from the hw-doloop reordering
> > code and generates more hardware loops if it's enabled. So we want to be
> > able to run it at -O2.
>
> I looked at hw-doloop briefly and since it seems to be doing some
> manual bb reordering I guess it can't simply be moved before bbro. It
> seems like a better long-term solution would be to make bbro
> hw-doloop-aware as Felix suggested earlier.

Maybe. It could be argued that the code in hw-doloop is relevant only 
for a small class of targets so it should only be enabled for them. In 
any case, that's not stage 3 material and two ports are broken...


Bernd

	* cfgrtl.c (cfg_layout_initialize): Weaken assert to only trigger if
	flag_reorder_blocks_and_partition.
	* hw-doloop.c (reorg_loops): Avoid reordering if that flag is set.

diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
index 1a63249..2d75845 100644
--- a/gcc/cfgrtl.c
+++ b/gcc/cfgrtl.c
@@ -4222,14 +4222,14 @@ cfg_layout_initialize (unsigned int flags)
   rtx x;
   basic_block bb;
 
-  /* Once bb reordering is complete, cfg layout mode should not be re-entered.
-     Entering cfg layout mode will perform optimizations on the cfg that
-     could affect the bb layout negatively or even require fixups. An
-     example of the latter is if edge forwarding performed when optimizing
-     the cfg layout required moving a block from the hot to the cold section
-     under -freorder-blocks-and-partition. This would create an illegal
-     partitioning unless some manual fixup was performed.  */
-  gcc_assert (!crtl->bb_reorder_complete);
+  /* Once bb partitioning is complete, cfg layout mode should not be
+     re-entered.  Entering cfg layout mode may require fixups.  As an
+     example, if edge forwarding performed when optimizing the cfg
+     layout required moving a block from the hot to the cold
+     section. This would create an illegal partitioning unless some
+     manual fixup was performed.  */
+  gcc_assert (!(crtl->bb_reorder_complete
+		&& flag_reorder_blocks_and_partition));
 
   initialize_original_copy_tables ();
 
diff --git a/gcc/hw-doloop.c b/gcc/hw-doloop.c
index b6184a2..9c2c874 100644
--- a/gcc/hw-doloop.c
+++ b/gcc/hw-doloop.c
@@ -636,7 +636,9 @@ reorg_loops (bool do_reorder, struct hw_doloop_hooks *hooks)
 
   loops = discover_loops (&loop_stack, hooks);
 
-  if (do_reorder)
+  /* We can't enter cfglayout mode anymore if basic block partitioning
+     already happened.  */
+  if (do_reorder && !flag_reorder_blocks_and_partition)
     {
       reorder_loops (loops);
       free_loops (loops);