public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch] fix expand_builtin_init_dwarf_reg_sizes wrt register spans
@ 2014-09-30 10:47 Olivier Hainque
  2014-10-15 15:28 ` [ping] account for register spans in expand_builtin_init_dwarf_reg_sizes Olivier Hainque
  2014-11-18  8:45 ` [patch] fix expand_builtin_init_dwarf_reg_sizes wrt register spans Richard Henderson
  0 siblings, 2 replies; 11+ messages in thread
From: Olivier Hainque @ 2014-09-30 10:47 UTC (permalink / raw)
  To: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 2467 bytes --]

Hello,

Exception propagation has been failing for a while for the SPE/e500 family of
powerpc targets. The issue boils down to an assert failure through:

    uw_init_context_1 ()
    ...
    _Unwind_SetSpColumn (context, outer_cfa, &sp_slot);

then

    _Unwind_SetSpColumn ()
    ...
    int size = dwarf_reg_size_table[__builtin_dwarf_sp_column ()];

    if (size == sizeof(_Unwind_Ptr))
       tmp_sp->ptr = (_Unwind_Ptr) cfa;
    else
      {
        gcc_assert (size == sizeof(_Unwind_Word));

Indeed, dwarf_reg_size_table[sp] is 8 while sizeof(_Unwind_Word) and
sizeof(_Unwind_Ptr) are both 4. dwarf_reg_size[sp] 8 is an outcome of:

  commit 275035b56823b26d5fb7e90fad945b998648edf2
  Date:   Thu Sep 5 14:09:07 2013 +0000

       PR target/58139
       * reginfo.c (choose_hard_reg_mode): Scan through all mode classes
       looking for widest mode.

choose_hard_reg_mode returning 8 for register r1 is not incorrect per se,
I think.

The problem, IMO, is expand_builtin_init_dwarf_reg_sizes ignoring the
targetm.dwarf_register_span hook, unlike other functions in dwarf2outcfi.c.

This patch is a proposal to fix this.

The general idea is isolate the dwarf_reg_size computation for a single
register in a separate function, called either for the "current" register
if it doesn't span, or for each items of the span otherwise.

Working on this, I noticed that expand_builtin_init_dwarf_reg_sizes was
also not honoring DWARF_REG_TO_UNWIND_COLUMN. ISTM that it should so the
patch adjusts on this front as well.

We have been using a slight variant of this in production for a few months
on a gcc-4.9 base. Our 4.9 patch required adjustment to account for the
introduction of the dwarf_frame_reg_mode hook on mainline in the interim.

Tested by verifying that the proper size is stored for GPRs on
powerpc-eabispe, then with bootstrap and regtest for languages=all,ada on
x86_64-linux.

OK to commit ?

Thanks in advance for your feedback,

With Kind Regards,

Olivier

2014-09-30  Olivier Hainque  <hainque@adacore.com>

        libgcc/
        * unwind-dw2.c (DWARF_REG_TO_UNWIND_COLUMN): Move default def to ...

        gcc/
        * defaults.h: ... here.
        * dwarf2cfi.c (init_one_dwarf_reg_size): New helper, processing
        one particular reg for expand_builtin_init_dwarf_reg_sizes. Apply
        DWARF_REG_TO_UNWIND_COLUMN.
        (expand_builtin_init_dwarf_reg_sizes): Rework to use helper and
        account for dwarf register spans.


[-- Attachment #2: cfispan.diff --]
[-- Type: application/octet-stream, Size: 4075 bytes --]

diff --git gcc/defaults.h gcc/defaults.h
index c1776b0..38bb9fa 100644
--- gcc/defaults.h
+++ gcc/defaults.h
@@ -438,6 +438,11 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 #define DWARF_FRAME_REGNUM(REG) DBX_REGISTER_NUMBER (REG)
 #endif
 
+/* The mapping from dwarf CFA reg number to internal dwarf reg numbers.  */
+#ifndef DWARF_REG_TO_UNWIND_COLUMN
+#define DWARF_REG_TO_UNWIND_COLUMN(REGNO) (REGNO)
+#endif
+
 /* Map register numbers held in the call frame info that gcc has
    collected using DWARF_FRAME_REGNUM to those that should be output in
    .debug_frame and .eh_frame.  */
diff --git gcc/dwarf2cfi.c gcc/dwarf2cfi.c
index c8c8a42..9fd0e5a 100644
--- gcc/dwarf2cfi.c
+++ gcc/dwarf2cfi.c
@@ -252,7 +252,44 @@ init_return_column_size (enum machine_mode mode, rtx mem, unsigned int c)
 		  gen_int_mode (size, mode));
 }
 
