PR bootstrap/87338: Fix ia64 bootstrap comparison regression in r257511

Message ID 20190425175227.67109-1-jrtc27@jrtc27.com
State New
Headers show
Series
  • PR bootstrap/87338: Fix ia64 bootstrap comparison regression in r257511
Related show

Commit Message

Jessica Clarke April 25, 2019, 5:52 p.m.
By using ASM_OUTPUT_LABEL, r257511 forced the assembler to start a new
bundle when emitting an inline entry label on. Instead, use
ASM_OUTPUT_DEBUG_LABEL like for the block begin and end labels so tags are
emitted rather than labels.

gcc/
	PR bootstrap/87338
	* dwarf2out.c (dwarf2out_inline_entry): Use ASM_OUTPUT_DEBUG_LABEL
	instead of ASM_GENERATE_INTERNAL_LABEL and ASM_OUTPUT_LABEL.
---
 gcc/dwarf2out.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

-- 
1.8.5.3

Comments

Richard Biener April 26, 2019, 6:58 a.m. | #1
On Thu, Apr 25, 2019 at 7:52 PM James Clarke <jrtc27@jrtc27.com> wrote:
>

> By using ASM_OUTPUT_LABEL, r257511 forced the assembler to start a new

> bundle when emitting an inline entry label on. Instead, use

> ASM_OUTPUT_DEBUG_LABEL like for the block begin and end labels so tags are

> emitted rather than labels.


Looks sensible.  mips is the other port defining ASM_OUTPUT_DEBUG_LABEL,
so either you can do a bootstrap/test on mips as well or I'm asking Matthew
for approval here.

I'd say OK for trunk (GCC 10) and if things look good backport later to active
branches.

Thanks,
Richard.

> gcc/

>         PR bootstrap/87338

>         * dwarf2out.c (dwarf2out_inline_entry): Use ASM_OUTPUT_DEBUG_LABEL

>         instead of ASM_GENERATE_INTERNAL_LABEL and ASM_OUTPUT_LABEL.

> ---

>  gcc/dwarf2out.c | 7 ++-----

>  1 file changed, 2 insertions(+), 5 deletions(-)

>

> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c

> index b9a624e..c348692 100644

> --- a/gcc/dwarf2out.c

> +++ b/gcc/dwarf2out.c

> @@ -27670,11 +27670,8 @@ dwarf2out_inline_entry (tree block)

>    if (cur_line_info_table)

>      ied->view = cur_line_info_table->view;

>

> -  char label[MAX_ARTIFICIAL_LABEL_BYTES];

> -

> -  ASM_GENERATE_INTERNAL_LABEL (label, BLOCK_INLINE_ENTRY_LABEL,

> -                              BLOCK_NUMBER (block));

> -  ASM_OUTPUT_LABEL (asm_out_file, label);

> +  ASM_OUTPUT_DEBUG_LABEL (asm_out_file, BLOCK_INLINE_ENTRY_LABEL,

> +                         BLOCK_NUMBER (block));

>  }

>

