Re: [PATCH] Do not remove labels with LABEL_PRESERVE_P

[Steven Bosscher <stevenb dot gcc at gmail dot com>]

On Wed, Sep 24, 2014 at 8:41 AM, Ilya Enkovich wrote:
> 2014-09-23 20:06 GMT+04:00 Jeff Law:
>> On 09/23/14 10:01, Steven Bosscher wrote:
>>> Are you sure this patch is necessary, and is not just papering over
>>> another problem? In the past, all cases I've seen where labels were
>>> removed inadvertently were caused by incorrect reference counting or
>>> missing REG_LABEL_* notes.
>
> Description of LABEL_PRESERVE_P says label that should always be
> considered to be needed.

It's more specific than that, really:

@item LABEL_PRESERVE_P (@var{x})
In a @code{code_label} or @code{note}, indicates that the label is referenced by
code or data not visible to the RTL of a given function.


The "not visible" part is important. If there are visible references
to a label, then they should never be removed (obviously) and that
should work through LABEL_NUSES. Unfortunately we are not very good at
keeping LABEL_NUSES up-to-date (this is why all the
rebuild_jump_labels() are still required).

What appears to be the case here, is that you have a label between two
basic blocks B1 and B2, and the label acts as a control flow barrier:
B1 and B2 cannot be merged. Then this should be expressed in the CFG.
Otherwise: What else prevents the merge_blocks CFG hooks from deleting
the label?



> That means even if we do not have any usages
> we shouldn't remove it.

Sorry, no.
Even a LABEL_PRESERVE_P label can be deleted: It will be replaced by a
NOTE_INSN_DELETED_LABEL. See cfgrtl.c:delete_insn().

If you really want to prevent a label from being deleted, then
LABEL_PRESERVE_P is not a sufficient condition.


>  Why can't we add some additional usages
> later?

If you add the usages later, then you're lying to the compiler ;-)


>>>
>>> Did the label use count drop to zero? Is there a REG_LABEL_TARGET note
>>> for the label operand?
>
> In the current code of ix86_expand_prologue I don't see any notes
> generation for set_rip_rex64 instruction which actually uses label.
> But IMO this is another potential issue and we still shouldn't remove
> labels with LABEL_PRESERVE_P.

Notes are generated in jump.c:rebuild_jump_labels. They are
automatically added when a label is not

Ciao!
Steven