-/* Generate code to initialize the register size table.  */
+/* Helper for expand_builtin_init_dwarf_reg_sizes.  Generate code to
+   initialize the dwarf register size table entry corresponding to register
+   REGNO in REGMODE.  TABLE is the table base address, SLOTMODE is the mode
+   to use for the size entry to initialize, and WROTE_RETURN_COLUMN needs to
+   be set to true if the dwarf register number for REGNO is the dwarf return
+   column number.  */
+
+static
+void init_one_dwarf_reg_size (int regno, enum machine_mode regmode,
+			      rtx table, enum machine_mode slotmode,
+			      bool *wrote_return_column)
+{
+  const unsigned int dnum = DWARF_FRAME_REGNUM (regno);
+  const unsigned int rnum = DWARF2_FRAME_REG_OUT (dnum, 1);
+  const unsigned int dcol = DWARF_REG_TO_UNWIND_COLUMN (rnum);
+  
+  const HOST_WIDE_INT slotoffset = dcol * GET_MODE_SIZE (slotmode);
+  const HOST_WIDE_INT regsize = GET_MODE_SIZE (regmode);
+
+  if (rnum >= DWARF_FRAME_REGISTERS)
+    return;
+
+  if (dnum == DWARF_FRAME_RETURN_COLUMN)
+    {
+      if (regmode == VOIDmode)
+	return;
+      *wrote_return_column = true;
+    }
+
+  if (slotoffset < 0)
+    return;
+
+  emit_move_insn (adjust_address (table, slotmode, slotoffset),
+		  gen_int_mode (regsize, slotmode));
+}
+
+/* Generate code to initialize the dwarf register size table located
+   at the provided ADDRESS.  */
 
 void
 expand_builtin_init_dwarf_reg_sizes (tree address)
@@ -265,27 +302,21 @@ expand_builtin_init_dwarf_reg_sizes (tree address)
 
   for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
     {
-      unsigned int dnum = DWARF_FRAME_REGNUM (i);
-      unsigned int rnum = DWARF2_FRAME_REG_OUT (dnum, 1);
+      enum machine_mode save_mode = targetm.dwarf_frame_reg_mode (i);
+      rtx span;
 
-      if (rnum < DWARF_FRAME_REGISTERS)
+      span = targetm.dwarf_register_span (gen_rtx_REG (save_mode, i));
+      if (!span)
+	init_one_dwarf_reg_size (i, save_mode, mem, mode, &wrote_return_column);
+      else
 	{
-	  HOST_WIDE_INT offset = rnum * GET_MODE_SIZE (mode);
-	  HOST_WIDE_INT size;
-	  enum machine_mode save_mode = targetm.dwarf_frame_reg_mode (i);
-
-	  if (dnum == DWARF_FRAME_RETURN_COLUMN)
+	  for (int si = 0; si < XVECLEN (span, 0); si++)
 	    {
-	      if (save_mode == VOIDmode)
-		continue;
-	      wrote_return_column = true;
-	    }
-	  size = GET_MODE_SIZE (save_mode);
-	  if (offset < 0)
-	    continue;
+	      rtx reg = XVECEXP (span, 0, si);
 
-	  emit_move_insn (adjust_address (mem, mode, offset),
-			  gen_int_mode (size, mode));
+	      init_one_dwarf_reg_size
+		(REGNO (reg), GET_MODE (reg), mem, mode, &wrote_return_column);
+	    }
 	}
     }
 
diff --git libgcc/unwind-dw2.c libgcc/unwind-dw2.c
index e474433..b262fd9 100644
--- libgcc/unwind-dw2.c
+++ libgcc/unwind-dw2.c
@@ -55,10 +55,6 @@
 #define PRE_GCC3_DWARF_FRAME_REGISTERS __LIBGCC_DWARF_FRAME_REGISTERS__
 #endif
 
