| Message ID | mpttvbytby3.fsf@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Prevent tree-ssa-dce.c from deleting stores at -Og | expand |
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
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
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
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
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
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
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; +}
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; +}
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.