[_eh,dawin,Version2] Allow targets to suppress epilogues in _eh frames.

Hi Richard,

This kinda stalled ... I guess I should have pinged it.

On 17 Aug 2010, at 15:57, Richard Henderson wrote:

> On 08/17/2010 06:09 AM, IainS wrote:
>> heh, I guess I meant was solving the problem you noticed above a
>> pre-condition of applying the patch, or should they be considered
>> separate problems?
>
> Let's commit them as separate patches for bi-sect-ability,
> Just In Case.

>
>> cfi->dw_cfi_opc = DW_CFA_GNU_args_size;
>
> Oh, that.  If Darwin doesn't like that, then it's expecting the
> compiler to always use -maccumulate-outgoing-args.  Just make
> sure that gets set by default.  I suppose you'll have to do
> something about the hand-full of test cases that force that
> flag off.


I believe that I've addressed the points made in the thread, is this  
now OK?

I (re)-draw your attention to the fact that I've removed the  
unconditional setting of flag_asynchronous_unwind_tables when  
flag_non_call_exceptions is set in toplev.c.

if this cannot be done then, perhaps, I need to return to the original  
idea of using a target hook to control the suppression of eh epilogues.

thanks
Iain

----

amended & refreshed patch:

gcc:

	* gcc/dwarf2out.c: DW_CFA_INTERNAL_start_epilogue, CFI_T New.
	(struct dw_cfi_struct): Adjust type of dw_cfi_opc.
	(add_fde_cfi):  Handle DW_CFA_INTERNAL_start_epilogue.
	(dwarf2out_cfi_begin_epilogue): Insert epilogue marker.
	(dw_cfi_oprnd1_desc): Adjust argument type.
	(dw_cfi_oprnd2_desc): Likewise.
	(output_cfi): Handle DW_CFA_INTERNAL_start_epilogue as nop.
	(output_cfi_directive): Likewise.
	(emit_cfi_or_skip_epilogue): New.
	(output_fde): Use emit_cfi_or_skip_epilogue.
	* gcc/toplev.c (process_options): Do not set  
flag_asynchronous_unwind_tables
	unless it is unset by the target.  Set flag_unwind_tables for either
	flag_asynchronous_unwind_tables or flag_non_call_exceptions.
	* gcc/config/darwin.c (darwin_override_options): Warn the the target  
does
	not fully support flag_asynchronous_unwind_tables.  Switch  
flag_unwind_tables
	on for flag_non_call_exceptions  or flag_exceptions on darwin  
versions supporting
	_eh frames.  Ensure that all table flags are switched off for kernel  
code.
	* gcc/config/i386/darwin.h (SUBTARGET_OVERRIDE_OPTIONS): New, handle
	making a default unwinder when none is specified by the user.

testsuite:

	* g++.dg/eh/async-unwind1.C:  Prune warning for Darwin.
	* g++.dg/eh/async-unwind2.C: Likewise.


  #ifndef DARWIN_PREFER_DWARF

On Wed, Sep 15, 2010 at 03:23:08PM +0100, IainS wrote:
> Hi Richard,
>
> This kinda stalled ... I guess I should have pinged it.
>
> On 17 Aug 2010, at 15:57, Richard Henderson wrote:
>
>> On 08/17/2010 06:09 AM, IainS wrote:
>>> heh, I guess I meant was solving the problem you noticed above a
>>> pre-condition of applying the patch, or should they be considered
>>> separate problems?
>>
>> Let's commit them as separate patches for bi-sect-ability,
>> Just In Case.
>
>>
>>> cfi->dw_cfi_opc = DW_CFA_GNU_args_size;
>>
>> Oh, that.  If Darwin doesn't like that, then it's expecting the
>> compiler to always use -maccumulate-outgoing-args.  Just make
>> sure that gets set by default.  I suppose you'll have to do
>> something about the hand-full of test cases that force that
>> flag off.
>
>
> I believe that I've addressed the points made in the thread, is this now 
> OK?
>
> I (re)-draw your attention to the fact that I've removed the  
> unconditional setting of flag_asynchronous_unwind_tables when  
> flag_non_call_exceptions is set in toplev.c.
>
> if this cannot be done then, perhaps, I need to return to the original  
> idea of using a target hook to control the suppression of eh epilogues.
>
> thanks
> Iain
>

Iain,
   Can you repost the patch as an attachment? Your mailer seems to be
corrupting the patch...