-#ifndef DWARF_REG_TO_UNWIND_COLUMN
-#define DWARF_REG_TO_UNWIND_COLUMN(REGNO) (REGNO)
-#endif
-
 /* ??? For the public function interfaces, we tend to gcc_assert that the
    column numbers are in range.  For the dwarf2 unwind info this does happen,
    although so far in a case that doesn't actually matter.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [ping] account for register spans in expand_builtin_init_dwarf_reg_sizes
  2014-09-30 10:47 [patch] fix expand_builtin_init_dwarf_reg_sizes wrt register spans Olivier Hainque
@ 2014-10-15 15:28 ` Olivier Hainque
  2014-10-31 10:38   ` [ping*2] fix unwinding on e500 processors Olivier Hainque
  2014-11-18  2:48   ` [ping] account for register spans in expand_builtin_init_dwarf_reg_sizes Jason Merrill
  2014-11-18  8:45 ` [patch] fix expand_builtin_init_dwarf_reg_sizes wrt register spans Richard Henderson
  1 sibling, 2 replies; 11+ messages in thread
From: Olivier Hainque @ 2014-10-15 15:28 UTC (permalink / raw)
  To: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 1207 bytes --]

Hello,

ping for https://gcc.gnu.org/ml/gcc-patches/2014-09/msg02625.html

a patch proposal to fix exception propagation on spe powerpc targets.

The idea is to improve expand_builtin_init_dwarf_reg_sizes to account
for register spans as other functions in dwarf2cfi.c do, by extracting
the processing for one dwarf register in a separate function.

The original patch merged that plus an adjustment to account for
DWARF_REG_TO_UNWIND_COLUMN. 

The two things are splitted apart here to facilitate review.

Thanks in advance for your feedback,

With Kind Regards,

Olivier

--

Patch1 (dwregspan.diff)

2014-10-15  Olivier Hainque  <hainque@adacore.com>

       * dwarf2cfi.c (init_one_dwarf_reg_size): New helper, processing
       one particular reg for expand_builtin_init_dwarf_reg_sizes.
       (expand_builtin_init_dwarf_reg_sizes): Rework to use helper and
       account for dwarf register spans.

Patch2 (uwcolumn.diff)

2014-10-15  Olivier Hainque  <hainque@adacore.com>

       libgcc/
       * unwind-dw2.c (DWARF_REG_TO_UNWIND_COLUMN): Move default def to ...

       gcc/
       * defaults.h: ... here.
       * dwarf2cfi.c (init_one_dwarf_reg_size): Honor
       DWARF_REG_TO_UNWIND_COLUMN.


[-- Attachment #2: dwregspan.diff --]
[-- Type: application/octet-stream, Size: 2859 bytes --]

diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c
index b7fa3bc..ac6c7c8 100644
--- a/gcc/dwarf2cfi.c
+++ b/gcc/dwarf2cfi.c
@@ -252,7 +252,43 @@ init_return_column_size (enum machine_mode mode, rtx mem, unsigned int c)
 		  gen_int_mode (size, mode));
 }
 
-/* Generate code to initialize the register size table.  */
+/* Helper for expand_builtin_init_dwarf_reg_sizes.  Generate code to
+   initialize the dwarf register size table entry corresponding to register
+   REGNO in REGMODE.  TABLE is the table base address, SLOTMODE is the mode
+   to use for the size entry to initialize, and WROTE_RETURN_COLUMN needs to
+   be set to true if the dwarf register number for REGNO is the dwarf return
+   column number.  */
+
+static
+void init_one_dwarf_reg_size (int regno, enum machine_mode regmode,
+			      rtx table, enum machine_mode slotmode,
+			      bool *wrote_return_column)
+{
+  const unsigned int dnum = DWARF_FRAME_REGNUM (regno);
+  const unsigned int rnum = DWARF2_FRAME_REG_OUT (dnum, 1);
+  
+  const HOST_WIDE_INT slotoffset = rnum * GET_MODE_SIZE (slotmode);
+  const HOST_WIDE_INT regsize = GET_MODE_SIZE (regmode);
+
+  if (rnum >= DWARF_FRAME_REGISTERS)
+    return;
+
+  if (dnum == DWARF_FRAME_RETURN_COLUMN)
+    {
+      if (regmode == VOIDmode)
+	return;
+      *wrote_return_column = true;
+    }
+
+  if (slotoffset < 0)
+    return;
+
+  emit_move_insn (adjust_address (table, slotmode, slotoffset),
+		  gen_int_mode (regsize, slotmode));
+}
+
+/* Generate code to initialize the dwarf register size table located
+   at the provided ADDRESS.  */
 
 void
 expand_builtin_init_dwarf_reg_sizes (tree address)
@@ -265,27 +301,21 @@ expand_builtin_init_dwarf_reg_sizes (tree address)
 
   for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
     {
-      unsigned int dnum = DWARF_FRAME_REGNUM (i);
-      unsigned int rnum = DWARF2_FRAME_REG_OUT (dnum, 1);
+      enum machine_mode save_mode = targetm.dwarf_frame_reg_mode (i);
+      rtx span;
 
-      if (rnum < DWARF_FRAME_REGISTERS)
+      span = targetm.dwarf_register_span (gen_rtx_REG (save_mode, i));
+      if (!span)
+	init_one_dwarf_reg_size (i, save_mode, mem, mode, &wrote_return_column);
+      else
 	{
-	  HOST_WIDE_INT offset = rnum * GET_MODE_SIZE (mode);
-	  HOST_WIDE_INT size;
-	  enum machine_mode save_mode = targetm.dwarf_frame_reg_mode (i);
-
-	  if (dnum == DWARF_FRAME_RETURN_COLUMN)
+	  for (int si = 0; si < XVECLEN (span, 0); si++)
 	    {
-	      if (save_mode == VOIDmode)
-		continue;
-	      wrote_return_column = true;
-	    }
-	  size = GET_MODE_SIZE (save_mode);
-	  if (offset < 0)
-	    continue;
+	      rtx reg = XVECEXP (span, 0, si);
 
-	  emit_move_insn (adjust_address (mem, mode, offset),
-			  gen_int_mode (size, mode));
+	      init_one_dwarf_reg_size
+		(REGNO (reg), GET_MODE (reg), mem, mode, &wrote_return_column);
+	    }
 	}
     }
 

