This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Prevent tree-ssa-dce.c from deleting stores at -Og
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;
+}