* Prevent tree-ssa-dce.c from deleting stores at -Og
@ 2019-07-07 11:59 Richard Sandiford
2019-07-07 20:13 ` Jeff Law
0 siblings, 1 reply; 8+ messages in thread
From: Richard Sandiford @ 2019-07-07 11:59 UTC (permalink / raw)
To: gcc-patches
DCE tries to delete dead stores to local data and also tries to insert
debug binds for simple cases:
/* If this is a store into a variable that is being optimized away,
add a debug bind stmt if possible. */
if (MAY_HAVE_DEBUG_BIND_STMTS
&& gimple_assign_single_p (stmt)
&& is_gimple_val (gimple_assign_rhs1 (stmt)))
{
tree lhs = gimple_assign_lhs (stmt);
if ((VAR_P (lhs) || TREE_CODE (lhs) == PARM_DECL)
&& !DECL_IGNORED_P (lhs)
&& is_gimple_reg_type (TREE_TYPE (lhs))
&& !is_global_var (lhs)
&& !DECL_HAS_VALUE_EXPR_P (lhs))
{
tree rhs = gimple_assign_rhs1 (stmt);
gdebug *note
= gimple_build_debug_bind (lhs, unshare_expr (rhs), stmt);
gsi_insert_after (i, note, GSI_SAME_STMT);
}
}
But this doesn't help for things like "print *ptr" when ptr points
to the local variable (tests Og-dce-1.c and Og-dce-2.c). It also tends
to make the *live* -- and thus useful -- values optimised out, because
we can't yet switch back to tracking the memory location as it evolves
over time (test Og-dce-3.c).
So for -Og I think it'd be better not to delete any stmts with
vdefs for now. This also means that we can avoid the potentially
expensive vop walks (which already have a cut-off, but still).
The patch also fixes the Og failures in gcc.dg/guality/pr54970.c
(PR 86638).
Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
Richard
2019-07-07 Richard Sandiford <richard.sandiford@arm.com>
gcc/
PR debug/86638
* tree-ssa-dce.c (keep_all_vdefs_p): New function.
(mark_stmt_if_obviously_necessary): Mark all stmts with vdefs as
necessary if keep_all_vdefs_p is true.
(mark_aliased_reaching_defs_necessary): Add a gcc_checking_assert
that keep_all_vdefs_p is false.
(mark_all_reaching_defs_necessary): Likewise.
(propagate_necessity): Skip the vuse scan if keep_all_vdefs_p is true.
gcc/testsuite/
* c-c++-common/guality/Og-dce-1.c: New test.
* c-c++-common/guality/Og-dce-2.c: Likewise.
* c-c++-common/guality/Og-dce-3.c: Likewise.
Index: gcc/tree-ssa-dce.c
===================================================================
--- gcc/tree-ssa-dce.c 2019-06-29 17:20:49.000000000 +0100
+++ gcc/tree-ssa-dce.c 2019-07-07 10:34:47.616717593 +0100
@@ -115,6 +115,14 @@ #define STMT_NECESSARY GF_PLF_1
static int *bb_postorder;
+/* True if we should treat any stmt with a vdef as necessary. */
+
+static inline bool
+keep_all_vdefs_p ()
+{
+ return optimize_debug;
+}
+
/* If STMT is not already marked necessary, mark it, and add it to the
worklist if ADD_TO_WORKLIST is true. */
@@ -311,6 +319,12 @@ mark_stmt_if_obviously_necessary (gimple
return;
}
+ if (gimple_vdef (stmt) && keep_all_vdefs_p ())
+ {
+ mark_stmt_necessary (stmt, true);
+ return;
+ }
+
return;
}
@@ -526,6 +540,9 @@ mark_aliased_reaching_defs_necessary_1 (
static void
mark_aliased_reaching_defs_necessary (gimple *stmt, tree ref)
{
+ /* Should have been caught before calling this function. */
+ gcc_checking_assert (!keep_all_vdefs_p ());
+
unsigned int chain;
ao_ref refd;
gcc_assert (!chain_ovfl);
@@ -599,6 +616,8 @@ mark_all_reaching_defs_necessary_1 (ao_r
static void
mark_all_reaching_defs_necessary (gimple *stmt)
{
+ /* Should have been caught before calling this function. */
+ gcc_checking_assert (!keep_all_vdefs_p ());
walk_aliased_vdefs (NULL, gimple_vuse (stmt),
mark_all_reaching_defs_necessary_1, NULL, &visited);
}
@@ -798,6 +817,10 @@ propagate_necessity (bool aggressive)
if (!use)
continue;
+ /* No need to search for vdefs if we intrinsicly keep them all. */
+ if (keep_all_vdefs_p ())
+ continue;
+
/* If we dropped to simple mode make all immediately
reachable definitions necessary. */
if (chain_ovfl)
Index: gcc/testsuite/c-c++-common/guality/Og-dce-1.c
===================================================================
--- /dev/null 2019-06-14 15:59:19.298479944 +0100
+++ gcc/testsuite/c-c++-common/guality/Og-dce-1.c 2019-07-07 10:34:47.612717625 +0100
@@ -0,0 +1,14 @@
+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+int *__attribute__((noipa)) consume (int *ptr) { return ptr; }
+
+int
+main (void)
+{
+ int x;
+ int *volatile ptr = consume (&x);
+ x = 0;
+ x = 1; /* { dg-final { gdb-test . "*ptr" "0" } } */
+ return 0; /* { dg-final { gdb-test . "*ptr" "1" } } */
+}
Index: gcc/testsuite/c-c++-common/guality/Og-dce-2.c
===================================================================
--- /dev/null 2019-06-14 15:59:19.298479944 +0100
+++ gcc/testsuite/c-c++-common/guality/Og-dce-2.c 2019-07-07 10:34:47.612717625 +0100
@@ -0,0 +1,19 @@
+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+struct s { int a, b, c, d; };
+
+struct s gs1 = { 1, 2, 3, 4 };
+struct s gs2 = { 5, 6, 7, 8 };
+
+struct s *__attribute__((noipa)) consume (struct s *ptr) { return ptr; }
+
+int
+main (void)
+{
+ struct s x;
+ struct s *volatile ptr = consume (&x);
+ x = gs1;
+ x = gs2; /* { dg-final { gdb-test . "ptr->a" "1" } } */
+ return 0; /* { dg-final { gdb-test . "ptr->a" "5" } } */
+}
Index: gcc/testsuite/c-c++-common/guality/Og-dce-3.c
===================================================================
--- /dev/null 2019-06-14 15:59:19.298479944 +0100
+++ gcc/testsuite/c-c++-common/guality/Og-dce-3.c 2019-07-07 10:34:47.612717625 +0100
@@ -0,0 +1,36 @@
+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+struct s { int a, b, c, d; };
+
+struct s gs1 = { 1, 2, 3, 4 };
+struct s gs2 = { 5, 6, 7, 8 };
+
+volatile int amount = 10;
+
+int __attribute__((noipa))
+foo (int count)
+{
+ struct s x;
+ x = gs1;
+ x = gs2;
+ for (int i = 0; i < count; ++i)
+ {
+ x.a += amount;
+ x.b += amount;
+ x.c += amount;
+ x.d += amount;
+ }
+ return x.a + x.b + x.c + x.d;
+ /* { dg-final { gdb-test .-1 "x.a" "105" } } */
+ /* { dg-final { gdb-test .-2 "x.b" "106" } } */
+ /* { dg-final { gdb-test .-3 "x.c" "107" } } */
+ /* { dg-final { gdb-test .-4 "x.d" "108" } } */
+}
+
+int
+main (void)
+{
+ foo (10);
+ return 0;
+}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Prevent tree-ssa-dce.c from deleting stores at -Og
2019-07-07 11:59 Prevent tree-ssa-dce.c from deleting stores at -Og Richard Sandiford
@ 2019-07-07 20:13 ` Jeff Law
2019-07-08 11:44 ` Richard Biener
2019-07-29 9:11 ` Richard Sandiford
0 siblings, 2 replies; 8+ messages in thread
From: Jeff Law @ 2019-07-07 20:13 UTC (permalink / raw)
To: gcc-patches, richard.sandiford
On 7/7/19 3:45 AM, Richard Sandiford wrote:
> DCE tries to delete dead stores to local data and also tries to insert
> debug binds for simple cases:
>
> /* If this is a store into a variable that is being optimized away,
> add a debug bind stmt if possible. */
> if (MAY_HAVE_DEBUG_BIND_STMTS
> && gimple_assign_single_p (stmt)
> && is_gimple_val (gimple_assign_rhs1 (stmt)))
> {
> tree lhs = gimple_assign_lhs (stmt);
> if ((VAR_P (lhs) || TREE_CODE (lhs) == PARM_DECL)
> && !DECL_IGNORED_P (lhs)
> && is_gimple_reg_type (TREE_TYPE (lhs))
> && !is_global_var (lhs)
> && !DECL_HAS_VALUE_EXPR_P (lhs))
> {
> tree rhs = gimple_assign_rhs1 (stmt);
> gdebug *note
> = gimple_build_debug_bind (lhs, unshare_expr (rhs), stmt);
> gsi_insert_after (i, note, GSI_SAME_STMT);
> }
> }
>
> But this doesn't help for things like "print *ptr" when ptr points
> to the local variable (tests Og-dce-1.c and Og-dce-2.c). It also tends
> to make the *live* -- and thus useful -- values optimised out, because
> we can't yet switch back to tracking the memory location as it evolves
> over time (test Og-dce-3.c).
>
> So for -Og I think it'd be better not to delete any stmts with
> vdefs for now. This also means that we can avoid the potentially
> expensive vop walks (which already have a cut-off, but still).
>
> The patch also fixes the Og failures in gcc.dg/guality/pr54970.c
> (PR 86638).
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
>
> Richard
>
>
> 2019-07-07 Richard Sandiford <richard.sandiford@arm.com>
>
> gcc/
> PR debug/86638
> * tree-ssa-dce.c (keep_all_vdefs_p): New function.
> (mark_stmt_if_obviously_necessary): Mark all stmts with vdefs as
> necessary if keep_all_vdefs_p is true.
> (mark_aliased_reaching_defs_necessary): Add a gcc_checking_assert
> that keep_all_vdefs_p is false.
> (mark_all_reaching_defs_necessary): Likewise.
> (propagate_necessity): Skip the vuse scan if keep_all_vdefs_p is true.
>
> gcc/testsuite/
> * c-c++-common/guality/Og-dce-1.c: New test.
> * c-c++-common/guality/Og-dce-2.c: Likewise.
> * c-c++-common/guality/Og-dce-3.c: Likewise.
OK
jeff
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Prevent tree-ssa-dce.c from deleting stores at -Og
2019-07-07 20:13 ` Jeff Law
@ 2019-07-08 11:44 ` Richard Biener
2019-07-08 14:59 ` Richard Sandiford
2019-07-08 15:06 ` Jeff Law
2019-07-29 9:11 ` Richard Sandiford
1 sibling, 2 replies; 8+ messages in thread
From: Richard Biener @ 2019-07-08 11:44 UTC (permalink / raw)
To: Jeff Law; +Cc: GCC Patches, Richard Sandiford
On Sun, Jul 7, 2019 at 9:07 PM Jeff Law <law@redhat.com> wrote:
>
> On 7/7/19 3:45 AM, Richard Sandiford wrote:
> > DCE tries to delete dead stores to local data and also tries to insert
> > debug binds for simple cases:
> >
> > /* If this is a store into a variable that is being optimized away,
> > add a debug bind stmt if possible. */
> > if (MAY_HAVE_DEBUG_BIND_STMTS
> > && gimple_assign_single_p (stmt)
> > && is_gimple_val (gimple_assign_rhs1 (stmt)))
> > {
> > tree lhs = gimple_assign_lhs (stmt);
> > if ((VAR_P (lhs) || TREE_CODE (lhs) == PARM_DECL)
> > && !DECL_IGNORED_P (lhs)
> > && is_gimple_reg_type (TREE_TYPE (lhs))
> > && !is_global_var (lhs)
> > && !DECL_HAS_VALUE_EXPR_P (lhs))
> > {
> > tree rhs = gimple_assign_rhs1 (stmt);
> > gdebug *note
> > = gimple_build_debug_bind (lhs, unshare_expr (rhs), stmt);
> > gsi_insert_after (i, note, GSI_SAME_STMT);
> > }
> > }
> >
> > But this doesn't help for things like "print *ptr" when ptr points
> > to the local variable (tests Og-dce-1.c and Og-dce-2.c). It also tends
> > to make the *live* -- and thus useful -- values optimised out, because
> > we can't yet switch back to tracking the memory location as it evolves
> > over time (test Og-dce-3.c).
> >
> > So for -Og I think it'd be better not to delete any stmts with
> > vdefs for now. This also means that we can avoid the potentially
> > expensive vop walks (which already have a cut-off, but still).
> >
> > The patch also fixes the Og failures in gcc.dg/guality/pr54970.c
> > (PR 86638).
> >
> > Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
> >
> > Richard
> >
> >
> > 2019-07-07 Richard Sandiford <richard.sandiford@arm.com>
> >
> > gcc/
> > PR debug/86638
> > * tree-ssa-dce.c (keep_all_vdefs_p): New function.
> > (mark_stmt_if_obviously_necessary): Mark all stmts with vdefs as
> > necessary if keep_all_vdefs_p is true.
> > (mark_aliased_reaching_defs_necessary): Add a gcc_checking_assert
> > that keep_all_vdefs_p is false.
> > (mark_all_reaching_defs_necessary): Likewise.
> > (propagate_necessity): Skip the vuse scan if keep_all_vdefs_p is true.
> >
> > gcc/testsuite/
> > * c-c++-common/guality/Og-dce-1.c: New test.
> > * c-c++-common/guality/Og-dce-2.c: Likewise.
> > * c-c++-common/guality/Og-dce-3.c: Likewise.
> OK
I wonder how code size (and compile-time) is affected by the DSE/DCE patch?
Say just look at -Og built cc1? Can you restrict the keep-all-vdefs to
user variables (and measure the difference this makes)? Again I wonder if
this makes C++ with -Og impractical runtime-wise.
Richard.
> jeff
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Prevent tree-ssa-dce.c from deleting stores at -Og
2019-07-08 11:44 ` Richard Biener
@ 2019-07-08 14:59 ` Richard Sandiford
2019-07-08 16:32 ` Richard Biener
2019-07-08 15:06 ` Jeff Law
1 sibling, 1 reply; 8+ messages in thread
From: Richard Sandiford @ 2019-07-08 14:59 UTC (permalink / raw)
To: Richard Biener; +Cc: Jeff Law, GCC Patches
Richard Biener <richard.guenther@gmail.com> writes:
> On Sun, Jul 7, 2019 at 9:07 PM Jeff Law <law@redhat.com> wrote:
>>
>> On 7/7/19 3:45 AM, Richard Sandiford wrote:
>> > DCE tries to delete dead stores to local data and also tries to insert
>> > debug binds for simple cases:
>> >
>> > /* If this is a store into a variable that is being optimized away,
>> > add a debug bind stmt if possible. */
>> > if (MAY_HAVE_DEBUG_BIND_STMTS
>> > && gimple_assign_single_p (stmt)
>> > && is_gimple_val (gimple_assign_rhs1 (stmt)))
>> > {
>> > tree lhs = gimple_assign_lhs (stmt);
>> > if ((VAR_P (lhs) || TREE_CODE (lhs) == PARM_DECL)
>> > && !DECL_IGNORED_P (lhs)
>> > && is_gimple_reg_type (TREE_TYPE (lhs))
>> > && !is_global_var (lhs)
>> > && !DECL_HAS_VALUE_EXPR_P (lhs))
>> > {
>> > tree rhs = gimple_assign_rhs1 (stmt);
>> > gdebug *note
>> > = gimple_build_debug_bind (lhs, unshare_expr (rhs), stmt);
>> > gsi_insert_after (i, note, GSI_SAME_STMT);
>> > }
>> > }
>> >
>> > But this doesn't help for things like "print *ptr" when ptr points
>> > to the local variable (tests Og-dce-1.c and Og-dce-2.c). It also tends
>> > to make the *live* -- and thus useful -- values optimised out, because
>> > we can't yet switch back to tracking the memory location as it evolves
>> > over time (test Og-dce-3.c).
>> >
>> > So for -Og I think it'd be better not to delete any stmts with
>> > vdefs for now. This also means that we can avoid the potentially
>> > expensive vop walks (which already have a cut-off, but still).
>> >
>> > The patch also fixes the Og failures in gcc.dg/guality/pr54970.c
>> > (PR 86638).
>> >
>> > Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
>> >
>> > Richard
>> >
>> >
>> > 2019-07-07 Richard Sandiford <richard.sandiford@arm.com>
>> >
>> > gcc/
>> > PR debug/86638
>> > * tree-ssa-dce.c (keep_all_vdefs_p): New function.
>> > (mark_stmt_if_obviously_necessary): Mark all stmts with vdefs as
>> > necessary if keep_all_vdefs_p is true.
>> > (mark_aliased_reaching_defs_necessary): Add a gcc_checking_assert
>> > that keep_all_vdefs_p is false.
>> > (mark_all_reaching_defs_necessary): Likewise.
>> > (propagate_necessity): Skip the vuse scan if keep_all_vdefs_p is true.
>> >
>> > gcc/testsuite/
>> > * c-c++-common/guality/Og-dce-1.c: New test.
>> > * c-c++-common/guality/Og-dce-2.c: Likewise.
>> > * c-c++-common/guality/Og-dce-3.c: Likewise.
>> OK
>
> I wonder how code size (and compile-time) is affected by the DSE/DCE patch?
> Say just look at -Og built cc1?
Overall I see a ~2.5% slowdown and a 4.7% increase in load size.
That comes almost entirely from the (RTL) DSE side; this patch
and gimple DSE part don't seem to make much difference.
If I keep the gimple passes as-is and just disable RTL DSE, the slowdown
is still ~2.5% and there's a 4.4% increase in load size.
These are all measuring cc1plus (built from post-patch sources)
and using -O2 -g tree-into-ssa.ii for the speed checks.
> Can you restrict the keep-all-vdefs to user variables (and measure the
> difference this makes)?
In order to avoid wrong debug for pointer dereferences, I think it would
have to be keep-all-vdefs for writes to either user variables or unknown
locations. But as above, I can't measure a significant difference with
the patch.
> Again I wonder if this makes C++ with -Og impractical runtime-wise.
Got a particular test in mind?
Richard
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Prevent tree-ssa-dce.c from deleting stores at -Og
2019-07-08 11:44 ` Richard Biener
2019-07-08 14:59 ` Richard Sandiford
@ 2019-07-08 15:06 ` Jeff Law
1 sibling, 0 replies; 8+ messages in thread
From: Jeff Law @ 2019-07-08 15:06 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, Richard Sandiford
On 7/8/19 5:34 AM, Richard Biener wrote:
> On Sun, Jul 7, 2019 at 9:07 PM Jeff Law <law@redhat.com> wrote:
>>
>> On 7/7/19 3:45 AM, Richard Sandiford wrote:
>>> DCE tries to delete dead stores to local data and also tries to insert
>>> debug binds for simple cases:
>>>
>>> /* If this is a store into a variable that is being optimized away,
>>> add a debug bind stmt if possible. */
>>> if (MAY_HAVE_DEBUG_BIND_STMTS
>>> && gimple_assign_single_p (stmt)
>>> && is_gimple_val (gimple_assign_rhs1 (stmt)))
>>> {
>>> tree lhs = gimple_assign_lhs (stmt);
>>> if ((VAR_P (lhs) || TREE_CODE (lhs) == PARM_DECL)
>>> && !DECL_IGNORED_P (lhs)
>>> && is_gimple_reg_type (TREE_TYPE (lhs))
>>> && !is_global_var (lhs)
>>> && !DECL_HAS_VALUE_EXPR_P (lhs))
>>> {
>>> tree rhs = gimple_assign_rhs1 (stmt);
>>> gdebug *note
>>> = gimple_build_debug_bind (lhs, unshare_expr (rhs), stmt);
>>> gsi_insert_after (i, note, GSI_SAME_STMT);
>>> }
>>> }
>>>
>>> But this doesn't help for things like "print *ptr" when ptr points
>>> to the local variable (tests Og-dce-1.c and Og-dce-2.c). It also tends
>>> to make the *live* -- and thus useful -- values optimised out, because
>>> we can't yet switch back to tracking the memory location as it evolves
>>> over time (test Og-dce-3.c).
>>>
>>> So for -Og I think it'd be better not to delete any stmts with
>>> vdefs for now. This also means that we can avoid the potentially
>>> expensive vop walks (which already have a cut-off, but still).
>>>
>>> The patch also fixes the Og failures in gcc.dg/guality/pr54970.c
>>> (PR 86638).
>>>
>>> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
>>>
>>> Richard
>>>
>>>
>>> 2019-07-07 Richard Sandiford <richard.sandiford@arm.com>
>>>
>>> gcc/
>>> PR debug/86638
>>> * tree-ssa-dce.c (keep_all_vdefs_p): New function.
>>> (mark_stmt_if_obviously_necessary): Mark all stmts with vdefs as
>>> necessary if keep_all_vdefs_p is true.
>>> (mark_aliased_reaching_defs_necessary): Add a gcc_checking_assert
>>> that keep_all_vdefs_p is false.
>>> (mark_all_reaching_defs_necessary): Likewise.
>>> (propagate_necessity): Skip the vuse scan if keep_all_vdefs_p is true.
>>>
>>> gcc/testsuite/
>>> * c-c++-common/guality/Og-dce-1.c: New test.
>>> * c-c++-common/guality/Og-dce-2.c: Likewise.
>>> * c-c++-common/guality/Og-dce-3.c: Likewise.
>> OK
>
> I wonder how code size (and compile-time) is affected by the DSE/DCE patch?
> Say just look at -Og built cc1? Can you restrict the keep-all-vdefs to
> user variables (and measure the difference this makes)? Again I wonder if
> this makes C++ with -Og impractical runtime-wise.I've never really seen DSE be terribly important for any codebase. It's
unfortunate, but that's my experience.
Jeff
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Prevent tree-ssa-dce.c from deleting stores at -Og
2019-07-08 14:59 ` Richard Sandiford
@ 2019-07-08 16:32 ` Richard Biener
2019-07-12 11:20 ` Richard Sandiford
0 siblings, 1 reply; 8+ messages in thread
From: Richard Biener @ 2019-07-08 16:32 UTC (permalink / raw)
To: Richard Biener, Jeff Law, GCC Patches, Richard Sandiford
On Mon, Jul 8, 2019 at 4:41 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Richard Biener <richard.guenther@gmail.com> writes:
> > On Sun, Jul 7, 2019 at 9:07 PM Jeff Law <law@redhat.com> wrote:
> >>
> >> On 7/7/19 3:45 AM, Richard Sandiford wrote:
> >> > DCE tries to delete dead stores to local data and also tries to insert
> >> > debug binds for simple cases:
> >> >
> >> > /* If this is a store into a variable that is being optimized away,
> >> > add a debug bind stmt if possible. */
> >> > if (MAY_HAVE_DEBUG_BIND_STMTS
> >> > && gimple_assign_single_p (stmt)
> >> > && is_gimple_val (gimple_assign_rhs1 (stmt)))
> >> > {
> >> > tree lhs = gimple_assign_lhs (stmt);
> >> > if ((VAR_P (lhs) || TREE_CODE (lhs) == PARM_DECL)
> >> > && !DECL_IGNORED_P (lhs)
> >> > && is_gimple_reg_type (TREE_TYPE (lhs))
> >> > && !is_global_var (lhs)
> >> > && !DECL_HAS_VALUE_EXPR_P (lhs))
> >> > {
> >> > tree rhs = gimple_assign_rhs1 (stmt);
> >> > gdebug *note
> >> > = gimple_build_debug_bind (lhs, unshare_expr (rhs), stmt);
> >> > gsi_insert_after (i, note, GSI_SAME_STMT);
> >> > }
> >> > }
> >> >
> >> > But this doesn't help for things like "print *ptr" when ptr points
> >> > to the local variable (tests Og-dce-1.c and Og-dce-2.c). It also tends
> >> > to make the *live* -- and thus useful -- values optimised out, because
> >> > we can't yet switch back to tracking the memory location as it evolves
> >> > over time (test Og-dce-3.c).
> >> >
> >> > So for -Og I think it'd be better not to delete any stmts with
> >> > vdefs for now. This also means that we can avoid the potentially
> >> > expensive vop walks (which already have a cut-off, but still).
> >> >
> >> > The patch also fixes the Og failures in gcc.dg/guality/pr54970.c
> >> > (PR 86638).
> >> >
> >> > Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
> >> >
> >> > Richard
> >> >
> >> >
> >> > 2019-07-07 Richard Sandiford <richard.sandiford@arm.com>
> >> >
> >> > gcc/
> >> > PR debug/86638
> >> > * tree-ssa-dce.c (keep_all_vdefs_p): New function.
> >> > (mark_stmt_if_obviously_necessary): Mark all stmts with vdefs as
> >> > necessary if keep_all_vdefs_p is true.
> >> > (mark_aliased_reaching_defs_necessary): Add a gcc_checking_assert
> >> > that keep_all_vdefs_p is false.
> >> > (mark_all_reaching_defs_necessary): Likewise.
> >> > (propagate_necessity): Skip the vuse scan if keep_all_vdefs_p is true.
> >> >
> >> > gcc/testsuite/
> >> > * c-c++-common/guality/Og-dce-1.c: New test.
> >> > * c-c++-common/guality/Og-dce-2.c: Likewise.
> >> > * c-c++-common/guality/Og-dce-3.c: Likewise.
> >> OK
> >
> > I wonder how code size (and compile-time) is affected by the DSE/DCE patch?
> > Say just look at -Og built cc1?
>
> Overall I see a ~2.5% slowdown and a 4.7% increase in load size.
> That comes almost entirely from the (RTL) DSE side; this patch
> and gimple DSE part don't seem to make much difference.
>
> If I keep the gimple passes as-is and just disable RTL DSE, the slowdown
> is still ~2.5% and there's a 4.4% increase in load size.
>
> These are all measuring cc1plus (built from post-patch sources)
> and using -O2 -g tree-into-ssa.ii for the speed checks.
>
> > Can you restrict the keep-all-vdefs to user variables (and measure the
> > difference this makes)?
>
> In order to avoid wrong debug for pointer dereferences, I think it would
> have to be keep-all-vdefs for writes to either user variables or unknown
> locations. But as above, I can't measure a significant difference with
> the patch.
>
> > Again I wonder if this makes C++ with -Og impractical runtime-wise.
>
> Got a particular test in mind?
Nothing specific - there are a few C/C++ benchmarks in SPEC and there's
also tramp3d-v4. I guess SRA is much more important for the abstraction
penalty than DSE - FRE should be able to remove the abstraction, just the
dead stores will remain (but they'd probably nicely execute out-of-order).
Anyway, the biggest runtime penalty from -Og is probably not running
any loop optimization (invariant motion mostly).
Richard.
>
> Richard
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Prevent tree-ssa-dce.c from deleting stores at -Og
2019-07-08 16:32 ` Richard Biener
@ 2019-07-12 11:20 ` Richard Sandiford
0 siblings, 0 replies; 8+ messages in thread
From: Richard Sandiford @ 2019-07-12 11:20 UTC (permalink / raw)
To: Richard Biener; +Cc: Jeff Law, GCC Patches
Richard Biener <richard.guenther@gmail.com> writes:
> On Mon, Jul 8, 2019 at 4:41 PM Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>>
>> Richard Biener <richard.guenther@gmail.com> writes:
>> > On Sun, Jul 7, 2019 at 9:07 PM Jeff Law <law@redhat.com> wrote:
>> >>
>> >> On 7/7/19 3:45 AM, Richard Sandiford wrote:
>> >> > DCE tries to delete dead stores to local data and also tries to insert
>> >> > debug binds for simple cases:
>> >> >
>> >> > /* If this is a store into a variable that is being optimized away,
>> >> > add a debug bind stmt if possible. */
>> >> > if (MAY_HAVE_DEBUG_BIND_STMTS
>> >> > && gimple_assign_single_p (stmt)
>> >> > && is_gimple_val (gimple_assign_rhs1 (stmt)))
>> >> > {
>> >> > tree lhs = gimple_assign_lhs (stmt);
>> >> > if ((VAR_P (lhs) || TREE_CODE (lhs) == PARM_DECL)
>> >> > && !DECL_IGNORED_P (lhs)
>> >> > && is_gimple_reg_type (TREE_TYPE (lhs))
>> >> > && !is_global_var (lhs)
>> >> > && !DECL_HAS_VALUE_EXPR_P (lhs))
>> >> > {
>> >> > tree rhs = gimple_assign_rhs1 (stmt);
>> >> > gdebug *note
>> >> > = gimple_build_debug_bind (lhs, unshare_expr (rhs), stmt);
>> >> > gsi_insert_after (i, note, GSI_SAME_STMT);
>> >> > }
>> >> > }
>> >> >
>> >> > But this doesn't help for things like "print *ptr" when ptr points
>> >> > to the local variable (tests Og-dce-1.c and Og-dce-2.c). It also tends
>> >> > to make the *live* -- and thus useful -- values optimised out, because
>> >> > we can't yet switch back to tracking the memory location as it evolves
>> >> > over time (test Og-dce-3.c).
>> >> >
>> >> > So for -Og I think it'd be better not to delete any stmts with
>> >> > vdefs for now. This also means that we can avoid the potentially
>> >> > expensive vop walks (which already have a cut-off, but still).
>> >> >
>> >> > The patch also fixes the Og failures in gcc.dg/guality/pr54970.c
>> >> > (PR 86638).
>> >> >
>> >> > Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
>> >> >
>> >> > Richard
>> >> >
>> >> >
>> >> > 2019-07-07 Richard Sandiford <richard.sandiford@arm.com>
>> >> >
>> >> > gcc/
>> >> > PR debug/86638
>> >> > * tree-ssa-dce.c (keep_all_vdefs_p): New function.
>> >> > (mark_stmt_if_obviously_necessary): Mark all stmts with vdefs as
>> >> > necessary if keep_all_vdefs_p is true.
>> >> > (mark_aliased_reaching_defs_necessary): Add a gcc_checking_assert
>> >> > that keep_all_vdefs_p is false.
>> >> > (mark_all_reaching_defs_necessary): Likewise.
>> >> > (propagate_necessity): Skip the vuse scan if keep_all_vdefs_p is true.
>> >> >
>> >> > gcc/testsuite/
>> >> > * c-c++-common/guality/Og-dce-1.c: New test.
>> >> > * c-c++-common/guality/Og-dce-2.c: Likewise.
>> >> > * c-c++-common/guality/Og-dce-3.c: Likewise.
>> >> OK
>> >
>> > I wonder how code size (and compile-time) is affected by the DSE/DCE patch?
>> > Say just look at -Og built cc1?
>>
>> Overall I see a ~2.5% slowdown and a 4.7% increase in load size.
>> That comes almost entirely from the (RTL) DSE side; this patch
>> and gimple DSE part don't seem to make much difference.
>>
>> If I keep the gimple passes as-is and just disable RTL DSE, the slowdown
>> is still ~2.5% and there's a 4.4% increase in load size.
>>
>> These are all measuring cc1plus (built from post-patch sources)
>> and using -O2 -g tree-into-ssa.ii for the speed checks.
>>
>> > Can you restrict the keep-all-vdefs to user variables (and measure the
>> > difference this makes)?
>>
>> In order to avoid wrong debug for pointer dereferences, I think it would
>> have to be keep-all-vdefs for writes to either user variables or unknown
>> locations. But as above, I can't measure a significant difference with
>> the patch.
>>
>> > Again I wonder if this makes C++ with -Og impractical runtime-wise.
>>
>> Got a particular test in mind?
>
> Nothing specific - there are a few C/C++ benchmarks in SPEC and there's
> also tramp3d-v4. I guess SRA is much more important for the abstraction
> penalty than DSE - FRE should be able to remove the abstraction, just the
> dead stores will remain (but they'd probably nicely execute out-of-order).
>
> Anyway, the biggest runtime penalty from -Og is probably not running
> any loop optimization (invariant motion mostly).
Finally tried it on tramp3d-v4, and I see a slowdown of ~1.6%.
Thanks,
Richard
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Prevent tree-ssa-dce.c from deleting stores at -Og
2019-07-07 20:13 ` Jeff Law
2019-07-08 11:44 ` Richard Biener
@ 2019-07-29 9:11 ` Richard Sandiford
1 sibling, 0 replies; 8+ messages in thread
From: Richard Sandiford @ 2019-07-29 9:11 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
Jeff Law <law@redhat.com> writes:
> On 7/7/19 3:45 AM, Richard Sandiford wrote:
>> DCE tries to delete dead stores to local data and also tries to insert
>> debug binds for simple cases:
>>
>> /* If this is a store into a variable that is being optimized away,
>> add a debug bind stmt if possible. */
>> if (MAY_HAVE_DEBUG_BIND_STMTS
>> && gimple_assign_single_p (stmt)
>> && is_gimple_val (gimple_assign_rhs1 (stmt)))
>> {
>> tree lhs = gimple_assign_lhs (stmt);
>> if ((VAR_P (lhs) || TREE_CODE (lhs) == PARM_DECL)
>> && !DECL_IGNORED_P (lhs)
>> && is_gimple_reg_type (TREE_TYPE (lhs))
>> && !is_global_var (lhs)
>> && !DECL_HAS_VALUE_EXPR_P (lhs))
>> {
>> tree rhs = gimple_assign_rhs1 (stmt);
>> gdebug *note
>> = gimple_build_debug_bind (lhs, unshare_expr (rhs), stmt);
>> gsi_insert_after (i, note, GSI_SAME_STMT);
>> }
>> }
>>
>> But this doesn't help for things like "print *ptr" when ptr points
>> to the local variable (tests Og-dce-1.c and Og-dce-2.c). It also tends
>> to make the *live* -- and thus useful -- values optimised out, because
>> we can't yet switch back to tracking the memory location as it evolves
>> over time (test Og-dce-3.c).
>>
>> So for -Og I think it'd be better not to delete any stmts with
>> vdefs for now. This also means that we can avoid the potentially
>> expensive vop walks (which already have a cut-off, but still).
>>
>> The patch also fixes the Og failures in gcc.dg/guality/pr54970.c
>> (PR 86638).
>>
>> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
>>
>> Richard
>>
>>
>> 2019-07-07 Richard Sandiford <richard.sandiford@arm.com>
>>
>> gcc/
>> PR debug/86638
>> * tree-ssa-dce.c (keep_all_vdefs_p): New function.
>> (mark_stmt_if_obviously_necessary): Mark all stmts with vdefs as
>> necessary if keep_all_vdefs_p is true.
>> (mark_aliased_reaching_defs_necessary): Add a gcc_checking_assert
>> that keep_all_vdefs_p is false.
>> (mark_all_reaching_defs_necessary): Likewise.
>> (propagate_necessity): Skip the vuse scan if keep_all_vdefs_p is true.
>>
>> gcc/testsuite/
>> * c-c++-common/guality/Og-dce-1.c: New test.
>> * c-c++-common/guality/Og-dce-2.c: Likewise.
>> * c-c++-common/guality/Og-dce-3.c: Likewise.
> OK
> jeff
Thanks. I realised later that Og-dce-3.c didn't test what I claimed it
was testing, so I committed the following version instead. (I re-checked
that all the tests failed without the patch.)
Richard
2019-07-29 Richard Sandiford <richard.sandiford@arm.com>
gcc/
PR debug/86638
* tree-ssa-dce.c (keep_all_vdefs_p): New function.
(mark_stmt_if_obviously_necessary): Mark all stmts with vdefs as
necessary if keep_all_vdefs_p is true.
(mark_aliased_reaching_defs_necessary): Add a gcc_checking_assert
that keep_all_vdefs_p is false.
(mark_all_reaching_defs_necessary): Likewise.
(propagate_necessity): Skip the vuse scan if keep_all_vdefs_p is true.
gcc/testsuite/
* c-c++-common/guality/Og-dce-1.c: New test.
* c-c++-common/guality/Og-dce-2.c: Likewise.
* c-c++-common/guality/Og-dce-3.c: Likewise.
Index: gcc/tree-ssa-dce.c
===================================================================
--- gcc/tree-ssa-dce.c 2019-07-29 09:39:50.030163053 +0100
+++ gcc/tree-ssa-dce.c 2019-07-29 09:47:56.498267828 +0100
@@ -115,6 +115,14 @@ #define STMT_NECESSARY GF_PLF_1
static int *bb_postorder;
+/* True if we should treat any stmt with a vdef as necessary. */
+
+static inline bool
+keep_all_vdefs_p ()
+{
+ return optimize_debug;
+}
+
/* If STMT is not already marked necessary, mark it, and add it to the
worklist if ADD_TO_WORKLIST is true. */
@@ -317,6 +325,12 @@ mark_stmt_if_obviously_necessary (gimple
return;
}
+ if (gimple_vdef (stmt) && keep_all_vdefs_p ())
+ {
+ mark_stmt_necessary (stmt, true);
+ return;
+ }
+
return;
}
@@ -532,6 +546,9 @@ mark_aliased_reaching_defs_necessary_1 (
static void
mark_aliased_reaching_defs_necessary (gimple *stmt, tree ref)
{
+ /* Should have been caught before calling this function. */
+ gcc_checking_assert (!keep_all_vdefs_p ());
+
unsigned int chain;
ao_ref refd;
gcc_assert (!chain_ovfl);
@@ -610,6 +627,8 @@ mark_all_reaching_defs_necessary_1 (ao_r
static void
mark_all_reaching_defs_necessary (gimple *stmt)
{
+ /* Should have been caught before calling this function. */
+ gcc_checking_assert (!keep_all_vdefs_p ());
walk_aliased_vdefs (NULL, gimple_vuse (stmt),
mark_all_reaching_defs_necessary_1, NULL, &visited);
}
@@ -813,6 +832,10 @@ propagate_necessity (bool aggressive)
if (!use)
continue;
+ /* No need to search for vdefs if we intrinsicly keep them all. */
+ if (keep_all_vdefs_p ())
+ continue;
+
/* If we dropped to simple mode make all immediately
reachable definitions necessary. */
if (chain_ovfl)
Index: gcc/testsuite/c-c++-common/guality/Og-dce-1.c
===================================================================
--- /dev/null 2019-06-14 15:59:19.298479944 +0100
+++ gcc/testsuite/c-c++-common/guality/Og-dce-1.c 2019-07-29 09:47:56.494267859 +0100
@@ -0,0 +1,14 @@
+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+int *__attribute__((noipa)) consume (int *ptr) { return ptr; }
+
+int
+main (void)
+{
+ int x;
+ int *volatile ptr = consume (&x);
+ x = 0;
+ x = 1; /* { dg-final { gdb-test . "*ptr" "0" } } */
+ return 0; /* { dg-final { gdb-test . "*ptr" "1" } } */
+}
Index: gcc/testsuite/c-c++-common/guality/Og-dce-2.c
===================================================================
--- /dev/null 2019-06-14 15:59:19.298479944 +0100
+++ gcc/testsuite/c-c++-common/guality/Og-dce-2.c 2019-07-29 09:47:56.498267828 +0100
@@ -0,0 +1,19 @@
+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+struct s { int a, b, c, d; };
+
+struct s gs1 = { 1, 2, 3, 4 };
+struct s gs2 = { 5, 6, 7, 8 };
+
+struct s *__attribute__((noipa)) consume (struct s *ptr) { return ptr; }
+
+int
+main (void)
+{
+ struct s x;
+ struct s *volatile ptr = consume (&x);
+ x = gs1;
+ x = gs2; /* { dg-final { gdb-test . "ptr->a" "1" } } */
+ return 0; /* { dg-final { gdb-test . "ptr->a" "5" } } */
+}
Index: gcc/testsuite/c-c++-common/guality/Og-dce-3.c
===================================================================
--- /dev/null 2019-06-14 15:59:19.298479944 +0100
+++ gcc/testsuite/c-c++-common/guality/Og-dce-3.c 2019-07-29 09:47:56.498267828 +0100
@@ -0,0 +1,29 @@
+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+volatile int amount = 10;
+
+void __attribute__((noipa))
+do_something (int *ptr)
+{
+ *ptr += 10;
+}
+
+int __attribute__((noipa))
+foo (int count)
+{
+ int x = 1;
+ for (int i = 0; i < count; ++i)
+ do_something (&x); /* { dg-final { gdb-test . "x" "1" } } */
+ int res = x; /* { dg-final { gdb-test . "x" "101" } } */
+ x = res + 1;
+ return res; /* { dg-final { gdb-test . "x" "102" } } */
+
+}
+
+int
+main (void)
+{
+ foo (10);
+ return 0;
+}
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-07-29 8:53 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-07 11:59 Prevent tree-ssa-dce.c from deleting stores at -Og Richard Sandiford
2019-07-07 20:13 ` Jeff Law
2019-07-08 11:44 ` Richard Biener
2019-07-08 14:59 ` Richard Sandiford
2019-07-08 16:32 ` Richard Biener
2019-07-12 11:20 ` Richard Sandiford
2019-07-08 15:06 ` Jeff Law
2019-07-29 9:11 ` Richard Sandiford
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).