[-- Attachment #3: uwcolumn.diff --]
[-- Type: application/octet-stream, Size: 1810 bytes --]

diff --git a/gcc/defaults.h b/gcc/defaults.h
index c1776b0..38bb9fa 100644
--- a/gcc/defaults.h
+++ b/gcc/defaults.h
@@ -438,6 +438,11 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 #define DWARF_FRAME_REGNUM(REG) DBX_REGISTER_NUMBER (REG)
 #endif
 
+/* The mapping from dwarf CFA reg number to internal dwarf reg numbers.  */
+#ifndef DWARF_REG_TO_UNWIND_COLUMN
+#define DWARF_REG_TO_UNWIND_COLUMN(REGNO) (REGNO)
+#endif
+
 /* Map register numbers held in the call frame info that gcc has
    collected using DWARF_FRAME_REGNUM to those that should be output in
    .debug_frame and .eh_frame.  */
diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c
index ac6c7c8..2da9a23 100644
--- a/gcc/dwarf2cfi.c
+++ b/gcc/dwarf2cfi.c
@@ -266,8 +266,9 @@ void init_one_dwarf_reg_size (int regno, enum machine_mode regmode,
 {
   const unsigned int dnum = DWARF_FRAME_REGNUM (regno);
   const unsigned int rnum = DWARF2_FRAME_REG_OUT (dnum, 1);
+  const unsigned int dcol = DWARF_REG_TO_UNWIND_COLUMN (rnum);
   
-  const HOST_WIDE_INT slotoffset = rnum * GET_MODE_SIZE (slotmode);
+  const HOST_WIDE_INT slotoffset = dcol * GET_MODE_SIZE (slotmode);
   const HOST_WIDE_INT regsize = GET_MODE_SIZE (regmode);
 
   if (rnum >= DWARF_FRAME_REGISTERS)
diff --git a/libgcc/unwind-dw2.c b/libgcc/unwind-dw2.c
index e474433..b262fd9 100644
--- a/libgcc/unwind-dw2.c
+++ b/libgcc/unwind-dw2.c
@@ -55,10 +55,6 @@
 #define PRE_GCC3_DWARF_FRAME_REGISTERS __LIBGCC_DWARF_FRAME_REGISTERS__
 #endif
 
-#ifndef DWARF_REG_TO_UNWIND_COLUMN
-#define DWARF_REG_TO_UNWIND_COLUMN(REGNO) (REGNO)
-#endif
-
 /* ??? For the public function interfaces, we tend to gcc_assert that the
    column numbers are in range.  For the dwarf2 unwind info this does happen,
    although so far in a case that doesn't actually matter.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [ping*2] fix unwinding on e500 processors
  2014-10-15 15:28 ` [ping] account for register spans in expand_builtin_init_dwarf_reg_sizes Olivier Hainque
@ 2014-10-31 10:38   ` Olivier Hainque
  2014-11-17 14:50     ` [ping*3] " Olivier Hainque
  2014-11-18  2:48   ` [ping] account for register spans in expand_builtin_init_dwarf_reg_sizes Jason Merrill
  1 sibling, 1 reply; 11+ messages in thread
From: Olivier Hainque @ 2014-10-31 10:38 UTC (permalink / raw)
  To: GCC Patches

Hello,

ping for https://gcc.gnu.org/ml/gcc-patches/2014-10/msg01369.html

a proposal to repair unwinding breakage on e500 targets.

The url above is that of the first ping, where the originally
submitted patch was splitted in two to facilitate review.

The original submission, with a more complete description of the
problem is at https://gcc.gnu.org/ml/gcc-patches/2014-09/msg02625.html.

Thanks in advance for your feedback,

Olivier


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [ping*3] fix unwinding on e500 processors
  2014-10-31 10:38   ` [ping*2] fix unwinding on e500 processors Olivier Hainque
@ 2014-11-17 14:50     ` Olivier Hainque
  0 siblings, 0 replies; 11+ messages in thread
From: Olivier Hainque @ 2014-11-17 14:50 UTC (permalink / raw)
  To: GCC Patches

Hello,

ping for https://gcc.gnu.org/ml/gcc-patches/2014-10/msg03264.html

TIA for your feedback,

Olivier

On Oct 31, 2014, at 11:31 , Olivier Hainque <hainque@adacore.com> wrote:

> Hello,
> 
> ping for https://gcc.gnu.org/ml/gcc-patches/2014-10/msg01369.html
> 
> a proposal to repair unwinding breakage on e500 targets.
> 
> The url above is that of the first ping, where the originally
> submitted patch was splitted in two to facilitate review.
> 
> The original submission, with a more complete description of the
> problem is at https://gcc.gnu.org/ml/gcc-patches/2014-09/msg02625.html.
> 
> Thanks in advance for your feedback,
> 
> Olivier
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [ping] account for register spans in expand_builtin_init_dwarf_reg_sizes
  2014-10-15 15:28 ` [ping] account for register spans in expand_builtin_init_dwarf_reg_sizes Olivier Hainque
  2014-10-31 10:38   ` [ping*2] fix unwinding on e500 processors Olivier Hainque
@ 2014-11-18  2:48   ` Jason Merrill
  2014-11-18 15:38     ` Olivier Hainque
  1 sibling, 1 reply; 11+ messages in thread
From: Jason Merrill @ 2014-11-18  2:48 UTC (permalink / raw)
  To: Olivier Hainque, GCC Patches

On 10/15/2014 11:07 AM, Olivier Hainque wrote:
>    for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
>      {
> +      enum machine_mode save_mode = targetm.dwarf_frame_reg_mode (i);
> +      rtx span;
>
> +      span = targetm.dwarf_register_span (gen_rtx_REG (save_mode, i));
> +      if (!span)
> +	init_one_dwarf_reg_size (i, save_mode, mem, mode, &wrote_return_column);
> +      else
>  	{
> +	  for (int si = 0; si < XVECLEN (span, 0); si++)
>  	    {
> +	      rtx reg = XVECEXP (span, 0, si);
>
> +	      init_one_dwarf_reg_size
> +		(REGNO (reg), GET_MODE (reg), mem, mode, &wrote_return_column);
> +	    }
>  	}
>      }

What happens when the outer loop hits a register that we've already seen 
as part of a span?

Jason

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch] fix expand_builtin_init_dwarf_reg_sizes wrt register spans
  2014-09-30 10:47 [patch] fix expand_builtin_init_dwarf_reg_sizes wrt register spans Olivier Hainque
  2014-10-15 15:28 ` [ping] account for register spans in expand_builtin_init_dwarf_reg_sizes Olivier Hainque
@ 2014-11-18  8:45 ` Richard Henderson
  1 sibling, 0 replies; 11+ messages in thread
From: Richard Henderson @ 2014-11-18  8:45 UTC (permalink / raw)
  To: Olivier Hainque, GCC Patches

On 09/30/2014 12:47 PM, Olivier Hainque wrote:
> 2014-09-30  Olivier Hainque  <hainque@adacore.com>
> 
>         libgcc/
>         * unwind-dw2.c (DWARF_REG_TO_UNWIND_COLUMN): Move default def to ...
> 
>         gcc/
>         * defaults.h: ... here.
>         * dwarf2cfi.c (init_one_dwarf_reg_size): New helper, processing
>         one particular reg for expand_builtin_init_dwarf_reg_sizes. Apply
>         DWARF_REG_TO_UNWIND_COLUMN.
>         (expand_builtin_init_dwarf_reg_sizes): Rework to use helper and
>         account for dwarf register spans.

Ok.


r~

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [ping] account for register spans in expand_builtin_init_dwarf_reg_sizes
  2014-11-18  2:48   ` [ping] account for register spans in expand_builtin_init_dwarf_reg_sizes Jason Merrill
@ 2014-11-18 15:38     ` Olivier Hainque
  2014-11-24  9:32       ` Olivier Hainque
  0 siblings, 1 reply; 11+ messages in thread
From: Olivier Hainque @ 2014-11-18 15:38 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Olivier Hainque, GCC Patches


On Nov 18, 2014, at 03:29 , Jason Merrill <jason@redhat.com> wrote:
> What happens when the outer loop hits a register that we've already seen as part of a span?

 Hmm, I was under the impression that this was supposed never to happen. I can
 see that this is not so clear though.

 What would happen with the current code is the production of extraneous
 instructions initializing the same slot, presumably with the same values.

 I have a candidate improvement to prevent processing the same regno
 multiple times.

 Will followup as soon as I have some test results.

 Thanks for your feedback,

 Olivier

 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [ping] account for register spans in expand_builtin_init_dwarf_reg_sizes
  2014-11-18 15:38     ` Olivier Hainque
@ 2014-11-24  9:32       ` Olivier Hainque
  2014-12-04  9:03         ` Olivier Hainque
  2014-12-04 22:14         ` Jason Merrill
  0 siblings, 2 replies; 11+ messages in thread
From: Olivier Hainque @ 2014-11-24  9:32 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Olivier Hainque, GCC Patches

[-- Attachment #1: Type: text/plain, Size: 1592 bytes --]


On Nov 18, 2014, at 16:21 , Olivier Hainque <hainque@adacore.com> wrote:
> On Nov 18, 2014, at 03:29 , Jason Merrill <jason@redhat.com> wrote:
>> What happens when the outer loop hits a register that we've already seen as part of a span?

> I have a candidate improvement to prevent processing the same regno
> multiple times.
> 
> Will followup as soon as I have some test results.

Slightly delayed by transient local events which deferred
the production of test results of relevance.

The attached patches combined bootstrap and regtest fine on x86_64-linux.
We also have nominal test results with our 4.9 based series of compilers
in-house, including for the e500 family of targets.

OK to commit ?

Thanks for your feedback,

Olivier

Fix for unwinder aborts on e500:

2014-11-24  Olivier Hainque  <hainque@adacore.com>

        * dwarf2cfi.c (init_one_dwarf_reg_size): New helper, processing
        one particular reg for expand_builtin_init_dwarf_reg_sizes.  Take
        care not to process the same register multiple times.
        (expand_builtin_init_dwarf_reg_sizes): Rework to use helper and
        account for dwarf register spans.


[see attached file: init-dw-reg-span.diff]

Honor DWARF_REG_TO_UNWIND_COLUMN in dwarf_reg_size_table init:

2014-11-24  Olivier Hainque  <hainque@adacore.com>

        libgcc/
        * unwind-dw2.c (DWARF_REG_TO_UNWIND_COLUMN): Move default def to ...
    
        gcc/
        * defaults.h: ... here.
        * dwarf2cfi.c (init_one_dwarf_reg_size): Honor
        DWARF_REG_TO_UNWIND_COLUMN.

[-- Attachment #2: init-dw-reg-column.diff --]
[-- Type: application/octet-stream, Size: 2309 bytes --]

commit 493c8b15aaa5461e7e152227d58c1a6ed285225b
Author: Olivier Hainque <hainque@adacore.com>
Date:   Wed Nov 19 12:12:13 2014 +0100

    Honor DWARF_REG_TO_UNWIND_COLUMN in dwarf_reg_size_table init
    
            libgcc/
            * unwind-dw2.c (DWARF_REG_TO_UNWIND_COLUMN): Move default def to ...
    
            gcc/
            * defaults.h: ... here.
            * dwarf2cfi.c (init_one_dwarf_reg_size): Honor
            DWARF_REG_TO_UNWIND_COLUMN.

diff --git a/gcc/defaults.h b/gcc/defaults.h
index d2609e7..26e5750b 100644
--- a/gcc/defaults.h
+++ b/gcc/defaults.h
@@ -438,6 +438,11 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 #define DWARF_FRAME_REGNUM(REG) DBX_REGISTER_NUMBER (REG)
 #endif
 
+/* The mapping from dwarf CFA reg number to internal dwarf reg numbers.  */
+#ifndef DWARF_REG_TO_UNWIND_COLUMN
+#define DWARF_REG_TO_UNWIND_COLUMN(REGNO) (REGNO)
+#endif
+
 /* Map register numbers held in the call frame info that gcc has
    collected using DWARF_FRAME_REGNUM to those that should be output in
    .debug_frame and .eh_frame.  */
diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c
index 7998871..e9b39d4 100644
--- a/gcc/dwarf2cfi.c
+++ b/gcc/dwarf2cfi.c
@@ -286,8 +286,9 @@ void init_one_dwarf_reg_size (int regno, machine_mode regmode,
 {
   const unsigned int dnum = DWARF_FRAME_REGNUM (regno);
   const unsigned int rnum = DWARF2_FRAME_REG_OUT (dnum, 1);
+  const unsigned int dcol = DWARF_REG_TO_UNWIND_COLUMN (rnum);
   
-  const HOST_WIDE_INT slotoffset = rnum * GET_MODE_SIZE (slotmode);
+  const HOST_WIDE_INT slotoffset = dcol * GET_MODE_SIZE (slotmode);
   const HOST_WIDE_INT regsize = GET_MODE_SIZE (regmode);
 
   /* No point in redoing things if we have processed this register already.
diff --git a/libgcc/unwind-dw2.c b/libgcc/unwind-dw2.c
index e474433..b262fd9 100644
--- a/libgcc/unwind-dw2.c
+++ b/libgcc/unwind-dw2.c
@@ -55,10 +55,6 @@
 #define PRE_GCC3_DWARF_FRAME_REGISTERS __LIBGCC_DWARF_FRAME_REGISTERS__
 #endif
 
-#ifndef DWARF_REG_TO_UNWIND_COLUMN
-#define DWARF_REG_TO_UNWIND_COLUMN(REGNO) (REGNO)
-#endif
-
 /* ??? For the public function interfaces, we tend to gcc_assert that the
    column numbers are in range.  For the dwarf2 unwind info this does happen,
    although so far in a case that doesn't actually matter.

[-- Attachment #3: init-dw-reg-span.diff --]
[-- Type: application/octet-stream, Size: 4522 bytes --]

commit bf75018b15bea00d4a245898a2383d5ba3a65e5d
Author: Olivier Hainque <hainque@adacore.com>
Date:   Wed Nov 19 12:11:23 2014 +0100

    Fix unwinding on e500:
    
    	* dwarf2cfi.c (init_one_dwarf_reg_size): New helper, processing
    	one particular reg for expand_builtin_init_dwarf_reg_sizes.  Take
    	care not to process the same register multiple times.
    	(expand_builtin_init_dwarf_reg_sizes): Rework to use helper and
    	account for dwarf register spans.

diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c
index f2d628c..7998871 100644
--- a/gcc/dwarf2cfi.c
+++ b/gcc/dwarf2cfi.c
@@ -258,7 +258,65 @@ init_return_column_size (machine_mode mode, rtx mem, unsigned int c)
 		  gen_int_mode (size, mode));
 }
 
-/* Generate code to initialize the register size table.  */
+/* Datastructure used by expand_builtin_init_dwarf_reg_sizes and
+   init_one_dwarf_reg_size to communicate on what has been done by the
+   latter.  */
+
+typedef struct
+{
+  /* Whether the dwarf return column was initialized.  */
+  bool wrote_return_column;
+
+  /* For each hard register REGNO, whether init_one_dwarf_reg_size
+     was given REGNO to process already.  */
+  bool processed_regno [FIRST_PSEUDO_REGISTER];
+
+} init_one_dwarf_reg_state;
+
+/* Helper for expand_builtin_init_dwarf_reg_sizes.  Generate code to
+   initialize the dwarf register size table entry corresponding to register
+   REGNO in REGMODE.  TABLE is the table base address, SLOTMODE is the mode to
+   use for the size entry to initialize, and INIT_STATE is the communication
+   datastructure conveying what we're doing to our caller.  */
+
+static
+void init_one_dwarf_reg_size (int regno, machine_mode regmode,
+			      rtx table, machine_mode slotmode,
+			      init_one_dwarf_reg_state *init_state)
+{
+  const unsigned int dnum = DWARF_FRAME_REGNUM (regno);
+  const unsigned int rnum = DWARF2_FRAME_REG_OUT (dnum, 1);
+  
+  const HOST_WIDE_INT slotoffset = rnum * GET_MODE_SIZE (slotmode);
+  const HOST_WIDE_INT regsize = GET_MODE_SIZE (regmode);
+
+  /* No point in redoing things if we have processed this register already.
+     This could happen with register spans, e.g. when regno is first processed
+     as a piece of a span, then as a register on its own later on.  */
+
+  if (init_state->processed_regno[regno])
+    return;
+  init_state->processed_regno[regno] = true;
+
+  if (rnum >= DWARF_FRAME_REGISTERS)
+    return;
+
+  if (dnum == DWARF_FRAME_RETURN_COLUMN)
+    {
+      if (regmode == VOIDmode)
+	return;
+      init_state->wrote_return_column = true;
+    }
+
+  if (slotoffset < 0)
+    return;
+
+  emit_move_insn (adjust_address (table, slotmode, slotoffset),
+		  gen_int_mode (regsize, slotmode));
+}
+
+/* Generate code to initialize the dwarf register size table located
+   at the provided ADDRESS.  */
 
 void
 expand_builtin_init_dwarf_reg_sizes (tree address)
@@ -267,35 +325,32 @@ expand_builtin_init_dwarf_reg_sizes (tree address)
   machine_mode mode = TYPE_MODE (char_type_node);
   rtx addr = expand_normal (address);
   rtx mem = gen_rtx_MEM (BLKmode, addr);
-  bool wrote_return_column = false;
+
+  init_one_dwarf_reg_state init_state;
+
+  memset ((char *)&init_state, 0, sizeof (init_state));
 
   for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
     {
-      unsigned int dnum = DWARF_FRAME_REGNUM (i);
-      unsigned int rnum = DWARF2_FRAME_REG_OUT (dnum, 1);
+      machine_mode save_mode = targetm.dwarf_frame_reg_mode (i);
+      rtx span;
 
-      if (rnum < DWARF_FRAME_REGISTERS)
+      span = targetm.dwarf_register_span (gen_rtx_REG (save_mode, i));
+      if (!span)
+	init_one_dwarf_reg_size (i, save_mode, mem, mode, &init_state);
+      else
 	{
-	  HOST_WIDE_INT offset = rnum * GET_MODE_SIZE (mode);
-	  HOST_WIDE_INT size;
-	  machine_mode save_mode = targetm.dwarf_frame_reg_mode (i);
-
-	  if (dnum == DWARF_FRAME_RETURN_COLUMN)
+	  for (int si = 0; si < XVECLEN (span, 0); si++)
 	    {
-	      if (save_mode == VOIDmode)
-		continue;
-	      wrote_return_column = true;
-	    }
-	  size = GET_MODE_SIZE (save_mode);
-	  if (offset < 0)
-	    continue;
+	      rtx reg = XVECEXP (span, 0, si);
 
-	  emit_move_insn (adjust_address (mem, mode, offset),
-			  gen_int_mode (size, mode));
+	      init_one_dwarf_reg_size
+		(REGNO (reg), GET_MODE (reg), mem, mode, &init_state);
+	    }
 	}
     }
 
-  if (!wrote_return_column)
+  if (!init_state.wrote_return_column)
     init_return_column_size (mode, mem, DWARF_FRAME_RETURN_COLUMN);
 
 #ifdef DWARF_ALT_FRAME_RETURN_COLUMN

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [ping] account for register spans in expand_builtin_init_dwarf_reg_sizes
  2014-11-24  9:32       ` Olivier Hainque
@ 2014-12-04  9:03         ` Olivier Hainque
  2014-12-04 22:14         ` Jason Merrill
  1 sibling, 0 replies; 11+ messages in thread
From: Olivier Hainque @ 2014-12-04  9:03 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches, Olivier Hainque

Hi Jason,

ping for https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02975.html,

a proposal to address comments you made on a patch I had sent
earlier on.

> The attached patches combined bootstrap and regtest fine on x86_64-linux.
> We also have nominal test results with our 4.9 based series of compilers
> in-house, including for the e500 family of targets.

As I mentioned in earlier comments, the DWARF_REG_TO_UNWIND_COLUMN part is
essentially a noop today and is not necessary to fix the breakage. It's just
something that ISTM should be there in principle.

Thanks in advance for your feedback,

With Kind Regards,

Olivier

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [ping] account for register spans in expand_builtin_init_dwarf_reg_sizes
  2014-11-24  9:32       ` Olivier Hainque
  2014-12-04  9:03         ` Olivier Hainque
@ 2014-12-04 22:14         ` Jason Merrill
  2014-12-05 17:05           ` Olivier Hainque
  1 sibling, 1 reply; 11+ messages in thread
From: Jason Merrill @ 2014-12-04 22:14 UTC (permalink / raw)
  To: Olivier Hainque; +Cc: GCC Patches

On 11/24/2014 03:08 AM, Olivier Hainque wrote:
> +  if (init_state->processed_regno[regno])
> +    return;

I would expect this to go in the loop in 
expand_builtin_init_dwarf_reg_sizes, before we look up a span for the 
regno.  OK with that change.

Jason

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [ping] account for register spans in expand_builtin_init_dwarf_reg_sizes
  2014-12-04 22:14         ` Jason Merrill
@ 2014-12-05 17:05           ` Olivier Hainque
  0 siblings, 0 replies; 11+ messages in thread
From: Olivier Hainque @ 2014-12-05 17:05 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Olivier Hainque, GCC Patches


On Dec 4, 2014, at 23:14 , Jason Merrill <jason@redhat.com> wrote:

> On 11/24/2014 03:08 AM, Olivier Hainque wrote:
>> +  if (init_state->processed_regno[regno])
>> +    return;
> 
> I would expect this to go in the loop in expand_builtin_init_dwarf_reg_sizes, before we look up a span for the regno.

Sure.

>  OK with that change.

Great, just checked in after re bootstrap and
regtest on x86_64-linux. Thanks :-)

Regards,

Olivier

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2014-12-05 17:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-30 10:47 [patch] fix expand_builtin_init_dwarf_reg_sizes wrt register spans Olivier Hainque
2014-10-15 15:28 ` [ping] account for register spans in expand_builtin_init_dwarf_reg_sizes Olivier Hainque
2014-10-31 10:38   ` [ping*2] fix unwinding on e500 processors Olivier Hainque
2014-11-17 14:50     ` [ping*3] " Olivier Hainque
2014-11-18  2:48   ` [ping] account for register spans in expand_builtin_init_dwarf_reg_sizes Jason Merrill
2014-11-18 15:38     ` Olivier Hainque
2014-11-24  9:32       ` Olivier Hainque
2014-12-04  9:03         ` Olivier Hainque
2014-12-04 22:14         ` Jason Merrill
2014-12-05 17:05           ` Olivier Hainque
2014-11-18  8:45 ` [patch] fix expand_builtin_init_dwarf_reg_sizes wrt register spans Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).