On 25/09/14 09:52, Zhenqiang Chen wrote:
-----Original Message-----
From: Jiong Wang [mailto:jiong.w...@arm.com]
Sent: Thursday, September 25, 2014 2:13 AM
To: Jeff Law; Zhenqiang Chen
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH, 2/2] shrink wrap a function with a single loop: split
live_edge
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));
According to the algorithm, it is impossible to split one edge twice. It's
possible to split two different edges. But for such cases, the control flow is
too complex to perform shrink-wrapping.
Anyway, your patch improves the accuracy. You can replace the "gcc_assert" to "return"; or change
"split_p" to "splitted_edge" then you can check one edge is not splitted twice.
thanks for the explanation.
actually, the old "bitmap_copy (df_get_live_in (next_block), df_get_live_out (bb));" will
let any "dest" reg
in entry block alive in the new splitted block. If there is another block which
"dest" also set in live_in, then
dest alive in two blocks, then those code in "live_edge_for_reg" will always
return NULL, thus the old
inaccurate data flow will actually never make split two different edges
happen... thus assert never triggered.
as from the whole x86 boostrap, and regression test, only two cases trigger
split two different edges, I think it's
trival case, thus prefer to be conservative to keep the old logic, as suggested, just replace
"gcc_assert" into "return false".
or if we want to allow multi split, I think just remove the assert is OK, because
"EDGE_COUNT (next_block->preds) == 2"
will guarantee split one edge twice never happen.
new patch updated.
pass bootstrap and no regression, both check-gcc and check-g++, on the x86.
OK for trunk?
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".
diff --git a/gcc/shrink-wrap.c b/gcc/shrink-wrap.c
index af23f02..bd4813c 100644
--- a/gcc/shrink-wrap.c
+++ b/gcc/shrink-wrap.c
@@ -250,16 +250,21 @@ 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. */
- gcc_assert (!(*split_p));
+ if (*split_p)
+ return false;
+
*split_p = true;
}
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" } } */