>  /* Called from finalize_size_functions for size functions so that their body

> --

> 1.8.5.3

>
Jakub Jelinek April 26, 2019, 8:01 a.m. | #2
On Fri, Apr 26, 2019 at 08:58:18AM +0200, Richard Biener wrote:
> On Thu, Apr 25, 2019 at 7:52 PM James Clarke <jrtc27@jrtc27.com> wrote:

> >

> > By using ASM_OUTPUT_LABEL, r257511 forced the assembler to start a new

> > bundle when emitting an inline entry label on. Instead, use

> > ASM_OUTPUT_DEBUG_LABEL like for the block begin and end labels so tags are

> > emitted rather than labels.

> 

> Looks sensible.  mips is the other port defining ASM_OUTPUT_DEBUG_LABEL,

> so either you can do a bootstrap/test on mips as well or I'm asking Matthew

> for approval here.


And arm and arc, while they don't define their own ASM_OUTPUT_DEBUG_LABEL,
they override TARGET_ASM_INTERNAL_LABEL which is the underlying
implementation of the default ASM_OUTPUT_DEBUG_LABEL.  But I agree that
for mips it is a significant change, while arm and arc call default_internal_label
from their hook, just do additional stuff.
> 

> I'd say OK for trunk (GCC 10) and if things look good backport later to active

> branches.


Yeah, I'd wait a few weeks before backporting though.

	Jakub
Jeff Law April 26, 2019, 2:24 p.m. | #3
On 4/26/19 2:01 AM, Jakub Jelinek wrote:
> On Fri, Apr 26, 2019 at 08:58:18AM +0200, Richard Biener wrote:

>> On Thu, Apr 25, 2019 at 7:52 PM James Clarke <jrtc27@jrtc27.com> wrote:

>>>

>>> By using ASM_OUTPUT_LABEL, r257511 forced the assembler to start a new

>>> bundle when emitting an inline entry label on. Instead, use

>>> ASM_OUTPUT_DEBUG_LABEL like for the block begin and end labels so tags are

>>> emitted rather than labels.

>>

>> Looks sensible.  mips is the other port defining ASM_OUTPUT_DEBUG_LABEL,

>> so either you can do a bootstrap/test on mips as well or I'm asking Matthew

>> for approval here.

> 

> And arm and arc, while they don't define their own ASM_OUTPUT_DEBUG_LABEL,

> they override TARGET_ASM_INTERNAL_LABEL which is the underlying

> implementation of the default ASM_OUTPUT_DEBUG_LABEL.  But I agree that

> for mips it is a significant change, while arm and arc call default_internal_label

> from their hook, just do additional stuff.

My tester will bootstrap the mips port within 24hrs after the change is
committed.  Happy to contact y'all if something goes wrong ;-)  If you
don't hear from me, assume it didn't cause problems.

Jeff
Jessica Clarke May 21, 2019, 1:08 p.m. | #4
On 26 Apr 2019, at 15:24, Jeff Law <law@redhat.com> wrote:
> On 4/26/19 2:01 AM, Jakub Jelinek wrote:

>> On Fri, Apr 26, 2019 at 08:58:18AM +0200, Richard Biener wrote:

>>> On Thu, Apr 25, 2019 at 7:52 PM James Clarke <jrtc27@jrtc27.com> wrote:

>>>> 

>>>> By using ASM_OUTPUT_LABEL, r257511 forced the assembler to start a new

>>>> bundle when emitting an inline entry label on. Instead, use

>>>> ASM_OUTPUT_DEBUG_LABEL like for the block begin and end labels so tags are

>>>> emitted rather than labels.

>>> 

>>> Looks sensible.  mips is the other port defining ASM_OUTPUT_DEBUG_LABEL,

>>> so either you can do a bootstrap/test on mips as well or I'm asking Matthew

>>> for approval here.

>> 

>> And arm and arc, while they don't define their own ASM_OUTPUT_DEBUG_LABEL,

>> they override TARGET_ASM_INTERNAL_LABEL which is the underlying

>> implementation of the default ASM_OUTPUT_DEBUG_LABEL.  But I agree that

>> for mips it is a significant change, while arm and arc call default_internal_label

>> from their hook, just do additional stuff.

> My tester will bootstrap the mips port within 24hrs after the change is

> committed.  Happy to contact y'all if something goes wrong ;-)  If you

> don't hear from me, assume it didn't cause problems.


Hi all,
It looks like there are no objections to this (assuming it doesn't regress
anything), so could somebody please commit this to trunk?

Thanks,
James
Jeff Law May 21, 2019, 3:43 p.m. | #5
On 5/21/19 7:08 AM, James Clarke wrote:
> On 26 Apr 2019, at 15:24, Jeff Law <law@redhat.com> wrote:

>> On 4/26/19 2:01 AM, Jakub Jelinek wrote:

>>> On Fri, Apr 26, 2019 at 08:58:18AM +0200, Richard Biener wrote:

>>>> On Thu, Apr 25, 2019 at 7:52 PM James Clarke <jrtc27@jrtc27.com> wrote:

>>>>>

>>>>> By using ASM_OUTPUT_LABEL, r257511 forced the assembler to start a new

>>>>> bundle when emitting an inline entry label on. Instead, use

>>>>> ASM_OUTPUT_DEBUG_LABEL like for the block begin and end labels so tags are

>>>>> emitted rather than labels.

>>>>

>>>> Looks sensible.  mips is the other port defining ASM_OUTPUT_DEBUG_LABEL,

>>>> so either you can do a bootstrap/test on mips as well or I'm asking Matthew

>>>> for approval here.

>>>

>>> And arm and arc, while they don't define their own ASM_OUTPUT_DEBUG_LABEL,

>>> they override TARGET_ASM_INTERNAL_LABEL which is the underlying

>>> implementation of the default ASM_OUTPUT_DEBUG_LABEL.  But I agree that

>>> for mips it is a significant change, while arm and arc call default_internal_label

>>> from their hook, just do additional stuff.

>> My tester will bootstrap the mips port within 24hrs after the change is

>> committed.  Happy to contact y'all if something goes wrong ;-)  If you

>> don't hear from me, assume it didn't cause problems.

> 

> Hi all,

> It looks like there are no objections to this (assuming it doesn't regress

> anything), so could somebody please commit this to trunk?

Sorry, it looks like this slipped through the cracks.  I've committed it
to the trunk.  I've also updated the BZ to reflect the bootstrap
regression for gcc-8 and gcc-9 on ia64.

jeff

Patch

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index b9a624e..c348692 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -27670,11 +27670,8 @@  dwarf2out_inline_entry (tree block)
   if (cur_line_info_table)
     ied->view = cur_line_info_table->view;
 
-  char label[MAX_ARTIFICIAL_LABEL_BYTES];
-
-  ASM_GENERATE_INTERNAL_LABEL (label, BLOCK_INLINE_ENTRY_LABEL,
-			       BLOCK_NUMBER (block));
-  ASM_OUTPUT_LABEL (asm_out_file, label);
+  ASM_OUTPUT_DEBUG_LABEL (asm_out_file, BLOCK_INLINE_ENTRY_LABEL,
+			  BLOCK_NUMBER (block));
 }
 
 /* Called from finalize_size_functions for size functions so that their body