patching file gcc/dwarf2out.c
patch: **** malformed patch at line 73: /* Otherwise, search forward to see if the return insn was the last

          Jack

> ----
>
> amended & refreshed patch:
>
> gcc:
>
> 	* gcc/dwarf2out.c: DW_CFA_INTERNAL_start_epilogue, CFI_T New.
> 	(struct dw_cfi_struct): Adjust type of dw_cfi_opc.
> 	(add_fde_cfi):  Handle DW_CFA_INTERNAL_start_epilogue.
> 	(dwarf2out_cfi_begin_epilogue): Insert epilogue marker.
> 	(dw_cfi_oprnd1_desc): Adjust argument type.
> 	(dw_cfi_oprnd2_desc): Likewise.
> 	(output_cfi): Handle DW_CFA_INTERNAL_start_epilogue as nop.
> 	(output_cfi_directive): Likewise.
> 	(emit_cfi_or_skip_epilogue): New.
> 	(output_fde): Use emit_cfi_or_skip_epilogue.
> 	* gcc/toplev.c (process_options): Do not set  
> flag_asynchronous_unwind_tables
> 	unless it is unset by the target.  Set flag_unwind_tables for either
> 	flag_asynchronous_unwind_tables or flag_non_call_exceptions.
> 	* gcc/config/darwin.c (darwin_override_options): Warn the the target  
> does
> 	not fully support flag_asynchronous_unwind_tables.  Switch  
> flag_unwind_tables
> 	on for flag_non_call_exceptions  or flag_exceptions on darwin versions 
> supporting
> 	_eh frames.  Ensure that all table flags are switched off for kernel  
> code.
> 	* gcc/config/i386/darwin.h (SUBTARGET_OVERRIDE_OPTIONS): New, handle
> 	making a default unwinder when none is specified by the user.
>
> testsuite:
>
> 	* g++.dg/eh/async-unwind1.C:  Prune warning for Darwin.
> 	* g++.dg/eh/async-unwind2.C: Likewise.
>
>
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c	(revision 164304)
> +++ gcc/dwarf2out.c	(working copy)
> @@ -268,9 +272,17 @@ typedef union GTY(()) dw_cfi_oprnd_struct {
>  }
>  dw_cfi_oprnd;
>
> +/* Use the first out-of-band opcode number as a marker for the start of
> +   epilogues.  */
> +#define DW_CFA_INTERNAL_start_epilogue 0x100
> +#define CFI_T enum dwarf_call_frame_info
> +
> +static enum dw_cfi_oprnd_type dw_cfi_oprnd1_desc (unsigned int cfi);
> +static enum dw_cfi_oprnd_type dw_cfi_oprnd2_desc (unsigned int cfi);
> +
>  typedef struct GTY(()) dw_cfi_struct {
>    dw_cfi_ref dw_cfi_next;
> -  enum dwarf_call_frame_info dw_cfi_opc;
> +  unsigned int dw_cfi_opc;
>    dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd1_desc (%1.dw_cfi_opc)")))
>      dw_cfi_oprnd1;
>    dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd2_desc (%1.dw_cfi_opc)")))
> @@ -820,9 +832,15 @@ add_fde_cfi (const char *label, dw_cfi_ref cfi)
>      }
>
>    list_head = &cie_cfi_head;
> -
> -  if (dwarf2out_do_cfi_asm ())
> +
> +  if (cfi->dw_cfi_opc == DW_CFA_INTERNAL_start_epilogue)
>      {
> +      dw_fde_ref fde = current_fde ();
> +      gcc_assert (fde != NULL);
> +      list_head = &fde->dw_fde_cfi;
> +    }
> +  else if (dwarf2out_do_cfi_asm ())
> +    {
>        if (label)
>  	{
>  	  dw_fde_ref fde = current_fde ();
> @@ -2893,12 +2911,22 @@ dwarf2out_cfi_begin_epilogue (rtx insn)
>      return;
>
>    /* Otherwise, search forward to see if the return insn was the last
> -     basic block of the function.  If so, we don't need save/restore.  
> */
> +     basic block of the function.  If so, we don't need save/restore.
> +     However, we do mark the position so that we can skip the epilogue
> +     in _eh frames where required.  */
>    gcc_assert (i != NULL);
>    i = next_real_insn (i);
>    if (i == NULL)
> -    return;
> +    {
> +      dw_cfi_ref cfi_epi_start;
>
> +      /* Emit a marker for the epilogue start. */
> +      cfi_epi_start = new_cfi ();
> +      cfi_epi_start->dw_cfi_opc = DW_CFA_INTERNAL_start_epilogue;
> +      add_fde_cfi ("", cfi_epi_start);
> +      return;
> +    }
> +
>    /* Insert the restore before that next real insn in the stream, and 
> before
>       a potential NOTE_INSN_EPILOGUE_BEG -- we do need these notes to be
>       properly nested.  This should be after any label or alignment.   
> This
> @@ -2940,11 +2968,9 @@ dwarf2out_frame_debug_restore_state (void)
>  }
>
>  /* Describe for the GTY machinery what parts of dw_cfi_oprnd1 are used.  
> */
> -static enum dw_cfi_oprnd_type dw_cfi_oprnd1_desc
> - (enum dwarf_call_frame_info cfi);
>
>  static enum dw_cfi_oprnd_type
> -dw_cfi_oprnd1_desc (enum dwarf_call_frame_info cfi)
> +dw_cfi_oprnd1_desc (unsigned int cfi)
>  {
>    switch (cfi)
>      {
> @@ -2952,6 +2978,7 @@ static enum dw_cfi_oprnd_type
>      case DW_CFA_GNU_window_save:
>      case DW_CFA_remember_state:
>      case DW_CFA_restore_state:
> +    case DW_CFA_INTERNAL_start_epilogue:
>        return dw_cfi_oprnd_unused;
>
>      case DW_CFA_set_loc:
> @@ -2989,11 +3016,9 @@ static enum dw_cfi_oprnd_type
>  }
>
>  /* Describe for the GTY machinery what parts of dw_cfi_oprnd2 are used.  
> */
> -static enum dw_cfi_oprnd_type dw_cfi_oprnd2_desc
> - (enum dwarf_call_frame_info cfi);
>
>  static enum dw_cfi_oprnd_type
> -dw_cfi_oprnd2_desc (enum dwarf_call_frame_info cfi)
> +dw_cfi_oprnd2_desc (unsigned int cfi)
>  {
>    switch (cfi)
>      {
> @@ -3120,6 +3146,8 @@ output_cfi (dw_cfi_ref cfi, dw_fde_ref fde, int fo
>        dw2_asm_output_data (1, (cfi->dw_cfi_opc | (r & 0x3f)),
>  			   "DW_CFA_restore, column %#lx", r);
>      }
> +  else if (cfi->dw_cfi_opc == DW_CFA_INTERNAL_start_epilogue)
> +    ;  /* This is a nop.  */
>    else
>      {
>        dw2_asm_output_data (1, cfi->dw_cfi_opc,
> @@ -3302,6 +3330,10 @@ output_cfi_directive (dw_cfi_ref cfi)
>  	       cfi->dw_cfi_oprnd1.dw_cfi_offset);
>        break;
>
> +    case DW_CFA_INTERNAL_start_epilogue:
> +      ; /* nop.  */
> +      break;
> +
>      case DW_CFA_remember_state:
>        fprintf (asm_out_file, "\t.cfi_remember_state\n");
>        break;
> @@ -3497,6 +3529,44 @@ output_cfis (dw_cfi_ref cfi, bool do_cfi_asm,  
> dw_f
>      }
>  }
>
> +/* Output cfi skipping save/restore and epilogues in _eh frames
> +   for targets that do not want them.  */
> +
> +static dw_cfi_ref
> +emit_cfi_or_skip_epilogue (dw_cfi_ref cfi, dw_fde_ref fde, bool for_eh)
> +{
> +  if (for_eh
> +      && !flag_asynchronous_unwind_tables)
> +    {
> +      if (cfi->dw_cfi_opc == DW_CFA_remember_state)
> +	{
> +	  if (flag_debug_asm)
> +	    fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START
> +	      " DW_CFA_remember/restore_state pair skipped\n",asm_out_file);
> +	  /* Skip to the restore, unless there's an error and we fall off
> +	     the end.  */
> +	  while (cfi->dw_cfi_next
> +		 && cfi->dw_cfi_opc != DW_CFA_restore_state)
> +	    cfi = cfi->dw_cfi_next;
> +	  return cfi;
> +        }
> +      if (cfi->dw_cfi_opc == DW_CFA_INTERNAL_start_epilogue)
> +        {
> +	  if (flag_debug_asm)
> +	    fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START
> +			" DW_CFA_INTERNAL_start_epilogue\n",asm_out_file);
> +	  /* Skip to the end.  */
> +	  while (cfi->dw_cfi_next)
> +	    cfi = cfi->dw_cfi_next;
> +          return cfi;
> +        }
> +    }
> +
> +  /* If it's not a special case, then just carry on.  */
> +  output_cfi (cfi, fde, for_eh);
> +  return cfi;
> +}
> +
>  /* Output one FDE.  */
>
>  static void
> @@ -3612,13 +3682,13 @@ output_fde (dw_fde_ref fde, bool for_eh, bool  
> seco
>    fde->dw_fde_current_label = begin;
>    if (!fde->dw_fde_switched_sections)
>      for (cfi = fde->dw_fde_cfi; cfi != NULL; cfi = cfi->dw_cfi_next)
> -      output_cfi (cfi, fde, for_eh);
> +      cfi = emit_cfi_or_skip_epilogue (cfi, fde, for_eh);
>    else if (!second)
>      {
>        if (fde->dw_fde_switch_cfi)
>  	for (cfi = fde->dw_fde_cfi; cfi != NULL; cfi = cfi->dw_cfi_next)
>  	  {
> -	    output_cfi (cfi, fde, for_eh);
> +	    cfi = emit_cfi_or_skip_epilogue (cfi, fde, for_eh);
>  	    if (cfi == fde->dw_fde_switch_cfi)
>  	      break;
>  	  }
> @@ -3626,7 +3696,6 @@ output_fde (dw_fde_ref fde, bool for_eh, bool seco
>    else
>      {
>        dw_cfi_ref cfi_next = fde->dw_fde_cfi;
> -
>        if (fde->dw_fde_switch_cfi)
>  	{
>  	  cfi_next = fde->dw_fde_switch_cfi->dw_cfi_next;
> @@ -3635,7 +3704,7 @@ output_fde (dw_fde_ref fde, bool for_eh, bool seco
>  	  fde->dw_fde_switch_cfi->dw_cfi_next = cfi_next;
>  	}
>        for (cfi = cfi_next; cfi != NULL; cfi = cfi->dw_cfi_next)
> -	output_cfi (cfi, fde, for_eh);
> +	cfi = emit_cfi_or_skip_epilogue (cfi, fde, for_eh);
>      }
>
>    /* If we are to emit a ref/link from function bodies to their frame 
> tables,
> Index: gcc/toplev.c
> ===================================================================
> --- gcc/toplev.c	(revision 164304)
> +++ gcc/toplev.c	(working copy)
> @@ -1881,9 +1884,9 @@ process_options (void)
>    if (flag_rename_registers == AUTODETECT_VALUE)
>      flag_rename_registers = flag_unroll_loops || flag_peel_loops;
>
> -  if (flag_non_call_exceptions)
> +  if (flag_non_call_exceptions && flag_asynchronous_unwind_tables == 2)
>      flag_asynchronous_unwind_tables = 1;
> -  if (flag_asynchronous_unwind_tables)
> +  if (flag_asynchronous_unwind_tables || flag_non_call_exceptions)
>      flag_unwind_tables = 1;
>
>    if (flag_value_profile_transformations)
> Index: gcc/testsuite/g++.dg/eh/async-unwind2.C
> ===================================================================
> --- gcc/testsuite/g++.dg/eh/async-unwind2.C	(revision 164304)
> +++ gcc/testsuite/g++.dg/eh/async-unwind2.C	(working copy)
> @@ -2,6 +2,7 @@
>  // { dg-do run { target { { i?86-*-* x86_64-*-* } && ilp32 } } }
>  // { dg-require-effective-target fpic }
>  // { dg-options "-Os -fasynchronous-unwind-tables -fpic -fno-inline" }
> +// { dg-warning "this architecture does not fully support" "" { target 
> *-*-darwin* } 0 }
>
>  #include <stdarg.h>
>
> Index: gcc/testsuite/g++.dg/eh/async-unwind1.C
> ===================================================================
> --- gcc/testsuite/g++.dg/eh/async-unwind1.C	(revision 164304)
> +++ gcc/testsuite/g++.dg/eh/async-unwind1.C	(working copy)
> @@ -1,6 +1,7 @@
>  // PR rtl-optimization/36419
>  // { dg-do run { target { { i?86-*-* x86_64-*-* } && ilp32 } } }
>  // { dg-options "-Os -fasynchronous-unwind-tables -mpreferred-stack- 
> boundary=4" }
> +// { dg-warning "this architecture does not fully support" "" { target 
> *-*-darwin* } 0 }
>
>  extern "C" void abort ();
>
> Index: gcc/config/darwin.c
> ===================================================================
> --- gcc/config/darwin.c	(revision 164304)
> +++ gcc/config/darwin.c	(working copy)
> @@ -1869,11 +1900,25 @@ darwin_kextabi_p (void) {
>  void
>  darwin_override_options (void)
>  {
> +  bool darwin9plus = strverscmp (darwin_macosx_version_min, "10.5") >= 
> 0;
> +
>    /* Don't emit DWARF3/4 unless specifically selected.  This is a
>       workaround for tool bugs.  */
>    if (dwarf_strict < 0)
>      dwarf_strict = 1;
>
> +  /* Do not set asynchronous_unwinding (yet) unless the user  
> specifically
> +     asks, warn that it might not work (for Wall).  */
> +  if (flag_asynchronous_unwind_tables)
> +    {
> +      if (flag_asynchronous_unwind_tables == 2)
> +        flag_asynchronous_unwind_tables = 0;
> +      else
> +	warning (OPT_Wall,
> +	  "this architecture does not fully support"
> +	  " -fasynchronous-unwind-tables");
> +    }
> +
>    /* Disable -freorder-blocks-and-partition for  
> darwin_emit_unwind_label.  */
>    if (flag_reorder_blocks_and_partition
>        && (targetm.asm_out.emit_unwind_label ==  
> darwin_emit_unwind_label))
> @@ -1885,6 +1930,10 @@ darwin_override_options (void)
>        flag_reorder_blocks = 1;
>      }
>
> +  if (flag_non_call_exceptions == 1
> +      || flag_asynchronous_unwind_tables == 1)
> +    flag_unwind_tables = 1;
> +
>    if (flag_mkernel || flag_apple_kext)
>      {
>        /* -mkernel implies -fapple-kext for C++ */
> @@ -1897,18 +1946,21 @@ darwin_override_options (void)
>        flag_exceptions = 0;
>        /* No -fnon-call-exceptions data in kexts.  */
>        flag_non_call_exceptions = 0;
> +      /* so no tables either.. */
> +      flag_unwind_tables = 0;
> +      flag_asynchronous_unwind_tables = 0;
>        /* We still need to emit branch islands for kernel context.  */
>        darwin_emit_branch_islands = true;
>      }
> +
>    if (flag_var_tracking
> -      && strverscmp (darwin_macosx_version_min, "10.5") >= 0
> +      && darwin9plus
>        && debug_info_level >= DINFO_LEVEL_NORMAL
>        && debug_hooks->var_location !=  
> do_nothing_debug_hooks.var_location)
>      flag_var_tracking_uninit = 1;
>
>    /* It is assumed that branch island stubs are needed for earlier  
> systems.  */
> -  if (darwin_macosx_version_min
> -      && strverscmp (darwin_macosx_version_min, "10.5") < 0)
> +  if (! darwin9plus)
>      darwin_emit_branch_islands = true;
>  }
>
> Index: gcc/config/i386/darwin.h
> ===================================================================
> --- gcc/config/i386/darwin.h	(revision 164304)
> +++ gcc/config/i386/darwin.h	(working copy)
> @@ -253,6 +253,39 @@ extern int darwin_emit_branch_islands;
>      SUBTARGET_C_COMMON_OVERRIDE_OPTIONS;				\
>    } while (0)
>
> +#undef SUBTARGET_OVERRIDE_OPTIONS
> +#define SUBTARGET_OVERRIDE_OPTIONS \
> +do {									\
> +   bool darwin9plus = strverscmp (darwin_macosx_version_min, "10.5") >= 
> 0;\
> +  /* If no unwind  model is set, then decide on a default based 	\
> +     on Darwin rev.  */							\
> +  if (flag_omit_frame_pointer >= 1 && flag_unwind_tables == 0)		\
> +    {									\
> +      if (darwin9plus)							\
> +	flag_unwind_tables = 1;	/* Emit Unwind tables.  */		\
> +     else 								\
> +	{								\
> +	  /* The user has asked to omit fp - emit Unwind tables.  */	\
> +	  if (flag_omit_frame_pointer == 1)				\
> +	    flag_unwind_tables = 1;					\
> +	  else 								\
> +	    flag_omit_frame_pointer = 0;/* Use the frame pointer.  */	\
> +	}								\
> +    }									\
> +  /* Default frame pointers on for earlier versions of Darwin.  */	\
> +  if (flag_omit_frame_pointer == 2 && !darwin9plus)			\
> +    flag_omit_frame_pointer = 0;					\
> +  /* Do not set asynchronous_unwinding (yet) unless the user		\
> +     specifically asks.  */						\
> +  if (flag_asynchronous_unwind_tables == 2)				\
> +    flag_asynchronous_unwind_tables = 0;				\
> +  /* Jam this on to avoid a GNU-specific dwarf opcodeup setting		\
> +     Darwin10's eh frame compacter.  */					\
> +  if (flag_exceptions || flag_asynchronous_unwind_tables 		\
> +      || flag_unwind_tables)						\
> +    target_flags |= MASK_ACCUMULATE_OUTGOING_ARGS;			\
> +} while (0)
> +
>  /* Darwin on x86_64 uses dwarf-2 by default.  Pre-darwin9 32-bit
>     compiles default to stabs+.  darwin9+ defaults to dwarf-2.  */
>  #ifndef DARWIN_PREFER_DWARF

On Wed, Sep 15, 2010 at 03:23:08PM +0100, IainS wrote:
> Hi Richard,
>
> This kinda stalled ... I guess I should have pinged it.
>
> On 17 Aug 2010, at 15:57, Richard Henderson wrote:
>
>> On 08/17/2010 06:09 AM, IainS wrote:
>>> heh, I guess I meant was solving the problem you noticed above a
>>> pre-condition of applying the patch, or should they be considered
>>> separate problems?
>>
>> Let's commit them as separate patches for bi-sect-ability,
>> Just In Case.
>
>>
>>> cfi->dw_cfi_opc = DW_CFA_GNU_args_size;
>>
>> Oh, that.  If Darwin doesn't like that, then it's expecting the
>> compiler to always use -maccumulate-outgoing-args.  Just make
>> sure that gets set by default.  I suppose you'll have to do
>> something about the hand-full of test cases that force that
>> flag off.
>

Iain,
   This patch appears to regress...

FAIL: Throw_2 execution - source compiled test
FAIL: Throw_2 -findirect-dispatch execution - source compiled test
FAIL: Throw_2 -O3 execution - source compiled test
FAIL: Throw_2 -O3 -findirect-dispatch execution - source compiled test

at -m32 on x86_64-apple-darwin10...

http://gcc.gnu.org/ml/gcc-testresults/2010-09/msg01563.html

         Jack

>
> I believe that I've addressed the points made in the thread, is this now 
> OK?
>
> I (re)-draw your attention to the fact that I've removed the  
> unconditional setting of flag_asynchronous_unwind_tables when  
> flag_non_call_exceptions is set in toplev.c.
>
> if this cannot be done then, perhaps, I need to return to the original  
> idea of using a target hook to control the suppression of eh epilogues.
>
> thanks
> Iain
>
> ----
>
> amended & refreshed patch:
>
> gcc:
>
> 	* gcc/dwarf2out.c: DW_CFA_INTERNAL_start_epilogue, CFI_T New.
> 	(struct dw_cfi_struct): Adjust type of dw_cfi_opc.
> 	(add_fde_cfi):  Handle DW_CFA_INTERNAL_start_epilogue.
> 	(dwarf2out_cfi_begin_epilogue): Insert epilogue marker.
> 	(dw_cfi_oprnd1_desc): Adjust argument type.
> 	(dw_cfi_oprnd2_desc): Likewise.
> 	(output_cfi): Handle DW_CFA_INTERNAL_start_epilogue as nop.
> 	(output_cfi_directive): Likewise.
> 	(emit_cfi_or_skip_epilogue): New.
> 	(output_fde): Use emit_cfi_or_skip_epilogue.
> 	* gcc/toplev.c (process_options): Do not set  
> flag_asynchronous_unwind_tables
> 	unless it is unset by the target.  Set flag_unwind_tables for either
> 	flag_asynchronous_unwind_tables or flag_non_call_exceptions.
> 	* gcc/config/darwin.c (darwin_override_options): Warn the the target  
> does
> 	not fully support flag_asynchronous_unwind_tables.  Switch  
> flag_unwind_tables
> 	on for flag_non_call_exceptions  or flag_exceptions on darwin versions 
> supporting
> 	_eh frames.  Ensure that all table flags are switched off for kernel  
> code.
> 	* gcc/config/i386/darwin.h (SUBTARGET_OVERRIDE_OPTIONS): New, handle
> 	making a default unwinder when none is specified by the user.
>
> testsuite:
>
> 	* g++.dg/eh/async-unwind1.C:  Prune warning for Darwin.
> 	* g++.dg/eh/async-unwind2.C: Likewise.
>
>
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c	(revision 164304)
> +++ gcc/dwarf2out.c	(working copy)
> @@ -268,9 +272,17 @@ typedef union GTY(()) dw_cfi_oprnd_struct {
>  }
>  dw_cfi_oprnd;
>
> +/* Use the first out-of-band opcode number as a marker for the start of
> +   epilogues.  */
> +#define DW_CFA_INTERNAL_start_epilogue 0x100
> +#define CFI_T enum dwarf_call_frame_info
> +
> +static enum dw_cfi_oprnd_type dw_cfi_oprnd1_desc (unsigned int cfi);
> +static enum dw_cfi_oprnd_type dw_cfi_oprnd2_desc (unsigned int cfi);
> +
>  typedef struct GTY(()) dw_cfi_struct {
>    dw_cfi_ref dw_cfi_next;
> -  enum dwarf_call_frame_info dw_cfi_opc;
> +  unsigned int dw_cfi_opc;
>    dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd1_desc (%1.dw_cfi_opc)")))
>      dw_cfi_oprnd1;
>    dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd2_desc (%1.dw_cfi_opc)")))
> @@ -820,9 +832,15 @@ add_fde_cfi (const char *label, dw_cfi_ref cfi)
>      }
>
>    list_head = &cie_cfi_head;
> -
> -  if (dwarf2out_do_cfi_asm ())
> +
> +  if (cfi->dw_cfi_opc == DW_CFA_INTERNAL_start_epilogue)
>      {
> +      dw_fde_ref fde = current_fde ();
> +      gcc_assert (fde != NULL);
> +      list_head = &fde->dw_fde_cfi;
> +    }
> +  else if (dwarf2out_do_cfi_asm ())
> +    {
>        if (label)
>  	{
>  	  dw_fde_ref fde = current_fde ();
> @@ -2893,12 +2911,22 @@ dwarf2out_cfi_begin_epilogue (rtx insn)
>      return;
>
>    /* Otherwise, search forward to see if the return insn was the last
> -     basic block of the function.  If so, we don't need save/restore.  
> */
> +     basic block of the function.  If so, we don't need save/restore.
> +     However, we do mark the position so that we can skip the epilogue
> +     in _eh frames where required.  */
>    gcc_assert (i != NULL);
>    i = next_real_insn (i);
>    if (i == NULL)
> -    return;
> +    {
> +      dw_cfi_ref cfi_epi_start;
>
> +      /* Emit a marker for the epilogue start. */
> +      cfi_epi_start = new_cfi ();
> +      cfi_epi_start->dw_cfi_opc = DW_CFA_INTERNAL_start_epilogue;
> +      add_fde_cfi ("", cfi_epi_start);
> +      return;
> +    }
> +
>    /* Insert the restore before that next real insn in the stream, and 
> before
>       a potential NOTE_INSN_EPILOGUE_BEG -- we do need these notes to be
>       properly nested.  This should be after any label or alignment.   
> This
> @@ -2940,11 +2968,9 @@ dwarf2out_frame_debug_restore_state (void)
>  }
>
>  /* Describe for the GTY machinery what parts of dw_cfi_oprnd1 are used.  
> */
> -static enum dw_cfi_oprnd_type dw_cfi_oprnd1_desc
> - (enum dwarf_call_frame_info cfi);
>
>  static enum dw_cfi_oprnd_type
> -dw_cfi_oprnd1_desc (enum dwarf_call_frame_info cfi)
> +dw_cfi_oprnd1_desc (unsigned int cfi)
>  {
>    switch (cfi)
>      {
> @@ -2952,6 +2978,7 @@ static enum dw_cfi_oprnd_type
>      case DW_CFA_GNU_window_save:
>      case DW_CFA_remember_state:
>      case DW_CFA_restore_state:
> +    case DW_CFA_INTERNAL_start_epilogue:
>        return dw_cfi_oprnd_unused;
>
>      case DW_CFA_set_loc:
> @@ -2989,11 +3016,9 @@ static enum dw_cfi_oprnd_type
>  }
>
>  /* Describe for the GTY machinery what parts of dw_cfi_oprnd2 are used.  
> */
> -static enum dw_cfi_oprnd_type dw_cfi_oprnd2_desc
> - (enum dwarf_call_frame_info cfi);
>
>  static enum dw_cfi_oprnd_type
> -dw_cfi_oprnd2_desc (enum dwarf_call_frame_info cfi)
> +dw_cfi_oprnd2_desc (unsigned int cfi)
>  {
>    switch (cfi)
>      {
> @@ -3120,6 +3146,8 @@ output_cfi (dw_cfi_ref cfi, dw_fde_ref fde, int fo
>        dw2_asm_output_data (1, (cfi->dw_cfi_opc | (r & 0x3f)),
>  			   "DW_CFA_restore, column %#lx", r);
>      }
> +  else if (cfi->dw_cfi_opc == DW_CFA_INTERNAL_start_epilogue)
> +    ;  /* This is a nop.  */
>    else
>      {
>        dw2_asm_output_data (1, cfi->dw_cfi_opc,
> @@ -3302,6 +3330,10 @@ output_cfi_directive (dw_cfi_ref cfi)
>  	       cfi->dw_cfi_oprnd1.dw_cfi_offset);
>        break;
>
> +    case DW_CFA_INTERNAL_start_epilogue:
> +      ; /* nop.  */
> +      break;
> +
>      case DW_CFA_remember_state:
>        fprintf (asm_out_file, "\t.cfi_remember_state\n");
>        break;
> @@ -3497,6 +3529,44 @@ output_cfis (dw_cfi_ref cfi, bool do_cfi_asm,  
> dw_f
>      }
>  }
>
> +/* Output cfi skipping save/restore and epilogues in _eh frames
> +   for targets that do not want them.  */
> +
> +static dw_cfi_ref
> +emit_cfi_or_skip_epilogue (dw_cfi_ref cfi, dw_fde_ref fde, bool for_eh)
> +{
> +  if (for_eh
> +      && !flag_asynchronous_unwind_tables)
> +    {
> +      if (cfi->dw_cfi_opc == DW_CFA_remember_state)
> +	{
> +	  if (flag_debug_asm)
> +	    fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START
> +	      " DW_CFA_remember/restore_state pair skipped\n",asm_out_file);
> +	  /* Skip to the restore, unless there's an error and we fall off
> +	     the end.  */
> +	  while (cfi->dw_cfi_next
> +		 && cfi->dw_cfi_opc != DW_CFA_restore_state)
> +	    cfi = cfi->dw_cfi_next;
> +	  return cfi;
> +        }
> +      if (cfi->dw_cfi_opc == DW_CFA_INTERNAL_start_epilogue)
> +        {
> +	  if (flag_debug_asm)
> +	    fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START
> +			" DW_CFA_INTERNAL_start_epilogue\n",asm_out_file);
> +	  /* Skip to the end.  */
> +	  while (cfi->dw_cfi_next)
> +	    cfi = cfi->dw_cfi_next;
> +          return cfi;
> +        }
> +    }
> +
> +  /* If it's not a special case, then just carry on.  */
> +  output_cfi (cfi, fde, for_eh);
> +  return cfi;
> +}
> +
>  /* Output one FDE.  */
>
>  static void
> @@ -3612,13 +3682,13 @@ output_fde (dw_fde_ref fde, bool for_eh, bool  
> seco
>    fde->dw_fde_current_label = begin;
>    if (!fde->dw_fde_switched_sections)
>      for (cfi = fde->dw_fde_cfi; cfi != NULL; cfi = cfi->dw_cfi_next)
> -      output_cfi (cfi, fde, for_eh);
> +      cfi = emit_cfi_or_skip_epilogue (cfi, fde, for_eh);
>    else if (!second)
>      {
>        if (fde->dw_fde_switch_cfi)
>  	for (cfi = fde->dw_fde_cfi; cfi != NULL; cfi = cfi->dw_cfi_next)
>  	  {
> -	    output_cfi (cfi, fde, for_eh);
> +	    cfi = emit_cfi_or_skip_epilogue (cfi, fde, for_eh);
>  	    if (cfi == fde->dw_fde_switch_cfi)
>  	      break;
>  	  }
> @@ -3626,7 +3696,6 @@ output_fde (dw_fde_ref fde, bool for_eh, bool seco
>    else
>      {
>        dw_cfi_ref cfi_next = fde->dw_fde_cfi;
> -
>        if (fde->dw_fde_switch_cfi)
>  	{
>  	  cfi_next = fde->dw_fde_switch_cfi->dw_cfi_next;
> @@ -3635,7 +3704,7 @@ output_fde (dw_fde_ref fde, bool for_eh, bool seco
>  	  fde->dw_fde_switch_cfi->dw_cfi_next = cfi_next;
>  	}
>        for (cfi = cfi_next; cfi != NULL; cfi = cfi->dw_cfi_next)
> -	output_cfi (cfi, fde, for_eh);
> +	cfi = emit_cfi_or_skip_epilogue (cfi, fde, for_eh);
>      }
>
>    /* If we are to emit a ref/link from function bodies to their frame 
> tables,
> Index: gcc/toplev.c
> ===================================================================
> --- gcc/toplev.c	(revision 164304)
> +++ gcc/toplev.c	(working copy)
> @@ -1881,9 +1884,9 @@ process_options (void)
>    if (flag_rename_registers == AUTODETECT_VALUE)
>      flag_rename_registers = flag_unroll_loops || flag_peel_loops;
>
> -  if (flag_non_call_exceptions)
> +  if (flag_non_call_exceptions && flag_asynchronous_unwind_tables == 2)
>      flag_asynchronous_unwind_tables = 1;
> -  if (flag_asynchronous_unwind_tables)
> +  if (flag_asynchronous_unwind_tables || flag_non_call_exceptions)
>      flag_unwind_tables = 1;
>
>    if (flag_value_profile_transformations)
> Index: gcc/testsuite/g++.dg/eh/async-unwind2.C
> ===================================================================
> --- gcc/testsuite/g++.dg/eh/async-unwind2.C	(revision 164304)
> +++ gcc/testsuite/g++.dg/eh/async-unwind2.C	(working copy)
> @@ -2,6 +2,7 @@
>  // { dg-do run { target { { i?86-*-* x86_64-*-* } && ilp32 } } }
>  // { dg-require-effective-target fpic }
>  // { dg-options "-Os -fasynchronous-unwind-tables -fpic -fno-inline" }
> +// { dg-warning "this architecture does not fully support" "" { target 
> *-*-darwin* } 0 }
>
>  #include <stdarg.h>
>
> Index: gcc/testsuite/g++.dg/eh/async-unwind1.C
> ===================================================================
> --- gcc/testsuite/g++.dg/eh/async-unwind1.C	(revision 164304)
> +++ gcc/testsuite/g++.dg/eh/async-unwind1.C	(working copy)
> @@ -1,6 +1,7 @@
>  // PR rtl-optimization/36419
>  // { dg-do run { target { { i?86-*-* x86_64-*-* } && ilp32 } } }
>  // { dg-options "-Os -fasynchronous-unwind-tables -mpreferred-stack- 
> boundary=4" }
> +// { dg-warning "this architecture does not fully support" "" { target 
> *-*-darwin* } 0 }
>
>  extern "C" void abort ();
>
> Index: gcc/config/darwin.c
> ===================================================================
> --- gcc/config/darwin.c	(revision 164304)
> +++ gcc/config/darwin.c	(working copy)
> @@ -1869,11 +1900,25 @@ darwin_kextabi_p (void) {
>  void
>  darwin_override_options (void)
>  {
> +  bool darwin9plus = strverscmp (darwin_macosx_version_min, "10.5") >= 
> 0;
> +
>    /* Don't emit DWARF3/4 unless specifically selected.  This is a
>       workaround for tool bugs.  */
>    if (dwarf_strict < 0)
>      dwarf_strict = 1;
>
> +  /* Do not set asynchronous_unwinding (yet) unless the user  
> specifically
> +     asks, warn that it might not work (for Wall).  */
> +  if (flag_asynchronous_unwind_tables)
> +    {
> +      if (flag_asynchronous_unwind_tables == 2)
> +        flag_asynchronous_unwind_tables = 0;
> +      else
> +	warning (OPT_Wall,
> +	  "this architecture does not fully support"
> +	  " -fasynchronous-unwind-tables");
> +    }
> +
>    /* Disable -freorder-blocks-and-partition for  
> darwin_emit_unwind_label.  */
>    if (flag_reorder_blocks_and_partition
>        && (targetm.asm_out.emit_unwind_label ==  
> darwin_emit_unwind_label))
> @@ -1885,6 +1930,10 @@ darwin_override_options (void)
>        flag_reorder_blocks = 1;
>      }
>
> +  if (flag_non_call_exceptions == 1
> +      || flag_asynchronous_unwind_tables == 1)
> +    flag_unwind_tables = 1;
> +
>    if (flag_mkernel || flag_apple_kext)
>      {
>        /* -mkernel implies -fapple-kext for C++ */
> @@ -1897,18 +1946,21 @@ darwin_override_options (void)
>        flag_exceptions = 0;
>        /* No -fnon-call-exceptions data in kexts.  */
>        flag_non_call_exceptions = 0;
> +      /* so no tables either.. */
> +      flag_unwind_tables = 0;
> +      flag_asynchronous_unwind_tables = 0;
>        /* We still need to emit branch islands for kernel context.  */
>        darwin_emit_branch_islands = true;
>      }
> +
>    if (flag_var_tracking
> -      && strverscmp (darwin_macosx_version_min, "10.5") >= 0
> +      && darwin9plus
>        && debug_info_level >= DINFO_LEVEL_NORMAL
>        && debug_hooks->var_location !=  
> do_nothing_debug_hooks.var_location)
>      flag_var_tracking_uninit = 1;
>
>    /* It is assumed that branch island stubs are needed for earlier  
> systems.  */
> -  if (darwin_macosx_version_min
> -      && strverscmp (darwin_macosx_version_min, "10.5") < 0)
> +  if (! darwin9plus)
>      darwin_emit_branch_islands = true;
>  }
>
> Index: gcc/config/i386/darwin.h
> ===================================================================
> --- gcc/config/i386/darwin.h	(revision 164304)
> +++ gcc/config/i386/darwin.h	(working copy)
> @@ -253,6 +253,39 @@ extern int darwin_emit_branch_islands;
>      SUBTARGET_C_COMMON_OVERRIDE_OPTIONS;				\
>    } while (0)
>
> +#undef SUBTARGET_OVERRIDE_OPTIONS
> +#define SUBTARGET_OVERRIDE_OPTIONS \
> +do {									\
> +   bool darwin9plus = strverscmp (darwin_macosx_version_min, "10.5") >= 
> 0;\
> +  /* If no unwind  model is set, then decide on a default based 	\
> +     on Darwin rev.  */							\
> +  if (flag_omit_frame_pointer >= 1 && flag_unwind_tables == 0)		\
> +    {									\
> +      if (darwin9plus)							\
> +	flag_unwind_tables = 1;	/* Emit Unwind tables.  */		\
> +     else 								\
> +	{								\
> +	  /* The user has asked to omit fp - emit Unwind tables.  */	\
> +	  if (flag_omit_frame_pointer == 1)				\
> +	    flag_unwind_tables = 1;					\
> +	  else 								\
> +	    flag_omit_frame_pointer = 0;/* Use the frame pointer.  */	\
> +	}								\
> +    }									\
> +  /* Default frame pointers on for earlier versions of Darwin.  */	\
> +  if (flag_omit_frame_pointer == 2 && !darwin9plus)			\
> +    flag_omit_frame_pointer = 0;					\
> +  /* Do not set asynchronous_unwinding (yet) unless the user		\
> +     specifically asks.  */						\
> +  if (flag_asynchronous_unwind_tables == 2)				\
> +    flag_asynchronous_unwind_tables = 0;				\
> +  /* Jam this on to avoid a GNU-specific dwarf opcodeup setting		\
> +     Darwin10's eh frame compacter.  */					\
> +  if (flag_exceptions || flag_asynchronous_unwind_tables 		\
> +      || flag_unwind_tables)						\
> +    target_flags |= MASK_ACCUMULATE_OUTGOING_ARGS;			\
> +} while (0)
> +
>  /* Darwin on x86_64 uses dwarf-2 by default.  Pre-darwin9 32-bit
>     compiles default to stabs+.  darwin9+ defaults to dwarf-2.  */
>  #ifndef DARWIN_PREFER_DWARF

On 17 Sep 2010, at 14:31, Jack Howarth wrote:

> On Wed, Sep 15, 2010 at 03:23:08PM +0100, IainS wrote:
>> Hi Richard,
>>
>> This kinda stalled ... I guess I should have pinged it.
>>
>> On 17 Aug 2010, at 15:57, Richard Henderson wrote:
>>
>>> On 08/17/2010 06:09 AM, IainS wrote:
>>>> heh, I guess I meant was solving the problem you noticed above a
>>>> pre-condition of applying the patch, or should they be considered
>>>> separate problems?
>>>
>>> Let's commit them as separate patches for bi-sect-ability,
>>> Just In Case.
>>
>>>
>>>> cfi->dw_cfi_opc = DW_CFA_GNU_args_size;
>>>
>>> Oh, that.  If Darwin doesn't like that, then it's expecting the
>>> compiler to always use -maccumulate-outgoing-args.  Just make
>>> sure that gets set by default.  I suppose you'll have to do
>>> something about the hand-full of test cases that force that
>>> flag off.
>>
>
> Iain,
>   This patch appears to regress...
>
> FAIL: Throw_2 execution - source compiled test
> FAIL: Throw_2 -findirect-dispatch execution - source compiled test
> FAIL: Throw_2 -O3 execution - source compiled test
> FAIL: Throw_2 -O3 -findirect-dispatch execution - source compiled test
>
> at -m32 on x86_64-apple-darwin10...

I do not see these on x86_64-darwin10.4 (with the already approved  
libjava -lm spec patch applied).

---

I see Thread_Sleep_2 fails on darwin10 (both patched and unpatched  
trunk).
The Thread_Sleep_2 fails are not present on i686-darwin9
(although they seem to appear randomly on ppc-darwin9, suggesting a  
race or initialization issue).

Iain