This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Do not remove labels with LABEL_PRESERVE_P
- From: Ilya Enkovich <enkovich dot gnu at gmail dot com>
- To: Jeff Law <law at redhat dot com>
- Cc: Steven Bosscher <stevenb dot gcc at gmail dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 24 Sep 2014 10:41:26 +0400
- Subject: Re: [PATCH] Do not remove labels with LABEL_PRESERVE_P
- Authentication-results: sourceware.org; auth=none
- References: <20140919193604 dot GA48001 at msticlxl57 dot ims dot intel dot com> <541C8C0C dot 8040300 at redhat dot com> <CABu31nN-HZ_qYM4N2w_dstwehmvwTj66XMNB=jfHerwj4NdBXQ at mail dot gmail dot com> <54219A77 dot 70403 at redhat dot com>
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