| Message ID | 4824451.YNsNmM2r07@polaris |
|---|---|
| State | New |
| Headers | show |
| Series | Small improvements to coverage info (1/n) | expand |
On 7/3/19 4:46 AM, Eric Botcazou wrote: > Hi, > > we have collected a number of small improvements to coverage info generated by > the compiler over the years. One of the issues is when a new expression or > statement is built without source location information and ends up inheriting > the source location information of the previous instruction in the debug info, > which can be totally unrelated. > > Tested on x86_64-suse-linux, both GCC and GDB, OK for mainline? > > > 2019-07-03 Eric Botcazou <ebotcazou@adacore.com> > > * tree-cfg.c (gimple_make_forwarder_block): Propagate location info on > phi nodes if possible. > * tree-scalar-evolution.c (final_value_replacement_loop): Propagate > location info on the newly created statement. > * tree-ssa-loop-manip.c (create_iv): Propagate location info on the > newly created increment if needed. > OK jeff
On Wed, Jul 03, 2019 at 12:46:37PM +0200, Eric Botcazou wrote: > 2019-07-03 Eric Botcazou <ebotcazou@adacore.com> > > * tree-cfg.c (gimple_make_forwarder_block): Propagate location info on > phi nodes if possible. > * tree-scalar-evolution.c (final_value_replacement_loop): Propagate > location info on the newly created statement. > * tree-ssa-loop-manip.c (create_iv): Propagate location info on the > newly created increment if needed. > --- tree-ssa-loop-manip.c (revision 272930) > +++ tree-ssa-loop-manip.c (working copy) > @@ -126,10 +126,22 @@ create_iv (tree base, tree step, tree va > gsi_insert_seq_on_edge_immediate (pe, stmts); > > stmt = gimple_build_assign (va, incr_op, vb, step); > + /* Prevent the increment from inheriting a bogus location if it is not put > + immediately after a statement whose location is known. */ > if (after) > - gsi_insert_after (incr_pos, stmt, GSI_NEW_STMT); > + { > + if (gsi_end_p (*incr_pos)) > + { > + edge e = single_succ_edge (gsi_bb (*incr_pos)); > + gimple_set_location (stmt, e->goto_locus); > + } > + gsi_insert_after (incr_pos, stmt, GSI_NEW_STMT); > + } > else > - gsi_insert_before (incr_pos, stmt, GSI_NEW_STMT); > + { > + gimple_set_location (stmt, gimple_location (gsi_stmt (*incr_pos))); > + gsi_insert_before (incr_pos, stmt, GSI_NEW_STMT); > + } This change broke gomp/pr88107.c test: FAIL: gcc.dg/gomp/pr88107.c (internal compiler error) FAIL: gcc.dg/gomp/pr88107.c (test for excess errors) Excess errors: during GIMPLE pass: graphite /usr/src/gcc/gcc/testsuite/gcc.dg/gomp/pr88107.c:26:1: internal compiler error: Segmentation fault 0x11942a4 crash_signal ../../gcc/toplev.c:326 0x13b5861 gimple_location ../../gcc/gimple.h:1805 0x13b76f8 create_iv(tree_node*, tree_node*, tree_node*, loop*, gimple_stmt_iterator*, bool, tree_node**, tree_node**) ../../gcc/tree-ssa-loop-manip.c:142 0xaa2c95 create_empty_loop_on_edge(edge_def*, tree_node*, tree_node*, tree_node*, tree_node*, tree_node**, tree_node**, loop*) ../../gcc/cfgloopmanip.c:831 Apparently gsi_end_p (*incr_pos) is true and after is false, so gsi_stmt (*incr_pos) is NULL. One needs graphite enabled. Jakub
On 7/5/19 6:12 AM, Jakub Jelinek wrote: > On Wed, Jul 03, 2019 at 12:46:37PM +0200, Eric Botcazou wrote: >> 2019-07-03 Eric Botcazou <ebotcazou@adacore.com> >> >> * tree-cfg.c (gimple_make_forwarder_block): Propagate location info on >> phi nodes if possible. >> * tree-scalar-evolution.c (final_value_replacement_loop): Propagate >> location info on the newly created statement. >> * tree-ssa-loop-manip.c (create_iv): Propagate location info on the >> newly created increment if needed. > >> --- tree-ssa-loop-manip.c (revision 272930) >> +++ tree-ssa-loop-manip.c (working copy) >> @@ -126,10 +126,22 @@ create_iv (tree base, tree step, tree va >> gsi_insert_seq_on_edge_immediate (pe, stmts); >> >> stmt = gimple_build_assign (va, incr_op, vb, step); >> + /* Prevent the increment from inheriting a bogus location if it is not put >> + immediately after a statement whose location is known. */ >> if (after) >> - gsi_insert_after (incr_pos, stmt, GSI_NEW_STMT); >> + { >> + if (gsi_end_p (*incr_pos)) >> + { >> + edge e = single_succ_edge (gsi_bb (*incr_pos)); >> + gimple_set_location (stmt, e->goto_locus); >> + } >> + gsi_insert_after (incr_pos, stmt, GSI_NEW_STMT); >> + } >> else >> - gsi_insert_before (incr_pos, stmt, GSI_NEW_STMT); >> + { >> + gimple_set_location (stmt, gimple_location (gsi_stmt (*incr_pos))); >> + gsi_insert_before (incr_pos, stmt, GSI_NEW_STMT); >> + } > > This change broke gomp/pr88107.c test: > FAIL: gcc.dg/gomp/pr88107.c (internal compiler error) > FAIL: gcc.dg/gomp/pr88107.c (test for excess errors) > Excess errors: > during GIMPLE pass: graphite > /usr/src/gcc/gcc/testsuite/gcc.dg/gomp/pr88107.c:26:1: internal compiler error: Segmentation fault > 0x11942a4 crash_signal > ../../gcc/toplev.c:326 > 0x13b5861 gimple_location > ../../gcc/gimple.h:1805 > 0x13b76f8 create_iv(tree_node*, tree_node*, tree_node*, loop*, gimple_stmt_iterator*, bool, tree_node**, tree_node**) > ../../gcc/tree-ssa-loop-manip.c:142 > 0xaa2c95 create_empty_loop_on_edge(edge_def*, tree_node*, tree_node*, tree_node*, tree_node*, tree_node**, tree_node**, loop*) > ../../gcc/cfgloopmanip.c:831 > > Apparently gsi_end_p (*incr_pos) is true and after is false, so > gsi_stmt (*incr_pos) is NULL. One needs graphite enabled. Which might explain the massive failures I'm seeing with graphite enabled on various *-elf targets. ft32-elf for example: > Tests that now fail, but worked before (34 tests): > > ft32-sim: gcc.dg/graphite/id-10.c (test for excess errors) > ft32-sim: gcc.dg/graphite/id-16.c (test for excess errors) > ft32-sim: gcc.dg/graphite/id-25.c (test for excess errors) > ft32-sim: gcc.dg/graphite/id-pr46834.c (test for excess errors) > ft32-sim: gcc.dg/graphite/id-pr47046.c (test for excess errors) > ft32-sim: gcc.dg/graphite/id-pr48805.c (test for excess errors) > ft32-sim: gcc.dg/graphite/interchange-8.c scan-tree-dump graphite "tiled by" > ft32-sim: gcc.dg/graphite/pr29330.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr30565.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr31183.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr33576.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr36287.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr42211.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr42914.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr42917.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr46168.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr46966.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr68428.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr68493.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr68756.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr69068.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr69292.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr70045.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr77362.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr80906.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr80906.c scan-tree-dump graphite "isl AST to Gimple succeeded" > ft32-sim: gcc.dg/graphite/pr81373-2.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr81373.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr82421.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr83255.c scan-tree-dump graphite "loop nest optimized" > ft32-sim: gcc.dg/graphite/pr84204.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr84205.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr85935.c (test for excess errors) > ft32-sim: gcc.dg/graphite/pr87931.c (test for excess errors) SImilarly for c6x, arc, & nds32 (everything that's run in the last few hours). I suspect there'll be more as the day goes on. jeff
> This change broke gomp/pr88107.c test: > FAIL: gcc.dg/gomp/pr88107.c (internal compiler error) > FAIL: gcc.dg/gomp/pr88107.c (test for excess errors) > Excess errors: > during GIMPLE pass: graphite > /usr/src/gcc/gcc/testsuite/gcc.dg/gomp/pr88107.c:26:1: internal compiler > error: Segmentation fault 0x11942a4 crash_signal > ../../gcc/toplev.c:326 > 0x13b5861 gimple_location > ../../gcc/gimple.h:1805 > 0x13b76f8 create_iv(tree_node*, tree_node*, tree_node*, loop*, > gimple_stmt_iterator*, bool, tree_node**, tree_node**) > ../../gcc/tree-ssa-loop-manip.c:142 > 0xaa2c95 create_empty_loop_on_edge(edge_def*, tree_node*, tree_node*, > tree_node*, tree_node*, tree_node**, tree_node**, loop*) > ../../gcc/cfgloopmanip.c:831 > > Apparently gsi_end_p (*incr_pos) is true and after is false, so > gsi_stmt (*incr_pos) is NULL. One needs graphite enabled. OK, thanks for the heads up. I have installed the attached fixlet for now. * tree-ssa-loop-manip.c (create_iv): Add missing guard for gsi_end_p.
Index: tree-cfg.c =================================================================== --- tree-cfg.c (revision 272930) +++ tree-cfg.c (working copy) @@ -5756,6 +5756,7 @@ gimple_make_forwarder_block (edge fallth basic_block dummy, bb; tree var; gphi_iterator gsi; + bool forward_location_p; dummy = fallthru->src; bb = fallthru->dest; @@ -5763,6 +5764,9 @@ gimple_make_forwarder_block (edge fallth if (single_pred_p (bb)) return; + /* We can forward location info if we have only one predecessor. */ + forward_location_p = single_pred_p (dummy); + /* If we redirected a branch we must create new PHI nodes at the start of BB. */ for (gsi = gsi_start_phis (dummy); !gsi_end_p (gsi); gsi_next (&gsi)) @@ -5774,7 +5778,8 @@ gimple_make_forwarder_block (edge fallth new_phi = create_phi_node (var, bb); gimple_phi_set_result (phi, copy_ssa_name (var, phi)); add_phi_arg (new_phi, gimple_phi_result (phi), fallthru, - UNKNOWN_LOCATION); + forward_location_p + ? gimple_phi_arg_location (phi, 0) : UNKNOWN_LOCATION); } /* Add the arguments we have stored on edges. */ Index: tree-scalar-evolution.c =================================================================== --- tree-scalar-evolution.c (revision 272930) +++ tree-scalar-evolution.c (working copy) @@ -3680,6 +3680,8 @@ final_value_replacement_loop (struct loo true, GSI_SAME_STMT); gassign *ass = gimple_build_assign (rslt, def); + gimple_set_location (ass, + gimple_phi_arg_location (phi, exit->dest_idx)); gsi_insert_before (&gsi, ass, GSI_SAME_STMT); if (dump_file) { Index: tree-ssa-loop-manip.c =================================================================== --- tree-ssa-loop-manip.c (revision 272930) +++ tree-ssa-loop-manip.c (working copy) @@ -126,10 +126,22 @@ create_iv (tree base, tree step, tree va gsi_insert_seq_on_edge_immediate (pe, stmts); stmt = gimple_build_assign (va, incr_op, vb, step); + /* Prevent the increment from inheriting a bogus location if it is not put + immediately after a statement whose location is known. */ if (after) - gsi_insert_after (incr_pos, stmt, GSI_NEW_STMT); + { + if (gsi_end_p (*incr_pos)) + { + edge e = single_succ_edge (gsi_bb (*incr_pos)); + gimple_set_location (stmt, e->goto_locus); + } + gsi_insert_after (incr_pos, stmt, GSI_NEW_STMT); + } else - gsi_insert_before (incr_pos, stmt, GSI_NEW_STMT); + { + gimple_set_location (stmt, gimple_location (gsi_stmt (*incr_pos))); + gsi_insert_before (incr_pos, stmt, GSI_NEW_STMT); + } initial = force_gimple_operand (base, &stmts, true, var); if (stmts)