[PATCH] Do not remove labels with LABEL_PRESERVE_P

Ilya Enkovich enkovich.gnu@gmail.com
Wed Sep 24 06:41:00 GMT 2014


2014-09-23 20:06 GMT+04:00 Jeff Law <law@redhat.com>:
> On 09/23/14 10:01, Steven Bosscher wrote:
>>
>> On Fri, Sep 19, 2014 at 10:03 PM, Jeff Law <law@redhat.com> wrote:
>>>
>>> On 09/19/14 13:36, Ilya Enkovich wrote:
>>>>
>>>>
>>>> Hi,
>>>>
>>>> During my work on enabling pseudo PIC register I've found that cfg
>>>> cleaunp
>>>> may remove lables with LABEL_PRESERVE_P set to 1.  In my case I
>>>> generated
>>>> SET_RIP during expand pass and cfg cleanup removed label it used as an
>>>> operand.  Below is a patch that fixes it.  It is not actually required
>>>> for
>>>> our latest PIC related patch but still seems to make sense.
>>>>
>>>> Bootstrapped and tested on linux-x86_64.
>>>>
>>>> Thanks,
>>>> Ilya
>>>> --
>>>> 2014-09-19  Ilya Enkovich  <ilya.enkovich@intel.com>
>>>>
>>>>          * cfgcleanup.c (try_optimize_cfg): Do not remove label
>>>>          with LABEL_PRESERVE_P flag set.
>>>
>>>
>>> OK.  Please install.
>>>
>>> Note for those not following the x86 32 bit PIC register discussion, I
>>> asked
>>> Ilya to submit this separately.  It was something an earlier version of
>>> his
>>> patch triggered and it stood out as something that ought to be fixed
>>> regardless of the final form of the PIC register changes that are in
>>> progress.
>>
>>
>> Jeff,
>>
>> 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.  That means even if we do not have any usages
we shouldn't remove it.  Why can't we add some additional usages
later?

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

>
> The way it was described to me is, yes, the label count dropped to zero.  In
> simplest terms, it was a single use label that was marked with
> LABEL_PRESERVE_P.  The combiner removed the last reference, then cfgcleanup
> came along and *boom*.
>

There was also another case in 64bit target with large code model
where I had combiner unrelated problem with removed label used by
still existing set_rip_rex64.

Ilya

> It was with some ongoing development work that's going in a slight different
> direction, so we don't have a testcase to include.
>
> jeff



More information about the Gcc-patches mailing list