Re: [PATCH] Do not remove labels with LABEL_PRESERVE_P

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

2014-09-24 14:35 GMT+04:00 Steven Bosscher <stevenb.gcc@gmail.com>:
> On Wed, Sep 24, 2014 at 11:57 AM, Ilya Enkovich wrote:
>> 2014-09-24 13:30 GMT+04:00 Steven Bosscher :
>>>> 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)
>
> Yes, from rtl.h.
>
> I'd recommend to always read the descriptions in doc/ (in this case
> doc/rtl.texi). The "documentation" in the header files is often not
> very comprehensive.
>
>
>>> 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?
>
> Yes. Patterns are walked (deep) and REG_LABEL notes are added for all
> labels encountered that are not already the JUMP_LABEL of INSN. If the
> label is reachable from XEXP(UNSPEC, 0) -- the 'E' operand -- then
> that label is visible.
>
>
>>> 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.
>
> Understood. Only, LABEL_PRESERVE_P is not the right means to achieve that.
>
> So let's get back to basics and see what the usages look like. AFAIU
> now, you emit the code label early, and add the references much later
> (in machine reorg?). Does your UNSPEC have the code_label as an
> operand? If so, what breaks if cfgcleanup removes the label? Is the
> insn no longer recognized? Or does the label not end up in the
> assembly output? Or ...? I can try to help figure out what breaks if
> you have a test case.
>
> FWIW, the LABEL_PRESERVE_P uses in config/i386/i386.c look suspect. It
> probably only works because those labels are added late, and the code
> paths that use (x86_64 large PIC code model) are not tested all that
> well...

I didn't generate references separately from label.  Now I found an
old patch and a test where this problem appeared.  In this patch I
moved set_rip generation currently performed in ix86_expand_prologue
into expand pass.  And I got following code in expand dump for
testsuite/gcc.target/i386/pr55154.c test:

(note 7 1 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
(note/s 2 7 3 2 "" NOTE_INSN_DELETED_LABEL 2)
(insn 3 2 4 2 (set (reg:DI 85)
        (unspec:DI [
                (label_ref [2 deleted])
            ] UNSPEC_SET_RIP))
/export/users/ienkovic/issues/4161/gcc/gcc/testsuite/gcc.target/i386/pr55154.c:9
-1
     (insn_list:REG_LABEL_OPERAND 2 (nil)))

There is a REG_LABEL_OPERAND generated but label is still removed.

Ilya

>
>
>>>> 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?
>
> The code_label rtx is removed, but the label itself is still output to
> the object file. The label number is retained in the CODE_LABEL_NUMBER
> of the NOTE_INSN_DELETED_LABEL. Look for how NOTE_INSN_DELETED_LABEL
> is handled in final.c. It's a hack IMHO, but that's how it has been
> since day 0 (see https://gcc.gnu.org/r104).
>
> Ciao!
> Steven