Re: [PATCH, 2/2] shrink wrap a function with a single loop: split live_edge

[Jiong Wang <jiong dot wang at arm dot com>]

On 22/09/14 18:51, Jeff Law wrote:
> On 09/22/14 04:24, Jiong Wang wrote:
> > > Great.  Can you send an updated patchkit for review.
> > patch attached.
> >
> > please review, thanks.
> >
> > gcc/ * shrink-wrap.c (move_insn_for_shrink_wrap): Initialize the
> > live-in of new created BB as the intersection of live-in from
> > "old_dest" and live-out from "bb".
> Looks good.  However, before committing we need a couple things.
>
> 1. Bootstrap & regression test this variant of the patch.  I know you
> tested an earlier one, but please test this one just to be sure.
>
> 2. Testcase.  I think you could test for either the reduction in the
> live-in set of the newly created block or that you're shrink wrapping
> one or more functions you didn't previously shrink-wrap.  I think it's
> fine if this test is target specific.

 bootstrap ok based on revision 215515.

 while the x86 regression result is interesting. there is no regression on check-g++, while there is four regression on check-gcc:

FAIL: gcc.dg/tree-ssa/loadpre10.c (internal compiler error)
FAIL: gcc.dg/tree-ssa/loadpre10.c (test for excess errors)
FAIL: gcc.dg/tree-ssa/pr21417.c (internal compiler error)
FAIL: gcc.dg/tree-ssa/pr21417.c (test for excess errors)

  this is caused by our improving the accuracy of live-in for new created basic block. Now we will split
  more than one edge for the above two testcase. thus trigger the following assert in move_insn_for_shrink_wrap:

      /* We should not split more than once for a function.  */
      gcc_assert (!(*split_p));

 take pr21417.c for example, after the patch, two edges will be split,

before this patch
=================
.L2:
        movq    %rdi, %rax
        cmpl    $142, (%rdi)
        jne     .L13
.L4:
        all insns sinked here  <-- the only split
        ...
        ...

        popq    %rbx
        popq    %rbp
.L13:
        ret

after this patch
================
.L2:

        cmpl    $142, (%rdi)
        jne     .L13
.L4:
        part of insns sinked into here  <-- first split
        ....
        ....

        popq    %rbx
        popq    %rbp
        ret

.L13:	
        movq    %rdi, %rax  <-- second split and one instruction moved here
        ret

I don't know why there is a assert to prevent multi split.

after I remove that assert, pass bootstrap and no regression.

and for pr21417.c, the multi split more cause one extra "ret" instruction, but the performance is better, because there
is no need to execute "movq    %rdi, %rax" if we go down to L4.

any comments?

BTW: I updated the patch with testcase which could not be shrink-wrapped before this patch.

thanks.

-- Jiong

> Jeff
diff --git a/gcc/shrink-wrap.c b/gcc/shrink-wrap.c
index fd24135..63deadf 100644
--- a/gcc/shrink-wrap.c
+++ b/gcc/shrink-wrap.c
@@ -217,12 +217,15 @@ move_insn_for_shrink_wrap (basic_block bb, rtx_insn *insn,
       if (!df_live)
 	return false;

+      basic_block old_dest = live_edge->dest;
       next_block = split_edge (live_edge);

       /* We create a new basic block.  Call df_grow_bb_info to make sure
 	 all data structures are allocated.  */
       df_grow_bb_info (df_live);
-      bitmap_copy (df_get_live_in (next_block), df_get_live_out (bb));
+
+      bitmap_and (df_get_live_in (next_block), df_get_live_out (bb),
+		  df_get_live_in (old_dest));
       df_set_bb_dirty (next_block);

       /* We should not split more than once for a function.  */
diff --git a/gcc/testsuite/gcc.target/i386/shrink_wrap_1.c b/gcc/testsuite/gcc.target/i386/shrink_wrap_1.c
new file mode 100644
index 0000000..47f2468
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/shrink_wrap_1.c
@@ -0,0 +1,48 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-rtl-pro_and_epilogue" } */
+
+enum machine_mode
+{
+  FAKE_0,
+  FAKE_1,
+  FAKE_2,
+  FAKE_3,
+  FAKE_4,
+  FAKE_5,
+  NUM_MACHINE_MODES,
+};
+
+typedef int *rtx;
+typedef long unsigned int size_t;
+extern unsigned char mode_size[NUM_MACHINE_MODES];
+
+extern rtx c_readstr (const char *, enum machine_mode);
+extern rtx convert_to_mode (enum machine_mode, rtx, int);
+extern rtx expand_mult (enum machine_mode, rtx, rtx, rtx, int);
+extern rtx force_reg (enum machine_mode, rtx);
+extern void *memset (void *__s, int __c, size_t __n);
+
+rtx
+builtin_memset_gen_str (void *data, long offset __attribute__ ((__unused__)),
+			enum machine_mode mode)
+{
+  rtx target, coeff;
+  size_t size;
+  char *p;
+
+  size = ((unsigned short) (__builtin_constant_p (mode)
+			    ? mode_size_inline (mode) : mode_size[mode]));
+  if (size == 1)
+    return (rtx) data;
+
+  p = ((char *) __builtin_alloca(sizeof (char) * (size)));
+  memset (p, 1, size);
+  coeff = c_readstr (p, mode);
+
+  target = convert_to_mode (mode, (rtx) data, 1);
+  target = expand_mult (mode, target, coeff, (rtx) 0, 1);
+  return force_reg (mode, target);
+}
+
+/* { dg-final { scan-rtl-dump "Performing shrink-wrapping" "pro_and_epilogue"  } } */
+/* { dg-final { cleanup-rtl-dump "pro_and_epilogue" } } */