Re: [patch] fix expand_builtin_init_dwarf_reg_sizes wrt register spans

Re: [patch] fix expand_builtin_init_dwarf_reg_sizes wrt register spans

[David Edelsohn]

Olivier,

The patch is okay with me, but I cannot approve it.

Maybe Jason, Richard or Jeff can take a look.

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

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

Thanks, David

Re: [patch] fix expand_builtin_init_dwarf_reg_sizes wrt register spans

[Olivier Hainque]

Hi David,

Thanks for your support :)

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

Olivier

On Nov 17, 2014, at 16:56 , David Edelsohn <dje.gcc@gmail.com> wrote:

> Olivier,
> 
> The patch is okay with me, but I cannot approve it.
> 
> Maybe Jason, Richard or Jeff can take a look.
> 
> https://gcc.gnu.org/ml/gcc-patches/2014-10/msg01369.html
> 
> https://gcc.gnu.org/ml/gcc-patches/2014-09/msg02625.html
> 
> Thanks, David
> 

Re: [patch] fix expand_builtin_init_dwarf_reg_sizes wrt register spans

[Jeff Law]

On 11/17/14 08:56, David Edelsohn wrote:
> Olivier,
>
> The patch is okay with me, but I cannot approve it.
>
> Maybe Jason, Richard or Jeff can take a look.
>
> https://gcc.gnu.org/ml/gcc-patches/2014-10/msg01369.html
>
> https://gcc.gnu.org/ml/gcc-patches/2014-09/msg02625.html
Best for Jason, Richard or Jakub.  My knowledge of dwarf2 and our 
implementation in dwarf*out.c is minimal at best.

jeff

Re: [patch] fix expand_builtin_init_dwarf_reg_sizes wrt register spans

[Olivier Hainque]

On Nov 18, 2014, at 04:01 , Jeff Law <law@redhat.com> wrote:
> Best for Jason, Richard or Jakub.  My knowledge of dwarf2 and our implementation in dwarf*out.c is minimal at best.

 Thanks for your answer Jeff. Richard & Jason have provided feedback
 (thanks for this as well :) on which I'll followup.

 Olivier

Re: [patch] fix expand_builtin_init_dwarf_reg_sizes wrt register spans

[Richard Henderson]

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~

[patch] fix expand_builtin_init_dwarf_reg_sizes wrt register spans

[Olivier Hainque]

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