From f33b9c40b97f6f8a72ee370068ad81e33d71434e Mon Sep 17 00:00:00 2001 From: Richard Sandiford Date: Mon, 29 Jul 2019 08:52:56 +0000 Subject: [PATCH] Prevent tree-ssa-dce.c from deleting stores at -Og 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 can also introduce wrong debug info for earlier references (second test in Og-dce-3.c) or make earlier references unavailable (first test in 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). 2019-07-29 Richard Sandiford 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. From-SVN: r273872 --- gcc/ChangeLog | 11 +++++++ gcc/testsuite/ChangeLog | 6 ++++ gcc/testsuite/c-c++-common/guality/Og-dce-1.c | 14 +++++++++ gcc/testsuite/c-c++-common/guality/Og-dce-2.c | 19 ++++++++++++ gcc/testsuite/c-c++-common/guality/Og-dce-3.c | 29 +++++++++++++++++++ gcc/tree-ssa-dce.c | 23 +++++++++++++++ 6 files changed, 102 insertions(+) create mode 100644 gcc/testsuite/c-c++-common/guality/Og-dce-1.c create mode 100644 gcc/testsuite/c-c++-common/guality/Og-dce-2.c create mode 100644 gcc/testsuite/c-c++-common/guality/Og-dce-3.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 3e84bafe8bd..8cd321ea61b 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,14 @@ +2019-07-29 Richard Sandiford + + 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. + 2019-07-29 Richard Sandiford * common.opt (Og): Change the initial value of flag_dse to 0. diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 6dab4515305..cc1aadfac55 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2019-07-29 Richard Sandiford + + * 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. + 2019-07-29 Richard Sandiford * c-c++-common/guality/Og-global-dse-1.c: New test. diff --git a/gcc/testsuite/c-c++-common/guality/Og-dce-1.c b/gcc/testsuite/c-c++-common/guality/Og-dce-1.c new file mode 100644 index 00000000000..a859e32521a --- /dev/null +++ b/gcc/testsuite/c-c++-common/guality/Og-dce-1.c @@ -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" } } */ +} diff --git a/gcc/testsuite/c-c++-common/guality/Og-dce-2.c b/gcc/testsuite/c-c++-common/guality/Og-dce-2.c new file mode 100644 index 00000000000..3df2c792193 --- /dev/null +++ b/gcc/testsuite/c-c++-common/guality/Og-dce-2.c @@ -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" } } */ +} diff --git a/gcc/testsuite/c-c++-common/guality/Og-dce-3.c b/gcc/testsuite/c-c++-common/guality/Og-dce-3.c new file mode 100644 index 00000000000..fa6186a7394 --- /dev/null +++ b/gcc/testsuite/c-c++-common/guality/Og-dce-3.c @@ -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; +} diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c index 90b3f4d7c45..17a8d5e2eee 100644 --- a/gcc/tree-ssa-dce.c +++ b/gcc/tree-ssa-dce.c @@ -115,6 +115,14 @@ static bool cfg_altered; 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 *stmt, bool aggressive) 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 (ao_ref *ref, tree vdef, void *data) 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_ref *ref ATTRIBUTE_UNUSED, 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) -- 2.30.2