There's a class of guality failures at Og that goes away when using -ftree-sra: ... FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+4 a[0] == 1 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+3 a[1] == 2 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+2 a[2] == 3 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+1 *p == 3 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line . *q == 2 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+4 a[0] == 1 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+3 a[1] == 2 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+2 a[2] == 13 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+1 *p == 13 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line . *q == 2 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+4 a[0] == 1 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+3 a[1] == 12 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+2 a[2] == 13 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+1 *p == 13 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line . *q == 12 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+3 a[1] == 5 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+2 a[2] == 6 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+1 *p == 6 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line . *q == 5 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+3 a[1] == 5 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+2 a[2] == 26 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+1 *p == 26 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line . *q == 5 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+7 a[1] == 25 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+6 a[2] == 26 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+5 *p == 26 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+4 p[-1] == 25 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line .+1 q[1] == 26 FAIL: gcc.dg/guality/pr54970.c -Og -DPREVENT_OPTIMIZATION line . *q == 25 FAIL: gcc.dg/guality/pr56154-1.c -Og -DPREVENT_OPTIMIZATION line pr56154-1.c:20 x.a == 6 FAIL: gcc.dg/guality/pr59776.c -Og -DPREVENT_OPTIMIZATION line pr59776.c:17 s1.f == 5.0 FAIL: gcc.dg/guality/pr59776.c -Og -DPREVENT_OPTIMIZATION line pr59776.c:17 s1.g == 6.0 FAIL: gcc.dg/guality/pr59776.c -Og -DPREVENT_OPTIMIZATION line pr59776.c:17 s2.f == 0.0 FAIL: gcc.dg/guality/pr59776.c -Og -DPREVENT_OPTIMIZATION line pr59776.c:17 s2.g == 6.0 FAIL: gcc.dg/guality/pr59776.c -Og -DPREVENT_OPTIMIZATION line pr59776.c:20 s1.f == 5.0 FAIL: gcc.dg/guality/pr59776.c -Og -DPREVENT_OPTIMIZATION line pr59776.c:20 s1.g == 6.0 FAIL: gcc.dg/guality/pr59776.c -Og -DPREVENT_OPTIMIZATION line pr59776.c:20 s2.f == 5.0 FAIL: gcc.dg/guality/pr59776.c -Og -DPREVENT_OPTIMIZATION line pr59776.c:20 s2.g == 6.0 ...
F.i., take pr56154-1.c: ... 1 /* PR debug/56154 */ 2 /* { dg-do run } */ 3 /* { dg-options "-g" } */ 4 /* { dg-additional-sources "pr56154-aux.c" } */ 5 6 #include "../nop.h" 7 8 union U { int a, b; }; 9 volatile int z; 10 11 __attribute__((noinline, noclone)) int 12 foo (int fd, union U x) 13 { 14 int result = x.a != 0; 15 if (fd != 0) 16 result = x.a == 0; 17 asm (NOP : : : "memory"); /* { dg-final { gdb-test pr56154-1.c:17 "x.a" "4" } } */ 18 z = x.a; 19 x.a = 6; 20 asm (NOP : : : "memory"); /* { dg-final { gdb-test pr56154-1.c:20 "x.a" "6" } } */ 21 return result; 22 } 23 24 void 25 test_main (void) 26 { 27 union U u = { .a = 4 }; 28 foo (0, u); 29 } ... which fails like this: ... FAIL: gcc.dg/guality/pr56154-1.c -Og -DPREVENT_OPTIMIZATION line pr56154-1.c:20 x.a == 6 ... Without -ftree-sra, we have: ... $ grep DEBUG pr56154-1.c.228t.optimized | grep -v BEGIN_STMT # DEBUG result => result_7 # DEBUG result => result_9 # DEBUG result => result_5 ... and with -ftree-sra, we have: ... $ grep DEBUG pr56154-1.c.228t.optimized | grep -v BEGIN_STMT # DEBUG x$a => x$a_11 # DEBUG result => result_5 # DEBUG result => result_7 # DEBUG result => result_3 # DEBUG x$a => 6 ... In general, we might be able to improve the situation by emitting var_location at expand for non-ssa vars that we emit in registers. But in this case it won't help us, because the store of 6 to x.a is already removed by dce by the time we arrive at expand. Using the fkeep-vars-live patch, we manage to prevent the dce, and are able to print the '6' value of x one line later, at line 21, but not at line 20, due to a "DEBUG x RESET". AFAIU, the var-tracking manages to deduce from the artificial use inserted by fkeep-vars-live that x is in reg si at ret, but it can't deduce that the store of 6 into reg si is also related to x. ... .loc 1 19 3 is_stmt 1 .loc 1 19 7 is_stmt 0 movl $6, %esi .LVL4: # DEBUG x RESET .loc 1 20 3 is_stmt 1 nop .loc 1 21 3 .LVL5: # DEBUG x => si .loc 1 22 1 is_stmt 0 ret ... But, if we'd insert the var_location of x at expand (maybe after every assign to x), we could deduce that the store of 6 is related to x, and we'd be able to print the value of x at line 20.
Hmm, it sounds like DCE/DSE should insert # DEBUG x$a => x$a_11 kind debug stmts. IIRC SRA does more than that, adding DECL_DEBUG_EXPRs with magic. Not sure if the debug stmts itself help enough here.
(In reply to Richard Biener from comment #2) > Hmm, it sounds like DCE/DSE should insert > > # DEBUG x$a => x$a_11 > > kind debug stmts. IIRC SRA does more than that, adding DECL_DEBUG_EXPRs > with magic. > > Not sure if the debug stmts itself help enough here. At cddce1, we have: ... __attribute__((noclone, noinline)) foo (int fd, union U x) { int result; int _1; _Bool _2; _Bool _4; int _5; <bb 2> : # DEBUG BEGIN_STMT _1 = x.a; _2 = _1 != 0; result_8 = (int) _2; # DEBUG result => result_8 # DEBUG BEGIN_STMT if (fd_9(D) != 0) goto <bb 3>; [INV] else goto <bb 4>; [INV] <bb 3> : # DEBUG BEGIN_STMT _4 = _1 == 0; result_10 = (int) _4; # DEBUG result => result_10 <bb 4> : # result_6 = PHI <result_8(2), result_10(3)> # DEBUG result => result_6 # DEBUG BEGIN_STMT __asm__ __volatile__("nop" : : : "memory"); # DEBUG BEGIN_STMT _5 = x.a; z ={v} _5; # DEBUG BEGIN_STMT - x.a = 6; # DEBUG BEGIN_STMT __asm__ __volatile__("nop" : : : "memory"); # DEBUG BEGIN_STMT return result_6; } ... So, are you proposing to keep track of components like this: ... - x.a = 6; + # DEBUG x.a => 6 ... ?
Author: rsandifo Date: Mon Jul 29 08:52:56 2019 New Revision: 273872 URL: https://gcc.gnu.org/viewcvs?rev=273872&root=gcc&view=rev Log: 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 <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. Added: trunk/gcc/testsuite/c-c++-common/guality/Og-dce-1.c trunk/gcc/testsuite/c-c++-common/guality/Og-dce-2.c trunk/gcc/testsuite/c-c++-common/guality/Og-dce-3.c Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-ssa-dce.c
(In reply to rsandifo@gcc.gnu.org from comment #4) > Author: rsandifo > Date: Mon Jul 29 08:52:56 2019 > New Revision: 273872 > > URL: https://gcc.gnu.org/viewcvs?rev=273872&root=gcc&view=rev > Log: > 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). > So can this be closed now then? > 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. > > Added: > trunk/gcc/testsuite/c-c++-common/guality/Og-dce-1.c > trunk/gcc/testsuite/c-c++-common/guality/Og-dce-2.c > trunk/gcc/testsuite/c-c++-common/guality/Og-dce-3.c > Modified: > trunk/gcc/ChangeLog > trunk/gcc/testsuite/ChangeLog > trunk/gcc/tree-ssa-dce.c
Fixed on trunk (a while ago... thanks Eric for the reminder :-))