Re: [PATCH] Do not remove labels with LABEL_PRESERVE_P

[Ilya Enkovich <enkovich dot gnu at gmail dot com>]

2014-09-24 13:30 GMT+04:00 Steven Bosscher <stevenb.gcc@gmail.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.

I read another description:
/* 1 if RTX is a code_label that should always be considered to be needed.  */
#define LABEL_PRESERVE_P(RTX)                                           \
  (RTL_FLAG_CHECK2 ("LABEL_PRESERVE_P", (RTX), CODE_LABEL, NOTE)->in_struct)

>
>
> 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).

Does rebuild handle all kinds of instructions including those which use UNSPEC?

>
> 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?

Label acts as a barrier here but it is a side effect.  I don't care
about block merging.  I just don't want label with usages to be
removed.

>
>
>
>> 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().

According to description you quoted label marked by LABEL_PRESERVE_P
is used by some code or data.  Let this use be not visible to the RTL
of a given function.  It is still used, right? How can you remove it?

Ilya

>
> 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