Message ID | 525B1BDD.2000306@mentor.com |
---|---|
State | New |
Headers | show |
On 14/10/13 00:17, Tom de Vries wrote: > This patch makes sure we emit insertions scheduled for the first real BB before > NOTE_INSN_FUNCTION_BEG. As a consequence, it moves the PIC register setup code > to before the NOTE_INSN_FUNCTION_BEG. This removes the second .loc, and the > breakpoint of main ends up at line 8. > > Bootstrapped and regtested on x86_64 (ada inclusive), no issues found. > > Tested gdb with target arm-none-linux-gnueabi and CFLAGS_FOR_TARGET=-fPIC. The > patch removes 174 FAILs. > > Re-testing gcc with target arm-none-linux-gnueabi atm. > No issues found. OK for trunk? Thanks, - Tom
Ping. Original submission at http://gcc.gnu.org/ml/gcc-patches/2013-10/msg00903.html . This patch fixes a regression of 174 tests in the gdb testsuite for arm-linux-gnueabi with -fPIC (or arm-linux-androideabi) caused by the fix for PR47028. The fix for PR47028 made sure that insertions on the single edge from ENTRY_BLOCK_PTR to the entry bb are committed after the parameters are available. However, the fix had as side-effect that the insertions could be committed both before and after NOTE_INSN_FUNCTION_BEG. Before the fix, the insertions were always committed before NOTE_INSN_FUNCTION_BEG. NOTE_INSN_FUNCTION_BEG has significance with respect to line number info, and insertions after the note caused wrong .loc info to be generated in the assembly in some cases, which caused the gdb test failures. This patch makes sure the insertions are committed before NOTE_INSN_FUNCTION_BEG. Thanks, - Tom
> The PIC register setup code is emitted after NOTE_INSNS_FUNCTION_BEG, > because it uses the insert_insn_on_edge mechanism, and the corresponding > insertion in cfgexpand.c:gimple_expand_cfg takes care to insert the code > after the parm_birth_insn: > ... > /* Avoid putting insns before parm_birth_insn. */ > if (e->src == ENTRY_BLOCK_PTR > && single_succ_p (ENTRY_BLOCK_PTR) > && parm_birth_insn) > { > rtx insns = e->insns.r; > e->insns.r = NULL_RTX; > emit_insn_after_noloc (insns, parm_birth_insn, e->dest); > } > ... > And in the case for this test-case, parm_birth_insn is the > NOTE_INSNS_FUNCTION_BEG. So this means that parm_birth_insn can never be null, right? > 2013-10-13 Tom de Vries <tom@codesourcery.com> > > * cfgexpand.c (gimple_expand_cfg): Don't commit insertions after > NOTE_INSN_FUNCTION_BEG. > > * gcc.target/arm/require-pic-register-loc.c: New test. OK if you also remove the test on parm_birth_insn.
Index: gcc/cfgexpand.c =================================================================== --- gcc/cfgexpand.c (revision 421892) +++ gcc/cfgexpand.c (working copy) @@ -4618,14 +4618,19 @@ gimple_expand_cfg (void) if (e->insns.r) { rebuild_jump_labels_chain (e->insns.r); - /* Avoid putting insns before parm_birth_insn. */ + /* Put insns after parm birth, but before + NOTE_INSNS_FUNCTION_BEG. */ if (e->src == ENTRY_BLOCK_PTR && single_succ_p (ENTRY_BLOCK_PTR) && parm_birth_insn) { rtx insns = e->insns.r; e->insns.r = NULL_RTX; - emit_insn_after_noloc (insns, parm_birth_insn, e->dest); + if (NOTE_P (parm_birth_insn) + && NOTE_KIND (parm_birth_insn) == NOTE_INSN_FUNCTION_BEG) + emit_insn_before_noloc (insns, parm_birth_insn, e->dest); + else + emit_insn_after_noloc (insns, parm_birth_insn, e->dest); } else commit_one_edge_insertion (e); Index: gcc/testsuite/gcc.target/arm/require-pic-register-loc.c =================================================================== --- /dev/null (new file) +++ gcc/testsuite/gcc.target/arm/require-pic-register-loc.c (revision 0) @@ -0,0 +1,29 @@ +/* { dg-do compile } */ +/* { dg-options "-g -fPIC" } */ + +void *v; +void a (void *x) { } +void b (void) { } + /* line 7. */ +int /* line 8. */ +main (int argc) /* line 9. */ +{ /* line 10. */ + if (argc == 12345) /* line 11. */ + { + a (v); + return 1; + } + b (); + + return 0; +} + +/* { dg-final { scan-assembler-not "\.loc 1 7 0" } } */ +/* { dg-final { scan-assembler-not "\.loc 1 8 0" } } */ +/* { dg-final { scan-assembler-not "\.loc 1 9 0" } } */ + +/* The loc at the start of the prologue. */ +/* { dg-final { scan-assembler-times "\.loc 1 10 0" 1 } } */ + +/* The loc at the end of the prologue, with the first user line. */ +/* { dg-final { scan-assembler-times "\.loc 1 11 0" 1 } } */