On Fri, Apr 12, 2013 at 9:48 AM, Eric Botcazou <ebotca...@adacore.com> wrote:
>> The problem is actually bigger than just the var-tracking notes. The
>> general problem is that there are no rules for whether or not notes
>> are part of a basic block or not. Some notes never appear inside a
>> basic block, some notes always must appear inside a basic block, and
>> some can appear virtually anywhere.
>
> Can we reorganize insn-notes.def so that the 3 classes are clearly separated
> (and optionally define a NOTE_INSN_CLASS macro so that we don't need to
> enumerate the notes each time)?

That's probably a good idea, yes. I can sort the notes in
insn-notes.def, add a field to DEF_INSN_NOTE, and add a
NOTE_INSN_CLASS to rtl.h.  I'll update the patch and re-submit.

> Reading your patch, it apppears that NOTE_INSN_SWITCH_TEXT_SECTIONS is the
> only member of the 1st class and that almost all notes are in the 2nd class
> except for the 2 LOCATION and the 2 EH_REGION recently added notes.

I think DELETED_LABEL and DELETED_DEBUG_LABEL also can only live
outside basic blocks (i.e. like SWITCH_TEXT_SECTIONS), but I'm not
sure.


>  - Remove the
>
>     This and the next should be the only functions called to insert an insn
>     once delay slots have been filled since only they know how to update a
>     SEQUENCE.
>
> from the head comment of functions that are now private to emit-rtl.c.

OK.


>  - Are there still public functions that cannot be invoked in emit-rtl.c to
> insert insns where there are SEQUENCEs in the stream?  If so, you need to mark
> the ones which can be invoked explicitly with a blurb in the head comment.

Well, only add_insn if you don't count the emit_* functions. add_insn
already has a comment that says what it can handle.  I will explicitly
add a remark to it that it doesn't handle SEQUENCE. NB: I didn't
really change anything here, I only wanted add_insn_before and
add_insn_after to share more code because I have follow-up patches to
make add_insn_before/add_insn_after capable of inserting a SEQUENCE
insn, too (reorg.c now "manually" splices them into the insn chain).

I'll send an updated patch this weekend.

Ciao!
Steven

Reply via email to