Re: [PATCH] Fix thread_prologue_and_epilogue_insns (PR middle-end/79499)

[Richard Biener <rguenther at suse dot de>]

On August 1, 2017 10:35:43 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>In this function we insert 0-2 prologue sequences (which can sometimes
>contain jumps and other insns that need to end basic blocks) on edges,
>then commit edge insertions and then finally attempts to find out
>into which basic blocks the sequences were inserted and calls
>find_many_sub_basic_blocks on those blocks.
>
>As the testcase shows, the guess is sometimes wrong,
>commit_one_edge_insertion doesn't always insert on edge->dest,
>sometimes
>it inserts on edge->src and in other cases splits edge and inserts into
>a
>new basic block.
>
>What is certain though is that all the real insns in the sequences are
>added
>somewhere together.  So, what this patch does is finds the first
>NONDEBUG_INSN_P, (if there are none, then we shouldn't need
>find_many_sub_basic_blocks), and checks in which block it is using
>BLOCK_FOR_INSN.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
>trunk/7.2?

OK.

Thanks,
Richard.

>2017-08-01  Jakub Jelinek  <jakub@redhat.com>
>
>	PR middle-end/79499
>	* function.c (thread_prologue_and_epilogue_insns): Determine blocks
>	for find_many_sub_basic_blocks bitmap by looking up BLOCK_FOR_INSN
>	of first NONDEBUG_INSN_P in each of the split_prologue_seq and
>	prologue_seq sequences - if any.
>
>--- gcc/function.c.jj	2017-07-26 13:37:45.000000000 +0200
>+++ gcc/function.c	2017-08-01 14:10:26.909836163 +0200
>@@ -6048,20 +6048,42 @@ thread_prologue_and_epilogue_insns (void
> 
>   if (split_prologue_seq || prologue_seq)
>     {
>+      rtx_insn *split_prologue_insn = split_prologue_seq;
>       if (split_prologue_seq)
>-	insert_insn_on_edge (split_prologue_seq, orig_entry_edge);
>+	{
>+	  while (split_prologue_insn && !NONDEBUG_INSN_P
>(split_prologue_insn))
>+	    split_prologue_insn = NEXT_INSN (split_prologue_insn);
>+	  insert_insn_on_edge (split_prologue_seq, orig_entry_edge);
>+	}
> 
>+      rtx_insn *prologue_insn = prologue_seq;
>       if (prologue_seq)
>-	insert_insn_on_edge (prologue_seq, entry_edge);
>+	{
>+	  while (prologue_insn && !NONDEBUG_INSN_P (prologue_insn))
>+	    prologue_insn = NEXT_INSN (prologue_insn);
>+	  insert_insn_on_edge (prologue_seq, entry_edge);
>+	}
> 
>       commit_edge_insertions ();
> 
>       /* Look for basic blocks within the prologue insns.  */
>-      auto_sbitmap blocks (last_basic_block_for_fn (cfun));
>-      bitmap_clear (blocks);
>-      bitmap_set_bit (blocks, entry_edge->dest->index);
>-      bitmap_set_bit (blocks, orig_entry_edge->dest->index);
>-      find_many_sub_basic_blocks (blocks);
>+      if (split_prologue_insn
>+	  && BLOCK_FOR_INSN (split_prologue_insn) == NULL)
>+	split_prologue_insn = NULL;
>+      if (prologue_insn
>+	  && BLOCK_FOR_INSN (prologue_insn) == NULL)
>+	prologue_insn = NULL;
>+      if (split_prologue_insn || prologue_insn)
>+	{
>+	  auto_sbitmap blocks (last_basic_block_for_fn (cfun));
>+	  bitmap_clear (blocks);
>+	  if (split_prologue_insn)
>+	    bitmap_set_bit (blocks,
>+			    BLOCK_FOR_INSN (split_prologue_insn)->index);
>+	  if (prologue_insn)
>+	    bitmap_set_bit (blocks, BLOCK_FOR_INSN (prologue_insn)->index);
>+	  find_many_sub_basic_blocks (blocks);
>+	}
>     }
> 
>   default_rtl_profile ();
>--- gcc/testsuite/gcc.dg/pr79499.c.jj	2017-08-01 13:57:36.120485689
>+0200
>+++ gcc/testsuite/gcc.dg/pr79499.c	2017-08-01 13:57:26.000000000 +0200
>@@ -0,0 +1,13 @@
>+/* PR middle-end/79499 */
>+/* { dg-do compile { target split_stack } } */
>+/* { dg-options "-O2 -fsplit-stack -fno-omit-frame-pointer" } */
>+
>+struct S { struct S *a, *b; };
>+
>+void
>+foo (struct S *x)
>+{
>+  do
>+    x->b = x->a;
>+  while (x = x->a);
>+}
>
>	Jakub