| Message ID | 20121114230229.GN1886@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Nov 15, 2012 at 12:02 AM, Jakub Jelinek wrote: > Hi! > > Steven has been complaining for some years that var-tracking inserts > NOTE_INSN_VAR_LOCATION (and NOTE_INSN_CALL_ARG_LOCATION) notes sometimes > in between basic blocks, but with BLOCK_FOR_INSN set (or sometimes extends > a bb ending originally with a noreturn or throwing call by a note or more > after it). Fixed thusly, bootstrapped/regtested on x86_64-linux and > i686-linux, ok for trunk? Many thanks again for fixing this! Do you think it's safe to add a verify_flow_info call at the end of var-tracking to make sure there will be no regressions from here? In case people are curious why this is IMHO so important: This was the last major obstacle for some ports to keep the CFG around until 'final'. Ciao! Steven
On Nov 14, 2012, Jakub Jelinek <jakub@redhat.com> wrote: > Steven has been complaining for some years that var-tracking inserts > NOTE_INSN_VAR_LOCATION (and NOTE_INSN_CALL_ARG_LOCATION) notes sometimes > in between basic blocks, but with BLOCK_FOR_INSN set (or sometimes extends > a bb ending originally with a noreturn or throwing call by a note or more > after it). Fixed thusly, bootstrapped/regtested on x86_64-linux and > i686-linux, ok for trunk? This is ok with me, although I'm thinking it might make more sense to consolidate the rules to decide whether or not to add basic block info to a note in add_insn_{before,after}, where this added in the first place. Ideally we'd use the same rules used by cfg to determine where the block boundaries are, i.e., what kinds of notes can be at BB boundaries and which are outside BBs. (Incidentally, it looks like add_insn_after, unlike add_insn_before, completely ignores and overwrites the bb it is passed as an argument; this seems wrong)
On Thu, Nov 15, 2012 at 12:02 AM, Jakub Jelinek wrote: > 2012-11-14 Jakub Jelinek < > > > PR middle-end/43631 > * var-tracking.c (emit_note_insn_var_location, emit_notes_in_bb): > Clear BLOCK_FOR_INSN on notes emitted in between basic blocks, > don't adjust BB_END when inserting note after BB_END of some bb. Hello Jakub, Please don't forget to commit this one! :-) Ciao! Steven
--- gcc/var-tracking.c.jj 2012-11-13 10:54:06.000000000 +0100 +++ gcc/var-tracking.c 2012-11-13 11:35:31.668284140 +0100 @@ -8566,9 +8566,30 @@ emit_note_insn_var_location (void **varp || NOTE_KIND (insn) == NOTE_INSN_CALL_ARG_LOCATION)) note = emit_note_after (NOTE_INSN_VAR_LOCATION, insn); else - note = emit_note_before (NOTE_INSN_VAR_LOCATION, insn); + { + note = emit_note_before (NOTE_INSN_VAR_LOCATION, insn); + /* If insn is BB_HEAD of some bb, make sure the note + doesn't have BLOCK_FOR_INSN set. The notes don't + extend the extents of a basic block, and e.g. notes emitted + for differences in between basic blocks should live in between + the basic blocks. */ + if (BLOCK_FOR_INSN (note) + && BB_HEAD (BLOCK_FOR_INSN (note)) == insn) + set_block_for_insn (note, NULL); + } } NOTE_VAR_LOCATION (note) = note_vl; + /* If insn is BB_END of some bb, make sure the note + doesn't have BLOCK_FOR_INSN set. The notes don't + extend the extents of a basic block, and e.g. a noreturn + call can still be followed by NOTE_INSN_CALL_ARG_LOCATION. */ + if (BLOCK_FOR_INSN (note) + && BB_END (BLOCK_FOR_INSN (note)) == note + && PREV_INSN (note) == insn) + { + BB_END (BLOCK_FOR_INSN (note)) = insn; + set_block_for_insn (note, NULL); + } set_dv_changed (var->dv, false); gcc_assert (var->in_changed_variables); @@ -8936,6 +8957,16 @@ emit_notes_in_bb (basic_block bb, datafl } note = emit_note_after (NOTE_INSN_CALL_ARG_LOCATION, insn); NOTE_VAR_LOCATION (note) = arguments; + /* If insn is BB_END of some bb, make sure the note + doesn't have BLOCK_FOR_INSN set. The notes don't + extend the extents of a basic block, and e.g. a noreturn + call can still be followed by NOTE_INSN_CALL_ARG_LOCATION. */ + if (BLOCK_FOR_INSN (note) + && BB_END (BLOCK_FOR_INSN (note)) == note) + { + BB_END (BLOCK_FOR_INSN (note)) = insn; + set_block_for_insn (note, NULL); + } } break;