public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [Patch, _eh, dawin]  Allow targets to suppress epilogues in _eh frames.
@ 2010-08-13 20:34 IainS
  2010-08-14  0:10 ` Richard Henderson
  2010-08-14  4:23 ` [Patch, _eh, dawin] " Jack Howarth
  0 siblings, 2 replies; 22+ messages in thread
From: IainS @ 2010-08-13 20:34 UTC (permalink / raw)
  To: GCC Patches; +Cc: Richard Henderson, mrs

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

As discussed off list and in  IRC with Richard,

=----=

Some (if not most) targets do not require the function epilogue in  
their _eh unwind frames.
In fact, it breaks the darwin unwinder (PR41991) my original  
motivation for looking at this.

However, as we went through the discussion it became apparent that  
this might have wider application than fixing a darwin bug, in saving  
some space in the eh.

What this does is to use the existing DW_CFA_save/restore_state and a  
new DW_CFA_GNU_start_epilogue marker to suppress the emitting of  
function epilogues - when (a) we're emitting _eh and (b) the target  
requests suppression via a hook.
The hook is a bool function to permit targets to choose whether to  
emit this data or not at run time rather than config time (the default  
hook does nothing).

----

The DW_CFA_GNU_start_epilogue marker is inserted under the  
circumstance that an epilogue is detected at the end of a function.
The save/restore markers are not touched and deal with the case that  
we are mid-function.

There is a  debug print of  # DW_CFA_GNU_start_epilogue to show where  
we are intercepting in the eh frames and not curtailing the  
debug_frames.

I'd particularly welcome someone's eye over what's happening in the  
case of section switches (it seems to me that the skipping of mid- 
function save/restore is handled OK, but I'm not familiar with that  
code - and my main target(s) don't use that facility).

====

This has only been lightly tested on i686/powerpc-darwin9 (regtested  
on i686) - but it appears to restore Unwind functionality to the  
platform :)
[gcj works again, Yay!]

====

so we get this in the _eh frame:

LECIE1:
	.globl _main.eh
_main.eh:
LSFDE1:
	.set L$set$1,LEFDE1-LASFDE1
	.long L$set$1	# FDE Length
LASFDE1:
	.long	LASFDE1-EH_frame1	# FDE CIE offset
	.long	LFB1-.	# FDE initial location
	.set L$set$2,LFE1-LFB1

<SNIP>

	.set L$set$5,LCFI4-LCFI1
	.long L$set$5
	.byte	0x83	# DW_CFA_offset, column 0x3
	.byte	0x3	# uleb128 0x3
	.byte	0x4	# DW_CFA_advance_loc4
	.set L$set$6,LCFI6-LCFI4
	.long L$set$6
#			# DW_CFA_GNU_start_epilogue
	.align 2

and this in the _debug_frame:
	.section __DWARF,__debug_frame,regular,debug
Lsection__debug_frame:
Lframe0:
	.set L$set$7,LECIE0-LSCIE0
	.long L$set$7	# Length of Common Information Entry
LSCIE0:

<SNIP>

	.byte	0x83	# DW_CFA_offset, column 0x3
	.byte	0x3	# uleb128 0x3
	.byte	0x4	# DW_CFA_advance_loc4
	.set L$set$14,LCFI6-LCFI4
	.long L$set$14
#			# DW_CFA_GNU_start_epilogue
	.byte	0xc3	# DW_CFA_restore, column 0x3
	.byte	0x4	# DW_CFA_advance_loc4
	.set L$set$15,LCFI7-LCFI6
	.long L$set$15
	.byte	0xc	# DW_CFA_def_cfa
	.byte	0x4	# uleb128 0x4
	.byte	0x4	# uleb128 0x4
	.byte	0xc5	# DW_CFA_restore, column 0x5
	.align 2
LEFDE2:

=====
thoughts?
Iain


Change notes (not a proper changelog, at this juncture)

include/dwarf2.h : DW_CFA_GNU_start_epilogue new enum.

dwarf2out.c: dwarf_cfi_name() recognize DW_CFA_GNU_start_epilogue
static scope emit_cfa_start_epilogue new var.
add_fde_cfi() : emit a marker for the epilogue start;
dwarf2out_cfi_begin_epilogue (): note that we need to emit the  
epilogue start marker when the epilogue is at the end.
dw_cfi_oprnd1_desc (): recognize DW_CFA_GNU_start_epilogue as a no-op.
output_cfi(): print debug message for DW_CFA_GNU_start_epilogue
emit_cfi_or_skip_epilogue (): New.
output_fde () : use emit_cfi_or_skip_epilogue ();

target.def: suppress_eh_epilogue_p (): New ASM Hook.

=== the remainder are the implementation on the darwin side:

gcc/config/darwin.h (TARGET_ASM_SUPPRESS_EH_EPILOGUE_P): New
gcc/config/darwin.c (darwin_asm_suppress_eh_epilogue_p): New.
gcc/config/darwin-protos.h: Declare darwin_asm_suppress_eh_epilogue_p.

========== - ===========

[-- Attachment #2: 163221-eh-epilogue.txt --]
[-- Type: text/plain, Size: 10000 bytes --]

Index: include/dwarf2.h
===================================================================
--- include/dwarf2.h	(revision 163221)
+++ include/dwarf2.h	(working copy)
@@ -854,7 +854,8 @@ enum dwarf_call_frame_info
     /* GNU extensions.  */
     DW_CFA_GNU_window_save = 0x2d,
     DW_CFA_GNU_args_size = 0x2e,
-    DW_CFA_GNU_negative_offset_extended = 0x2f
+    DW_CFA_GNU_negative_offset_extended = 0x2f,
+    DW_CFA_GNU_start_epilogue = 0x30
   };
 
 #define DW_CIE_ID	  0xffffffff
Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 163221)
+++ gcc/dwarf2out.c	(working copy)
@@ -720,7 +724,9 @@ dwarf_cfi_name (unsigned int cfi_opc)
       return "DW_CFA_GNU_args_size";
     case DW_CFA_GNU_negative_offset_extended:
       return "DW_CFA_GNU_negative_offset_extended";
-
+    case DW_CFA_GNU_start_epilogue:
+      return "DW_CFA_GNU_start_epilogue";
+      
     default:
       return "DW_CFA_<unknown>";
     }
@@ -801,6 +807,9 @@ dwarf2out_cfi_label (bool force)
 /* True if remember_state should be emitted before following CFI directive.  */
 static bool emit_cfa_remember;
 
+/* True if start_epilogue should be emitted before following CFI directive.  */
+static bool emit_cfa_start_epilogue;
+
 /* Add CFI to the current fde at the PC value indicated by LABEL if specified,
    or to the CIE if LABEL is NULL.  */
 
@@ -809,6 +818,17 @@ add_fde_cfi (const char *label, dw_cfi_ref cfi)
 {
   dw_cfi_ref *list_head;
 
+  if (emit_cfa_start_epilogue)
+    {
+      dw_cfi_ref cfi_epi_start;
+
+      /* Emit the state save.  */
+      emit_cfa_start_epilogue = false;
+      cfi_epi_start = new_cfi ();
+      cfi_epi_start->dw_cfi_opc = DW_CFA_GNU_start_epilogue;
+      add_fde_cfi (label, cfi_epi_start);   
+    }
+    
   if (emit_cfa_remember)
     {
       dw_cfi_ref cfi_remember;
@@ -2898,7 +2918,12 @@ dwarf2out_cfi_begin_epilogue (rtx insn)
   gcc_assert (i != NULL);
   i = next_real_insn (i);
   if (i == NULL)
-    return;
+    {
+      /* But we do mark the start of the epilogue to allow it to be skipped
+         in _eh frames.  */
+      emit_cfa_start_epilogue = true; 
+      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
@@ -2953,6 +2978,7 @@ dw_cfi_oprnd1_desc (enum dwarf_call_frame_info cfi
     case DW_CFA_GNU_window_save:
     case DW_CFA_remember_state:
     case DW_CFA_restore_state:
+    case DW_CFA_GNU_start_epilogue:
       return dw_cfi_oprnd_unused;
 
     case DW_CFA_set_loc:
@@ -3121,6 +3148,10 @@ 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_GNU_start_epilogue)
+/* DEBUG */
+    fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START
+    		" DW_CFA_GNU_start_epilogue\n",asm_out_file);
   else
     {
       dw2_asm_output_data (1, cfi->dw_cfi_opc,
@@ -3303,6 +3334,12 @@ output_cfi_directive (dw_cfi_ref cfi)
 	       cfi->dw_cfi_oprnd1.dw_cfi_offset);
       break;
 
+    case DW_CFA_GNU_start_epilogue:
+/*DEBUG */
+    fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START
+    		" DW_CFA_GNU_start_epilogue\n",asm_out_file);
+      break;
+      
     case DW_CFA_remember_state:
       fprintf (asm_out_file, "\t.cfi_remember_state\n");
       break;
@@ -3498,6 +3535,41 @@ 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 
+      && targetm.asm_out.suppress_eh_epilogue_p())
+    {
+      if (cfi->dw_cfi_opc == DW_CFA_remember_state)
+	{
+	  /* 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_GNU_start_epilogue)
+        {
+/*DEBUG */
+    fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START
+    		" DW_CFA_GNU_start_epilogue\n",asm_out_file);
+	  while (cfi->dw_cfi_next) 
+	    /* Skip to the end.  */
+	    cfi = cfi->dw_cfi_next;
+          return cfi;
+        }
+    }
+
+  /* if it's not a special case, then just emit it.  */
+  output_cfi (cfi, fde, for_eh);
+  return cfi;
+}
+
 /* Output one FDE.  */
 
 static void
@@ -3613,13 +3685,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;
 	  }
@@ -3636,7 +3707,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/target.def
===================================================================
--- gcc/target.def	(revision 163221)
+++ gcc/target.def	(working copy)
@@ -390,6 +406,15 @@ DEFHOOK
  void, (FILE *file, int size, rtx x),
  NULL)
 
+/* Targets might not need epilogue information in dwarf2 _eh frames.  This
+   hook should return true if the epilogue should be suppressed in such frames.
+   Epilogues will still be emitted in _debug_frames.  */
+DEFHOOK_UNDOC
+(suppress_eh_epilogue_p,
+ "",
+ bool, (void),
+ hook_bool_void_false)
+
 /* Some target machines need to postscan each insn after it is output.  */
 DEFHOOK
 (final_postscan_insn,
Index: gcc/config/darwin-protos.h
===================================================================
--- gcc/config/darwin-protos.h	(revision 163221)
+++ gcc/config/darwin-protos.h	(working copy)
@@ -83,10 +84,15 @@ extern tree darwin_handle_weak_import_attribute (t
 extern void machopic_output_stub (FILE *, const char *, const char *);
 extern void darwin_globalize_label (FILE *, const char *);
 extern void darwin_assemble_visibility (tree, int);
+
+extern bool darwin_asm_suppress_eh_epilogue_p (void);
+extern void darwin_asm_output_dwarf_section_start_label (FILE *file, 
+							section *sect);
 extern void darwin_asm_output_dwarf_delta (FILE *, int, const char *,
 					   const char *);
 extern void darwin_asm_output_dwarf_offset (FILE *, int, const char *,
 					    section *);
+
 extern void darwin_asm_declare_constant_name (FILE *, const char *,
 					      const_tree, HOST_WIDE_INT);
 extern bool darwin_binds_local_p (const_tree);
Index: gcc/config/darwin.h
===================================================================
--- gcc/config/darwin.h	(revision 163221)
+++ gcc/config/darwin.h	(working copy)
@@ -669,7 +675,6 @@ extern GTY(()) int darwin_ms_struct;
    Make Objective-C internal symbols local and in doing this, we need 
    to accommodate the name mangling done by c++ on file scope locals.  */
 
-
 int darwin_label_is_anonymous_local_objc_name (const char *name);
 
 #undef	ASM_OUTPUT_LABELREF
@@ -927,6 +932,16 @@ enum machopic_addr_class {
    ? (DW_EH_PE_pcrel | DW_EH_PE_indirect | DW_EH_PE_sdata4) : \
      ((CODE) == 1 || (GLOBAL) == 0) ? DW_EH_PE_pcrel : DW_EH_PE_absptr)
 
+/* Mark the start of each dwarf debug section to allow us to compute local
+   offsets within the sections.  We do this in darwin, rather than emitting
+   relocs.  */
+#define TARGET_ASM_OUTPUT_DWARF_SECTION_START_LABEL \
+	darwin_asm_output_dwarf_section_start_label
+
+/* For OSX compatibility we do not want to emit epilogues in _eh frames.  */
+#define TARGET_ASM_SUPPRESS_EH_EPILOGUE_P \
+	darwin_asm_suppress_eh_epilogue_p
+
 #define ASM_OUTPUT_DWARF_DELTA(FILE,SIZE,LABEL1,LABEL2)  \
   darwin_asm_output_dwarf_delta (FILE, SIZE, LABEL1, LABEL2)
 
Index: gcc/config/darwin.c
===================================================================
--- gcc/config/darwin.c	(revision 163221)
+++ gcc/config/darwin.c	(working copy)
@@ -1666,6 +1666,36 @@ darwin_assemble_visibility (tree decl, int vis)
 	     "not supported in this configuration; ignored");
 }
 
+/* For compatibility with OSX versions that do not emit epilogues in _eh
+   frames we suppress them.  This is made a predicate function to permit 
+   us to add an OSX/FSF compatibility switch should that be required.  */
+
+bool
+darwin_asm_suppress_eh_epilogue_p (void)
+{
+  return true;
+}
+
+/* So that we can compute dwarf offsets within sections, we emit a known
+   section marker at the begining of the section.  This is distinct from
+   the ones emitted by dwarf2out.  The label is constructed by extracting
+   sectname from __DWARF,__sectname,etc,etc.  The hook should be invoked
+   once, after the first switch to the section.  */
+   
+void
+darwin_asm_output_dwarf_section_start_label (FILE *file, section *sect)
+{
+  const char *dnam;
+  int namelen;
+  gcc_assert (sect && (sect->common.flags & (SECTION_NAMED|SECTION_DEBUG)));
+  dnam = ((struct named_section *)sect)->name;
+  gcc_assert (strncmp (dnam, "__DWARF,", 8) == 0);
+  gcc_assert (strchr (dnam + 8, ','));
+
+  namelen = strchr (dnam + 8, ',') - (dnam + 8);
+  fprintf (file, "Lsection%.*s:\n", namelen, dnam + 8);
+}
+
 /* Output a difference of two labels that will be an assembly time
    constant if the two labels are local.  (.long lab1-lab2 will be
    very different if lab1 is at the boundary between two sections; it

[-- Attachment #3: Type: text/plain, Size: 5 bytes --]







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

* Re: [Patch, _eh, dawin]  Allow targets to suppress epilogues in _eh frames.
  2010-08-13 20:34 [Patch, _eh, dawin] Allow targets to suppress epilogues in _eh frames IainS
@ 2010-08-14  0:10 ` Richard Henderson
  2010-08-15 20:03   ` [Patch, _eh, dawin, Version2] " IainS
  2010-08-14  4:23 ` [Patch, _eh, dawin] " Jack Howarth
  1 sibling, 1 reply; 22+ messages in thread
From: Richard Henderson @ 2010-08-14  0:10 UTC (permalink / raw)
  To: IainS; +Cc: GCC Patches, mrs

On 08/13/2010 12:50 PM, IainS wrote:
> +    DW_CFA_GNU_start_epilogue = 0x30

I'd rather not place this in dwarf2.h since we don't plan to emit this
as an actual opcode.  It only needs to be a marker in the dw_cfi_ref
linked list for internal use of gcc.

As such I think something like

# define DW_CFA_INTERNAL_start_epilogue  0x100

would be appropriate.  I believe you'll need to change

  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;

in order for this to be legal.  Unfortunately that may
require a host of other type changes or type casts in
order to pass the C++ warning mode.

> +/* True if start_epilogue should be emitted before following CFI directive.  */
> +static bool emit_cfa_start_epilogue;

I don't think this is needed...

>    if (i == NULL)
> -    return;
> +    {
> +      /* But we do mark the start of the epilogue to allow it to be skipped
> +         in _eh frames.  */
> +      emit_cfa_start_epilogue = true; 
> +      return;
> +    }

... instead we want to add the cfi entry here, by hand.  The reason
being that we want to avoid the DW_CFA_advance_loc4 that would be
added by actually inserting this opcode via add_fde_cfi.

> +/* Targets might not need epilogue information in dwarf2 _eh frames.  This
> +   hook should return true if the epilogue should be suppressed in such frames.
> +   Epilogues will still be emitted in _debug_frames.  */
> +DEFHOOK_UNDOC
> +(suppress_eh_epilogue_p,
> + "",
> + bool, (void),
> + hook_bool_void_false)

As discussed on IRC, I don't think we need a target hook for this.  We
should key this off of !flag_asynchronous_unwind_tables.  That will
allow all targets that only need unwind info for call sites omit this
extra epilogue unwind info.

At that point you can adjust Darwin's override_options hook(s) to make 
sure that this flag is appropriately off.  It also seems like it would
be a good idea to add a SPEC entry to disable compact unwind if the
user explicitly uses -fasynchronous-unwind-info.

> +/* Mark the start of each dwarf debug section to allow us to compute local
> +   offsets within the sections.  We do this in darwin, rather than emitting
> +   relocs.  */
> +#define TARGET_ASM_OUTPUT_DWARF_SECTION_START_LABEL \
> +	darwin_asm_output_dwarf_section_start_label

Is this an unrelated change?  I can't see how it relates to the rest.


r~

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

* Re: [Patch, _eh, dawin]  Allow targets to suppress epilogues in _eh frames.
  2010-08-13 20:34 [Patch, _eh, dawin] Allow targets to suppress epilogues in _eh frames IainS
  2010-08-14  0:10 ` Richard Henderson
@ 2010-08-14  4:23 ` Jack Howarth
  2010-08-14 15:21   ` IainS
  1 sibling, 1 reply; 22+ messages in thread
From: Jack Howarth @ 2010-08-14  4:23 UTC (permalink / raw)
  To: IainS; +Cc: GCC Patches, Richard Henderson, mrs

On Fri, Aug 13, 2010 at 08:50:29PM +0100, IainS wrote:
> As discussed off list and in  IRC with Richard,
>
> =----=
>
> Some (if not most) targets do not require the function epilogue in their 
> _eh unwind frames.
> In fact, it breaks the darwin unwinder (PR41991) my original motivation 
> for looking at this.
>
> However, as we went through the discussion it became apparent that this 
> might have wider application than fixing a darwin bug, in saving some 
> space in the eh.
>
> What this does is to use the existing DW_CFA_save/restore_state and a  
> new DW_CFA_GNU_start_epilogue marker to suppress the emitting of  
> function epilogues - when (a) we're emitting _eh and (b) the target  
> requests suppression via a hook.
> The hook is a bool function to permit targets to choose whether to emit 
> this data or not at run time rather than config time (the default hook 
> does nothing).
>
> ----
>
> The DW_CFA_GNU_start_epilogue marker is inserted under the circumstance 
> that an epilogue is detected at the end of a function.
> The save/restore markers are not touched and deal with the case that we 
> are mid-function.
>
> There is a  debug print of  # DW_CFA_GNU_start_epilogue to show where we 
> are intercepting in the eh frames and not curtailing the debug_frames.
>
> I'd particularly welcome someone's eye over what's happening in the case 
> of section switches (it seems to me that the skipping of mid-function 
> save/restore is handled OK, but I'm not familiar with that code - and my 
> main target(s) don't use that facility).
>
> ====
>
> This has only been lightly tested on i686/powerpc-darwin9 (regtested on 
> i686) - but it appears to restore Unwind functionality to the platform :)
> [gcj works again, Yay!]
>
> ====
>
> so we get this in the _eh frame:
>
> LECIE1:
> 	.globl _main.eh
> _main.eh:
> LSFDE1:
> 	.set L$set$1,LEFDE1-LASFDE1
> 	.long L$set$1	# FDE Length
> LASFDE1:
> 	.long	LASFDE1-EH_frame1	# FDE CIE offset
> 	.long	LFB1-.	# FDE initial location
> 	.set L$set$2,LFE1-LFB1
>
> <SNIP>
>
> 	.set L$set$5,LCFI4-LCFI1
> 	.long L$set$5
> 	.byte	0x83	# DW_CFA_offset, column 0x3
> 	.byte	0x3	# uleb128 0x3
> 	.byte	0x4	# DW_CFA_advance_loc4
> 	.set L$set$6,LCFI6-LCFI4
> 	.long L$set$6
> #			# DW_CFA_GNU_start_epilogue
> 	.align 2
>
> and this in the _debug_frame:
> 	.section __DWARF,__debug_frame,regular,debug
> Lsection__debug_frame:
> Lframe0:
> 	.set L$set$7,LECIE0-LSCIE0
> 	.long L$set$7	# Length of Common Information Entry
> LSCIE0:
>
> <SNIP>
>
> 	.byte	0x83	# DW_CFA_offset, column 0x3
> 	.byte	0x3	# uleb128 0x3
> 	.byte	0x4	# DW_CFA_advance_loc4
> 	.set L$set$14,LCFI6-LCFI4
> 	.long L$set$14
> #			# DW_CFA_GNU_start_epilogue
> 	.byte	0xc3	# DW_CFA_restore, column 0x3
> 	.byte	0x4	# DW_CFA_advance_loc4
> 	.set L$set$15,LCFI7-LCFI6
> 	.long L$set$15
> 	.byte	0xc	# DW_CFA_def_cfa
> 	.byte	0x4	# uleb128 0x4
> 	.byte	0x4	# uleb128 0x4
> 	.byte	0xc5	# DW_CFA_restore, column 0x5
> 	.align 2
> LEFDE2:
>
> =====
> thoughts?
> Iain
>

Iain,
   So far the only failure I see with your patch under darwin10.4.0, using the
compact unwinder (by eliminating the addition of -no_compact_unwind
in darwin10.h), is...

FAIL: g++.dg/eh/async-unwind2.C execution test

at -m32. Interestingly, if I pass -Wl,-warn_compact_unwind when building
that testcase, I get...

[MacPro:~/async_unwind] howarth% /sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/gcc/testsuite/g++/../../g++ -B/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/gcc/testsuite/g++/../../ /sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100813/gcc/testsuite/g++.dg/eh/async-unwind2.C -nostdinc++ -I/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/x86_64-apple-darwin10.4.0/i386/libstdc++-v3/include/x86_64-apple-darwin10.4.0 -I/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/x86_64-apple-darwin10.4.0/i386/libstdc++-v3/include -I/sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100813/libstdc++-v3/libsupc++ -I/sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100813/libstdc++-v3/include/backward -I/sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100813/libstdc++-v3/testsuite/util -fmessage-length=0 -Os -fasynchronous-unwind-tables -fpic -fno-inline -L/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/x86_64-apple-darwin10.4.0/i386/libstdc++-v3/src/.libs -B/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/x86_64-apple-darwin10.4.0/i386/libstdc++-v3/src/.libs -L/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/x86_64-apple-darwin10.4.0/i386/libstdc++-v3/src/.libs -L/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/x86_64-apple-darwin10.4.0/i386/libiberty -multiply_defined suppress -lm -m32 -g -Wl,-warn_compact_unwind -o ./async-unwind2.exe
ld: warning: can't make compact unwind encoding from dwarf for S::sfn2(int)  in /var/folders/1C/1CdoNxmNFHyOIjNBLNuJh++++TM/-Tmp-//ccw4LTKt.o because dwarf uses DW_CFA_GNU_args_size
ld: warning: can't make compact unwind encoding from dwarf for S::sfn3(char const*) in /var/folders/1C/1CdoNxmNFHyOIjNBLNuJh++++TM/-Tmp-//ccw4LTKt.o because dwarf uses DW_CFA_GNU_args_size
ld: warning: can't make compact unwind encoding from dwarf for baz1(S const&) in /var/folders/1C/1CdoNxmNFHyOIjNBLNuJh++++TM/-Tmp-//ccw4LTKt.o because dwarf uses DW_CFA_GNU_args_size

These errors don't appear when I do the same to build the non-failing async-unwind1.exe testcase. Can we inhibit the emission of
DW_CFA_GNU_args_size on darwin as well?
                Jack


>
> Change notes (not a proper changelog, at this juncture)
>
> include/dwarf2.h : DW_CFA_GNU_start_epilogue new enum.
>
> dwarf2out.c: dwarf_cfi_name() recognize DW_CFA_GNU_start_epilogue
> static scope emit_cfa_start_epilogue new var.
> add_fde_cfi() : emit a marker for the epilogue start;
> dwarf2out_cfi_begin_epilogue (): note that we need to emit the epilogue 
> start marker when the epilogue is at the end.
> dw_cfi_oprnd1_desc (): recognize DW_CFA_GNU_start_epilogue as a no-op.
> output_cfi(): print debug message for DW_CFA_GNU_start_epilogue
> emit_cfi_or_skip_epilogue (): New.
> output_fde () : use emit_cfi_or_skip_epilogue ();
>
> target.def: suppress_eh_epilogue_p (): New ASM Hook.
>
> === the remainder are the implementation on the darwin side:
>
> gcc/config/darwin.h (TARGET_ASM_SUPPRESS_EH_EPILOGUE_P): New
> gcc/config/darwin.c (darwin_asm_suppress_eh_epilogue_p): New.
> gcc/config/darwin-protos.h: Declare darwin_asm_suppress_eh_epilogue_p.
>
> ========== - ===========

> Index: include/dwarf2.h
> ===================================================================
> --- include/dwarf2.h	(revision 163221)
> +++ include/dwarf2.h	(working copy)
> @@ -854,7 +854,8 @@ enum dwarf_call_frame_info
>      /* GNU extensions.  */
>      DW_CFA_GNU_window_save = 0x2d,
>      DW_CFA_GNU_args_size = 0x2e,
> -    DW_CFA_GNU_negative_offset_extended = 0x2f
> +    DW_CFA_GNU_negative_offset_extended = 0x2f,
> +    DW_CFA_GNU_start_epilogue = 0x30
>    };
>  
>  #define DW_CIE_ID	  0xffffffff
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c	(revision 163221)
> +++ gcc/dwarf2out.c	(working copy)
> @@ -720,7 +724,9 @@ dwarf_cfi_name (unsigned int cfi_opc)
>        return "DW_CFA_GNU_args_size";
>      case DW_CFA_GNU_negative_offset_extended:
>        return "DW_CFA_GNU_negative_offset_extended";
> -
> +    case DW_CFA_GNU_start_epilogue:
> +      return "DW_CFA_GNU_start_epilogue";
> +      
>      default:
>        return "DW_CFA_<unknown>";
>      }
> @@ -801,6 +807,9 @@ dwarf2out_cfi_label (bool force)
>  /* True if remember_state should be emitted before following CFI directive.  */
>  static bool emit_cfa_remember;
>  
> +/* True if start_epilogue should be emitted before following CFI directive.  */
> +static bool emit_cfa_start_epilogue;
> +
>  /* Add CFI to the current fde at the PC value indicated by LABEL if specified,
>     or to the CIE if LABEL is NULL.  */
>  
> @@ -809,6 +818,17 @@ add_fde_cfi (const char *label, dw_cfi_ref cfi)
>  {
>    dw_cfi_ref *list_head;
>  
> +  if (emit_cfa_start_epilogue)
> +    {
> +      dw_cfi_ref cfi_epi_start;
> +
> +      /* Emit the state save.  */
> +      emit_cfa_start_epilogue = false;
> +      cfi_epi_start = new_cfi ();
> +      cfi_epi_start->dw_cfi_opc = DW_CFA_GNU_start_epilogue;
> +      add_fde_cfi (label, cfi_epi_start);   
> +    }
> +    
>    if (emit_cfa_remember)
>      {
>        dw_cfi_ref cfi_remember;
> @@ -2898,7 +2918,12 @@ dwarf2out_cfi_begin_epilogue (rtx insn)
>    gcc_assert (i != NULL);
>    i = next_real_insn (i);
>    if (i == NULL)
> -    return;
> +    {
> +      /* But we do mark the start of the epilogue to allow it to be skipped
> +         in _eh frames.  */
> +      emit_cfa_start_epilogue = true; 
> +      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
> @@ -2953,6 +2978,7 @@ dw_cfi_oprnd1_desc (enum dwarf_call_frame_info cfi
>      case DW_CFA_GNU_window_save:
>      case DW_CFA_remember_state:
>      case DW_CFA_restore_state:
> +    case DW_CFA_GNU_start_epilogue:
>        return dw_cfi_oprnd_unused;
>  
>      case DW_CFA_set_loc:
> @@ -3121,6 +3148,10 @@ 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_GNU_start_epilogue)
> +/* DEBUG */
> +    fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START
> +    		" DW_CFA_GNU_start_epilogue\n",asm_out_file);
>    else
>      {
>        dw2_asm_output_data (1, cfi->dw_cfi_opc,
> @@ -3303,6 +3334,12 @@ output_cfi_directive (dw_cfi_ref cfi)
>  	       cfi->dw_cfi_oprnd1.dw_cfi_offset);
>        break;
>  
> +    case DW_CFA_GNU_start_epilogue:
> +/*DEBUG */
> +    fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START
> +    		" DW_CFA_GNU_start_epilogue\n",asm_out_file);
> +      break;
> +      
>      case DW_CFA_remember_state:
>        fprintf (asm_out_file, "\t.cfi_remember_state\n");
>        break;
> @@ -3498,6 +3535,41 @@ 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 
> +      && targetm.asm_out.suppress_eh_epilogue_p())
> +    {
> +      if (cfi->dw_cfi_opc == DW_CFA_remember_state)
> +	{
> +	  /* 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_GNU_start_epilogue)
> +        {
> +/*DEBUG */
> +    fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START
> +    		" DW_CFA_GNU_start_epilogue\n",asm_out_file);
> +	  while (cfi->dw_cfi_next) 
> +	    /* Skip to the end.  */
> +	    cfi = cfi->dw_cfi_next;
> +          return cfi;
> +        }
> +    }
> +
> +  /* if it's not a special case, then just emit it.  */
> +  output_cfi (cfi, fde, for_eh);
> +  return cfi;
> +}
> +
>  /* Output one FDE.  */
>  
>  static void
> @@ -3613,13 +3685,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;
>  	  }
> @@ -3636,7 +3707,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/target.def
> ===================================================================
> --- gcc/target.def	(revision 163221)
> +++ gcc/target.def	(working copy)
> @@ -390,6 +406,15 @@ DEFHOOK
>   void, (FILE *file, int size, rtx x),
>   NULL)
>  
> +/* Targets might not need epilogue information in dwarf2 _eh frames.  This
> +   hook should return true if the epilogue should be suppressed in such frames.
> +   Epilogues will still be emitted in _debug_frames.  */
> +DEFHOOK_UNDOC
> +(suppress_eh_epilogue_p,
> + "",
> + bool, (void),
> + hook_bool_void_false)
> +
>  /* Some target machines need to postscan each insn after it is output.  */
>  DEFHOOK
>  (final_postscan_insn,
> Index: gcc/config/darwin-protos.h
> ===================================================================
> --- gcc/config/darwin-protos.h	(revision 163221)
> +++ gcc/config/darwin-protos.h	(working copy)
> @@ -83,10 +84,15 @@ extern tree darwin_handle_weak_import_attribute (t
>  extern void machopic_output_stub (FILE *, const char *, const char *);
>  extern void darwin_globalize_label (FILE *, const char *);
>  extern void darwin_assemble_visibility (tree, int);
> +
> +extern bool darwin_asm_suppress_eh_epilogue_p (void);
> +extern void darwin_asm_output_dwarf_section_start_label (FILE *file, 
> +							section *sect);
>  extern void darwin_asm_output_dwarf_delta (FILE *, int, const char *,
>  					   const char *);
>  extern void darwin_asm_output_dwarf_offset (FILE *, int, const char *,
>  					    section *);
> +
>  extern void darwin_asm_declare_constant_name (FILE *, const char *,
>  					      const_tree, HOST_WIDE_INT);
>  extern bool darwin_binds_local_p (const_tree);
> Index: gcc/config/darwin.h
> ===================================================================
> --- gcc/config/darwin.h	(revision 163221)
> +++ gcc/config/darwin.h	(working copy)
> @@ -669,7 +675,6 @@ extern GTY(()) int darwin_ms_struct;
>     Make Objective-C internal symbols local and in doing this, we need 
>     to accommodate the name mangling done by c++ on file scope locals.  */
>  
> -
>  int darwin_label_is_anonymous_local_objc_name (const char *name);
>  
>  #undef	ASM_OUTPUT_LABELREF
> @@ -927,6 +932,16 @@ enum machopic_addr_class {
>     ? (DW_EH_PE_pcrel | DW_EH_PE_indirect | DW_EH_PE_sdata4) : \
>       ((CODE) == 1 || (GLOBAL) == 0) ? DW_EH_PE_pcrel : DW_EH_PE_absptr)
>  
> +/* Mark the start of each dwarf debug section to allow us to compute local
> +   offsets within the sections.  We do this in darwin, rather than emitting
> +   relocs.  */
> +#define TARGET_ASM_OUTPUT_DWARF_SECTION_START_LABEL \
> +	darwin_asm_output_dwarf_section_start_label
> +
> +/* For OSX compatibility we do not want to emit epilogues in _eh frames.  */
> +#define TARGET_ASM_SUPPRESS_EH_EPILOGUE_P \
> +	darwin_asm_suppress_eh_epilogue_p
> +
>  #define ASM_OUTPUT_DWARF_DELTA(FILE,SIZE,LABEL1,LABEL2)  \
>    darwin_asm_output_dwarf_delta (FILE, SIZE, LABEL1, LABEL2)
>  
> Index: gcc/config/darwin.c
> ===================================================================
> --- gcc/config/darwin.c	(revision 163221)
> +++ gcc/config/darwin.c	(working copy)
> @@ -1666,6 +1666,36 @@ darwin_assemble_visibility (tree decl, int vis)
>  	     "not supported in this configuration; ignored");
>  }
>  
> +/* For compatibility with OSX versions that do not emit epilogues in _eh
> +   frames we suppress them.  This is made a predicate function to permit 
> +   us to add an OSX/FSF compatibility switch should that be required.  */
> +
> +bool
> +darwin_asm_suppress_eh_epilogue_p (void)
> +{
> +  return true;
> +}
> +
> +/* So that we can compute dwarf offsets within sections, we emit a known
> +   section marker at the begining of the section.  This is distinct from
> +   the ones emitted by dwarf2out.  The label is constructed by extracting
> +   sectname from __DWARF,__sectname,etc,etc.  The hook should be invoked
> +   once, after the first switch to the section.  */
> +   
> +void
> +darwin_asm_output_dwarf_section_start_label (FILE *file, section *sect)
> +{
> +  const char *dnam;
> +  int namelen;
> +  gcc_assert (sect && (sect->common.flags & (SECTION_NAMED|SECTION_DEBUG)));
> +  dnam = ((struct named_section *)sect)->name;
> +  gcc_assert (strncmp (dnam, "__DWARF,", 8) == 0);
> +  gcc_assert (strchr (dnam + 8, ','));
> +
> +  namelen = strchr (dnam + 8, ',') - (dnam + 8);
> +  fprintf (file, "Lsection%.*s:\n", namelen, dnam + 8);
> +}
> +
>  /* Output a difference of two labels that will be an assembly time
>     constant if the two labels are local.  (.long lab1-lab2 will be
>     very different if lab1 is at the boundary between two sections; it

>
>
>
>
>

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

* Re: [Patch, _eh, dawin]  Allow targets to suppress epilogues in _eh frames.
  2010-08-14  4:23 ` [Patch, _eh, dawin] " Jack Howarth
@ 2010-08-14 15:21   ` IainS
  2010-08-15 15:34     ` Mike Stump
  0 siblings, 1 reply; 22+ messages in thread
From: IainS @ 2010-08-14 15:21 UTC (permalink / raw)
  To: Jack Howarth; +Cc: GCC Patches, Richard Henderson, mrs, Jakub Jelinek

Thanks to Richard and Jakub for comments (on list and in irc)

I've nearly finished re-working the patch (in testing now).

On 14 Aug 2010, at 03:19, Jack Howarth wrote:
> FAIL: g++.dg/eh/async-unwind2.C execution test

if we say that darwin does not support -fasynchronous-unwind-tables,  
then we should simply skip this test.

> ld: warning: can't make compact unwind encoding from dwarf for  
> S::sfn2(int)  in /var/folders/1C/1CdoNxmNFHyOIjNBLNuJh++++TM/-Tmp-// 
> ccw4LTKt.o because dwarf uses DW_CFA_GNU_args_size

This is interesting - is this a dwarf-2 opcode?

It is enabled by -fasynchronous-unwind-tables (without any fall-back  
for non-gnu targets)

Does this imply that -fasynchronous-unwind-tables is only usable by a  
gnu target?

also I note that:
-fnon-call-exceptions  _unconditionally_  sets -fasynchronous-unwind- 
tables in toplev.c

(and, of course, libjava does set  -fnon-call-exceptions, so that  
builds with a bunch of warnings now - since I've added warnings for  
the two cases).

So ... where do we stand?

maybe toplev should do sth like;
if ( flag_non_call_exceptions && flag_asynchronous_unwind_tables == 2)
   flag_asynchronous_unwind_tables = 1;

Richard?, Jakub? any advice appreciated.

thanks
Iain



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

* Re: [Patch, _eh, dawin]  Allow targets to suppress epilogues in _eh frames.
  2010-08-14 15:21   ` IainS
@ 2010-08-15 15:34     ` Mike Stump
  0 siblings, 0 replies; 22+ messages in thread
From: Mike Stump @ 2010-08-15 15:34 UTC (permalink / raw)
  To: IainS; +Cc: Jack Howarth, GCC Patches, Richard Henderson, mrs, Jakub Jelinek

On Aug 14, 2010, at 6:49 AM, IainS wrote:
> if we say that darwin does not support -fasynchronous-unwind-tables, then we should simply skip this test.

darwin would like it...  I don't know of anyone that is twisting in the wind on it however, so it could be xfailed for now, if one wants a cleaner run.

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

* [Patch, _eh, dawin, Version2]  Allow targets to suppress epilogues in _eh frames.
  2010-08-14  0:10 ` Richard Henderson
@ 2010-08-15 20:03   ` IainS
  2010-08-16  1:18     ` Jack Howarth
                       ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: IainS @ 2010-08-15 20:03 UTC (permalink / raw)
  To: Richard Henderson; +Cc: GCC Patches, mrs

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

Hi Richard,

Well, of course your suggestions (and Jakub's comments in irc)   
resulted a better solution.

[the questions in a previous follow-up mail re. the GNU opcode still  
stand - but don't affect the base patch].

this has been looped around a few times on darwin9 & 10 and  
bootstrapped on x86_64-unk-linux-gnu.
Ok for trunk now?
Iain


On 14 Aug 2010, at 00:27, Richard Henderson wrote:

> On 08/13/2010 12:50 PM, IainS wrote:
>> +    DW_CFA_GNU_start_epilogue = 0x30
>
> I'd rather not place this in dwarf2.h since we don't plan to emit this
> as an actual opcode.  It only needs to be a marker in the dw_cfi_ref
> linked list for internal use of gcc.
>
> As such I think something like
>
> # define DW_CFA_INTERNAL_start_epilogue  0x100
>
> would be appropriate.  I believe you'll need to change
>
>  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;

done.
> in order for this to be legal.  Unfortunately that may
> require a host of other type changes or type casts in
> order to pass the C++ warning mode.
>
>> +/* True if start_epilogue should be emitted before following CFI  
>> directive.  */
>> +static bool emit_cfa_start_epilogue;
>
> I don't think this is needed...
>
>>   if (i == NULL)
>> -    return;
>> +    {
>> +      /* But we do mark the start of the epilogue to allow it to  
>> be skipped
>> +         in _eh frames.  */
>> +      emit_cfa_start_epilogue = true;
>> +      return;
>> +    }
>
> ... instead we want to add the cfi entry here, by hand.  The reason
> being that we want to avoid the DW_CFA_advance_loc4 that would be
> added by actually inserting this opcode via add_fde_cfi.

done

>> +/* Targets might not need epilogue information in dwarf2 _eh  
>> frames.  This
>> +   hook should return true if the epilogue should be suppressed in  
>> such frames.
>> +   Epilogues will still be emitted in _debug_frames.  */
>> +DEFHOOK_UNDOC
>> +(suppress_eh_epilogue_p,
>> + "",
>> + bool, (void),
>> + hook_bool_void_false)
>
> As discussed on IRC, I don't think we need a target hook for this.  We
> should key this off of !flag_asynchronous_unwind_tables.  That will
> allow all targets that only need unwind info for call sites omit this
> extra epilogue unwind info.

done.

> At that point you can adjust Darwin's override_options hook(s) to make
> sure that this flag is appropriately off.

not so simple - because toplev.c manipulates it unconditionally - I've  
adjusted toplev to set the flag only if it is unset.
and will make manipulation of default settings on darwin a separate  
patch (darwin does not yet use that hook).

>  It also seems like it would
> be a good idea to add a SPEC entry to disable compact unwind if the
> user explicitly uses -fasynchronous-unwind-info.

At present, I'm going to leave the compact unwinder disabled on  
Darwin10, it seems from the testing done by Jack that there are a  
bunch of other (probably non-gnu) issues to iron out on that yet.   
Ergo, the spec is not needed yet.

===

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): Ditto.
	(output_cfi_directive): Handle   DW_CFA_INTERNAL_start_epilogue.
	(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.

====


[-- Attachment #2: 163267-eh-frame-epilogue-v2.txt --]
[-- Type: text/plain, Size: 9319 bytes --]

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 163267)
+++ gcc/dwarf2out.c	(working copy)
@@ -268,12 +272,20 @@ 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;
-  dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd1_desc (%1.dw_cfi_opc)")))
+  unsigned int dw_cfi_opc;
+  dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd1_desc ((unsigned int)%1.dw_cfi_opc)")))
     dw_cfi_oprnd1;
-  dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd2_desc (%1.dw_cfi_opc)")))
+  dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd2_desc ((unsigned int)%1.dw_cfi_opc)")))
     dw_cfi_oprnd2;
 }
 dw_cfi_node;
@@ -821,9 +833,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 ();
@@ -850,6 +868,7 @@ add_fde_cfi (const char *label, dw_cfi_ref cfi)
 		case DW_CFA_def_cfa_sf:
 		case DW_CFA_def_cfa_expression:
 		case DW_CFA_restore_state:
+		case DW_CFA_INTERNAL_start_epilogue:
 		  if (*label == 0 || strcmp (label, "<do not output>") == 0)
 		    label = dwarf2out_cfi_label (true);
 
@@ -2894,12 +2913,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
@@ -2941,11 +2970,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)
     {
@@ -2953,6 +2980,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:
@@ -2990,11 +3018,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)
     {
@@ -3121,6 +3148,13 @@ 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 unless we want a debug message.  */
+      if (flag_debug_asm)
+	fputs (ASM_COMMENT_START"\t\t\t" ASM_COMMENT_START
+			" DW_CFA_INTERNAL_start_epilogue\n",asm_out_file);
+    }
   else
     {
       dw2_asm_output_data (1, cfi->dw_cfi_opc,
@@ -3303,6 +3337,13 @@ output_cfi_directive (dw_cfi_ref cfi)
 	       cfi->dw_cfi_oprnd1.dw_cfi_offset);
       break;
 
+    case DW_CFA_INTERNAL_start_epilogue:
+      /* no-op apart from an informational message.  */
+      if (flag_debug_asm)
+	fputs (ASM_COMMENT_START"\t\t\t" ASM_COMMENT_START
+			" DW_CFA_INTERNAL_start_epilogue\n",asm_out_file);
+      break;
+      
     case DW_CFA_remember_state:
       fprintf (asm_out_file, "\t.cfi_remember_state\n");
       break;
@@ -3498,6 +3539,46 @@ 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);
+	  while (cfi->dw_cfi_next) 
+	    /* Skip to the end.  */
+	    cfi = cfi->dw_cfi_next;
+          return cfi;
+        }
+    }
+
+  /* If it's not a special case, then just carry on.  
+     This will also cause the no-op 'DW_CFA_INTERNAL_start_epilogue' to be
+     listed when flag_debug_asm is set.  */
+  output_cfi (cfi, fde, for_eh);
+  return cfi;
+}
+
 /* Output one FDE.  */
 
 static void
@@ -3613,13 +3694,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;
 	  }
@@ -3627,7 +3708,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;
@@ -3636,7 +3716,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 163267)
+++ gcc/toplev.c	(working copy)
@@ -1798,9 +1801,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/config/darwin.c
===================================================================
--- gcc/config/darwin.c	(revision 163267)
+++ gcc/config/darwin.c	(working copy)
@@ -1884,7 +1915,22 @@ darwin_override_options (void)
       flag_reorder_blocks_and_partition = 0;
       flag_reorder_blocks = 1;
     }
+    
+  if (flag_asynchronous_unwind_tables)
+    {
+      if (flag_asynchronous_unwind_tables == 2)
+	flag_asynchronous_unwind_tables = 0;
+      else
+	/* Issue a warning */
+	warning (OPT_Wall,
+	  "this architecture does not fully support"
+	  " -fasynchronous-unwind-tables");
+    }
 
+  if ((flag_exceptions || flag_non_call_exceptions)
+       && strverscmp (darwin_macosx_version_min, "10.4") >= 0)
+    flag_unwind_tables = 1;
+
   if (flag_mkernel || flag_apple_kext)
     {
       /* -mkernel implies -fapple-kext for C++ */
@@ -1897,6 +1943,9 @@ 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;
     }

[-- Attachment #3: Type: text/plain, Size: 7 bytes --]




	



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

* Re: [Patch, _eh, dawin, Version2]  Allow targets to suppress epilogues in _eh frames.
  2010-08-15 20:03   ` [Patch, _eh, dawin, Version2] " IainS
@ 2010-08-16  1:18     ` Jack Howarth
  2010-08-16  9:20       ` IainS
  2010-08-16  4:15     ` Jack Howarth
  2010-08-16 16:21     ` Richard Henderson
  2 siblings, 1 reply; 22+ messages in thread
From: Jack Howarth @ 2010-08-16  1:18 UTC (permalink / raw)
  To: IainS; +Cc: Richard Henderson, GCC Patches, mrs

On Sun, Aug 15, 2010 at 08:53:31PM +0100, IainS wrote:
> On 14 Aug 2010, at 00:27, Richard Henderson wrote:
>
>>  It also seems like it would
>> be a good idea to add a SPEC entry to disable compact unwind if the
>> user explicitly uses -fasynchronous-unwind-info.
>
> At present, I'm going to leave the compact unwinder disabled on  
> Darwin10, it seems from the testing done by Jack that there are a bunch 
> of other (probably non-gnu) issues to iron out on that yet.  Ergo, the 
> spec is not needed yet.
>

Iain,
   On x86_64-apple-darwin10, I am seeing the following new failures
compard to your previous version of this patch...

FAIL: g++.dg/eh/async-unwind1.C (test for excess errors)
FAIL: g++.dg/eh/async-unwind2.C (test for excess errors)

These appear as...

Executing on host: /sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/gcc/testsuite/g++/../../g++ -B/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/gcc/testsuite/g++/../../ /sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100815/gcc/testsuite/g++.dg/eh/async-unwind1.C  -nostdinc++ -I/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/x86_64-apple-darwin10.5.0/i386/libstdc++-v3/include/x86_64-apple-darwin10.5.0 -I/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/x86_64-apple-darwin10.5.0/i386/libstdc++-v3/include -I/sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100815/libstdc++-v3/libsupc++ -I/sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100815/libstdc++-v3/include/backward -I/sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100815/libstdc++-v3/testsuite/util -fmessage-length=0  -Os -fasynchronous-unwind-tables -mpreferred-stack-boundary=4    -L/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/x86_64-apple-darwin10.5.0/i386/libstdc++-v3/src/.libs  -B/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/x86_64-apple-darwin10.5.0/i386/libstdc++-v3/src/.libs  -L/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/x86_64-apple-darwin10.5.0/i386/libstdc++-v3/src/.libs -L/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/x86_64-apple-darwin10.5.0/i386/libiberty  -multiply_defined suppress -lm   -m32 -o ./async-unwind1.exe    (timeout = 300)
/sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100815/gcc/testsuite/g++.dg/eh/async-unwind1.C:1:0: warning: this architecture does not fully support -fasynchronous-unwind-tables [-Wall]
output is:
/sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100815/gcc/testsuite/g++.dg/eh/async-unwind1.C:1:0: warning: this architecture does not fully support -fasynchronous-unwind-tables [-Wall]

FAIL: g++.dg/eh/async-unwind1.C (test for excess errors)
Excess errors:
/sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100815/gcc/testsuite/g++.dg/eh/async-unwind1.C:1:0: warning: this architecture does not fully support -fasynchronous-unwind-tables [-Wall]

Setting LD_LIBRARY_PATH to .:/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/x86_64-apple-darwin10.5.0/i386/libstdc++-v3/src/.libs:/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/x86_64-apple-darwin10.5.0/i386/libstdc++-v3/src/.libs:/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/gcc:.:/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/x86_64-apple-darwin10.5.0/i386/libstdc++-v3/src/.libs:/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/x86_64-apple-darwin10.5.0/i386/libstdc++-v3/src/.libs:/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/gcc
PASS: g++.dg/eh/async-unwind1.C execution test

so while we have a warning, the generated code works fine. I think that as
long as we stick with the compatibility unwinder, the asynchronous unwind
should be fine (since the compatibility unwinder should functionally be
the same as that from gcc 4.2.1 in darwin10). Or do you have some other
reason to suspect that there are problems with asynchronous unwind and
the compatibility unwinder in darwin10?
                  Jack
ps I don't think Richard's suggestion of a SPEC entry to disable
compact unwind if the user explicitly uses -fasynchronous-unwind-info
makes sense. The use of -no_compact_unwind causes the linked code to
use the compatibility unwinder (derived from libgcc in gcc 4.2.1).
If you added such a feature, the libjava shared libraries would be
linked expecting to run on the compatibility unwinder but
additional code compiled with the -fasynchronous-unwind-info flag
would suddenly switch FSF gcc over to the compact unwinder. It gives
me a headache just trying to imagine what that might do.

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

* Re: [Patch, _eh, dawin, Version2]  Allow targets to suppress epilogues in _eh frames.
  2010-08-15 20:03   ` [Patch, _eh, dawin, Version2] " IainS
  2010-08-16  1:18     ` Jack Howarth
@ 2010-08-16  4:15     ` Jack Howarth
  2010-08-16  7:46       ` Jack Howarth
  2010-08-16 16:21     ` Richard Henderson
  2 siblings, 1 reply; 22+ messages in thread
From: Jack Howarth @ 2010-08-16  4:15 UTC (permalink / raw)
  To: IainS; +Cc: Richard Henderson, GCC Patches, mrs

Iain,
   I am also seeing the failure...

FAIL: gfortran.dg/g77/980701-0.f  -Os  execution test

at -m64 under this second revision of your _eh darwin
patch. This represents a regression from both current
gcc trunk and your previous _eh darwin patch...

http://gcc.gnu.org/ml/gcc-patches/2010-08/msg01037.html

when I tested against either the compatibility or compact
unwinders on x86_64-apple-darwin10 or against the compatibility
unwinder on i386-apple-darwin10.

http://gcc.gnu.org/ml/gcc-testresults/2010-08/msg01454.html
http://gcc.gnu.org/ml/gcc-testresults/2010-08/msg01429.html
http://gcc.gnu.org/ml/gcc-testresults/2010-08/msg01573.html

It would appear that the second patch doesn't exactly reproduce
the behavior of the first. Can you break up the changes between
the first and second patches into sections to figure out which
part triggered the gfortran testsuite failure?
                    Jack

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

* Re: [Patch, _eh, dawin, Version2]  Allow targets to suppress epilogues in _eh frames.
  2010-08-16  4:15     ` Jack Howarth
@ 2010-08-16  7:46       ` Jack Howarth
  2010-08-16 10:07         ` IainS
  0 siblings, 1 reply; 22+ messages in thread
From: Jack Howarth @ 2010-08-16  7:46 UTC (permalink / raw)
  To: IainS; +Cc: Richard Henderson, GCC Patches, mrs

On Sun, Aug 15, 2010 at 10:31:13PM -0400, Jack Howarth wrote:
> Iain,
>    I am also seeing the failure...
> 
> FAIL: gfortran.dg/g77/980701-0.f  -Os  execution test
> 
> at -m64 under this second revision of your _eh darwin
> patch. This represents a regression from both current
> gcc trunk and your previous _eh darwin patch...
> 
> http://gcc.gnu.org/ml/gcc-patches/2010-08/msg01037.html
> 
> when I tested against either the compatibility or compact
> unwinders on x86_64-apple-darwin10 or against the compatibility
> unwinder on i386-apple-darwin10.
> 
> http://gcc.gnu.org/ml/gcc-testresults/2010-08/msg01454.html
> http://gcc.gnu.org/ml/gcc-testresults/2010-08/msg01429.html
> http://gcc.gnu.org/ml/gcc-testresults/2010-08/msg01573.html
> 
> It would appear that the second patch doesn't exactly reproduce
> the behavior of the first. Can you break up the changes between
> the first and second patches into sections to figure out which
> part triggered the gfortran testsuite failure?
>                     Jack

Iain,
   In case it helps, below is the diff between current gcc trunk
with your first and second patches applied so that the differences
between the two versions are more clearly seen.
                     Jack

diff --exclude=.svn -uNr gcc-4.6-20100815.old_eh_patch/gcc/config/darwin-protos.h gcc-4.6-20100815/gcc/config/darwin-protos.h
--- gcc-4.6-20100815.old_eh_patch/gcc/config/darwin-protos.h	2010-08-16 00:02:03.000000000 -0400
+++ gcc-4.6-20100815/gcc/config/darwin-protos.h	2010-08-15 16:44:14.000000000 -0400
@@ -83,15 +83,10 @@
 extern void machopic_output_stub (FILE *, const char *, const char *);
 extern void darwin_globalize_label (FILE *, const char *);
 extern void darwin_assemble_visibility (tree, int);
-
-extern bool darwin_asm_suppress_eh_epilogue_p (void);
-extern void darwin_asm_output_dwarf_section_start_label (FILE *file, 
-							section *sect);
 extern void darwin_asm_output_dwarf_delta (FILE *, int, const char *,
 					   const char *);
 extern void darwin_asm_output_dwarf_offset (FILE *, int, const char *,
 					    section *);
-
 extern void darwin_asm_declare_constant_name (FILE *, const char *,
 					      const_tree, HOST_WIDE_INT);
 extern bool darwin_binds_local_p (const_tree);
diff --exclude=.svn -uNr gcc-4.6-20100815.old_eh_patch/gcc/config/darwin.c gcc-4.6-20100815/gcc/config/darwin.c
--- gcc-4.6-20100815.old_eh_patch/gcc/config/darwin.c	2010-08-16 00:02:03.000000000 -0400
+++ gcc-4.6-20100815/gcc/config/darwin.c	2010-08-15 16:51:53.000000000 -0400
@@ -1666,36 +1666,6 @@
 	     "not supported in this configuration; ignored");
 }
 
-/* For compatibility with OSX versions that do not emit epilogues in _eh
-   frames we suppress them.  This is made a predicate function to permit 
-   us to add an OSX/FSF compatibility switch should that be required.  */
-
-bool
-darwin_asm_suppress_eh_epilogue_p (void)
-{
-  return true;
-}
-
-/* So that we can compute dwarf offsets within sections, we emit a known
-   section marker at the begining of the section.  This is distinct from
-   the ones emitted by dwarf2out.  The label is constructed by extracting
-   sectname from __DWARF,__sectname,etc,etc.  The hook should be invoked
-   once, after the first switch to the section.  */
-   
-void
-darwin_asm_output_dwarf_section_start_label (FILE *file, section *sect)
-{
-  const char *dnam;
-  int namelen;
-  gcc_assert (sect && (sect->common.flags & (SECTION_NAMED|SECTION_DEBUG)));
-  dnam = ((struct named_section *)sect)->name;
-  gcc_assert (strncmp (dnam, "__DWARF,", 8) == 0);
-  gcc_assert (strchr (dnam + 8, ','));
-
-  namelen = strchr (dnam + 8, ',') - (dnam + 8);
-  fprintf (file, "Lsection%.*s:\n", namelen, dnam + 8);
-}
-
 /* Output a difference of two labels that will be an assembly time
    constant if the two labels are local.  (.long lab1-lab2 will be
    very different if lab1 is at the boundary between two sections; it
@@ -1914,6 +1884,21 @@
       flag_reorder_blocks_and_partition = 0;
       flag_reorder_blocks = 1;
     }
+    
+  if (flag_asynchronous_unwind_tables)
+    {
+      if (flag_asynchronous_unwind_tables == 2)
+	flag_asynchronous_unwind_tables = 0;
+      else
+	/* Issue a warning */
+	warning (OPT_Wall,
+	  "this architecture does not fully support"
+	  " -fasynchronous-unwind-tables");
+    }
+
+  if ((flag_exceptions || flag_non_call_exceptions)
+       && strverscmp (darwin_macosx_version_min, "10.4") >= 0)
+    flag_unwind_tables = 1;
 
   if (flag_mkernel || flag_apple_kext)
     {
@@ -1927,6 +1912,9 @@
       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;
     }
diff --exclude=.svn -uNr gcc-4.6-20100815.old_eh_patch/gcc/config/darwin.h gcc-4.6-20100815/gcc/config/darwin.h
--- gcc-4.6-20100815.old_eh_patch/gcc/config/darwin.h	2010-08-16 00:02:03.000000000 -0400
+++ gcc-4.6-20100815/gcc/config/darwin.h	2010-08-15 16:48:16.000000000 -0400
@@ -672,6 +672,7 @@
    Make Objective-C internal symbols local and in doing this, we need 
    to accommodate the name mangling done by c++ on file scope locals.  */
 
+
 int darwin_label_is_anonymous_local_objc_name (const char *name);
 
 #undef	ASM_OUTPUT_LABELREF
@@ -929,16 +930,6 @@
    ? (DW_EH_PE_pcrel | DW_EH_PE_indirect | DW_EH_PE_sdata4) : \
      ((CODE) == 1 || (GLOBAL) == 0) ? DW_EH_PE_pcrel : DW_EH_PE_absptr)
 
-/* Mark the start of each dwarf debug section to allow us to compute local
-   offsets within the sections.  We do this in darwin, rather than emitting
-   relocs.  */
-#define TARGET_ASM_OUTPUT_DWARF_SECTION_START_LABEL \
-	darwin_asm_output_dwarf_section_start_label
-
-/* For OSX compatibility we do not want to emit epilogues in _eh frames.  */
-#define TARGET_ASM_SUPPRESS_EH_EPILOGUE_P \
-	darwin_asm_suppress_eh_epilogue_p
-
 #define ASM_OUTPUT_DWARF_DELTA(FILE,SIZE,LABEL1,LABEL2)  \
   darwin_asm_output_dwarf_delta (FILE, SIZE, LABEL1, LABEL2)
 
diff --exclude=.svn -uNr gcc-4.6-20100815.old_eh_patch/gcc/dwarf2out.c gcc-4.6-20100815/gcc/dwarf2out.c
--- gcc-4.6-20100815.old_eh_patch/gcc/dwarf2out.c	2010-08-16 00:02:03.000000000 -0400
+++ gcc-4.6-20100815/gcc/dwarf2out.c	2010-08-15 16:51:53.000000000 -0400
@@ -268,12 +268,20 @@
 }
 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;
-  dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd1_desc (%1.dw_cfi_opc)")))
+  unsigned int dw_cfi_opc;
+  dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd1_desc ((unsigned int)%1.dw_cfi_opc)")))
     dw_cfi_oprnd1;
-  dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd2_desc (%1.dw_cfi_opc)")))
+  dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd2_desc ((unsigned int)%1.dw_cfi_opc)")))
     dw_cfi_oprnd2;
 }
 dw_cfi_node;
@@ -720,9 +728,7 @@
       return "DW_CFA_GNU_args_size";
     case DW_CFA_GNU_negative_offset_extended:
       return "DW_CFA_GNU_negative_offset_extended";
-    case DW_CFA_GNU_start_epilogue:
-      return "DW_CFA_GNU_start_epilogue";
-      
+
     default:
       return "DW_CFA_<unknown>";
     }
@@ -803,9 +809,6 @@
 /* True if remember_state should be emitted before following CFI directive.  */
 static bool emit_cfa_remember;
 
-/* True if start_epilogue should be emitted before following CFI directive.  */
-static bool emit_cfa_start_epilogue;
-
 /* Add CFI to the current fde at the PC value indicated by LABEL if specified,
    or to the CIE if LABEL is NULL.  */
 
@@ -814,17 +817,6 @@
 {
   dw_cfi_ref *list_head;
 
-  if (emit_cfa_start_epilogue)
-    {
-      dw_cfi_ref cfi_epi_start;
-
-      /* Emit the state save.  */
-      emit_cfa_start_epilogue = false;
-      cfi_epi_start = new_cfi ();
-      cfi_epi_start->dw_cfi_opc = DW_CFA_GNU_start_epilogue;
-      add_fde_cfi (label, cfi_epi_start);   
-    }
-    
   if (emit_cfa_remember)
     {
       dw_cfi_ref cfi_remember;
@@ -837,8 +829,14 @@
     }
 
   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)
 	{
@@ -866,6 +864,7 @@
 		case DW_CFA_def_cfa_sf:
 		case DW_CFA_def_cfa_expression:
 		case DW_CFA_restore_state:
+		case DW_CFA_INTERNAL_start_epilogue:
 		  if (*label == 0 || strcmp (label, "<do not output>") == 0)
 		    label = dwarf2out_cfi_label (true);
 
@@ -2910,14 +2909,19 @@
     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)
     {
-      /* But we do mark the start of the epilogue to allow it to be skipped
-         in _eh frames.  */
-      emit_cfa_start_epilogue = true; 
+      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;
     }
 
@@ -2962,11 +2966,9 @@
 }
 
 /* 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)
     {
@@ -2974,7 +2976,7 @@
     case DW_CFA_GNU_window_save:
     case DW_CFA_remember_state:
     case DW_CFA_restore_state:
-    case DW_CFA_GNU_start_epilogue:
+    case DW_CFA_INTERNAL_start_epilogue:
       return dw_cfi_oprnd_unused;
 
     case DW_CFA_set_loc:
@@ -3012,11 +3014,9 @@
 }
 
 /* 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)
     {
@@ -3143,10 +3143,13 @@
       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_GNU_start_epilogue)
-/* DEBUG */
-    fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START
-    		" DW_CFA_GNU_start_epilogue\n",asm_out_file);
+  else if (cfi->dw_cfi_opc == DW_CFA_INTERNAL_start_epilogue)
+    {
+      /* This is a nop unless we want a debug message.  */
+      if (flag_debug_asm)
+	fputs (ASM_COMMENT_START"\t\t\t" ASM_COMMENT_START
+			" DW_CFA_INTERNAL_start_epilogue\n",asm_out_file);
+    }
   else
     {
       dw2_asm_output_data (1, cfi->dw_cfi_opc,
@@ -3329,10 +3332,11 @@
 	       cfi->dw_cfi_oprnd1.dw_cfi_offset);
       break;
 
-    case DW_CFA_GNU_start_epilogue:
-/*DEBUG */
-    fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START
-    		" DW_CFA_GNU_start_epilogue\n",asm_out_file);
+    case DW_CFA_INTERNAL_start_epilogue:
+      /* no-op apart from an informational message.  */
+      if (flag_debug_asm)
+	fputs (ASM_COMMENT_START"\t\t\t" ASM_COMMENT_START
+			" DW_CFA_INTERNAL_start_epilogue\n",asm_out_file);
       break;
       
     case DW_CFA_remember_state:
@@ -3537,10 +3541,13 @@
 emit_cfi_or_skip_epilogue (dw_cfi_ref cfi, dw_fde_ref fde, bool for_eh)
 {
   if (for_eh 
-      && targetm.asm_out.suppress_eh_epilogue_p())
+      && !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 
@@ -3548,11 +3555,11 @@
 	    cfi = cfi->dw_cfi_next;
 	  return cfi;
         }
-      if (cfi->dw_cfi_opc == DW_CFA_GNU_start_epilogue)
+      if (cfi->dw_cfi_opc == DW_CFA_INTERNAL_start_epilogue)
         {
-/*DEBUG */
-    fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START
-    		" DW_CFA_GNU_start_epilogue\n",asm_out_file);
+	  if (flag_debug_asm)
+	    fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START
+			" DW_CFA_INTERNAL_start_epilogue\n",asm_out_file);
 	  while (cfi->dw_cfi_next) 
 	    /* Skip to the end.  */
 	    cfi = cfi->dw_cfi_next;
@@ -3560,7 +3567,9 @@
         }
     }
 
-  /* if it's not a special case, then just emit it.  */
+  /* If it's not a special case, then just carry on.  
+     This will also cause the no-op 'DW_CFA_INTERNAL_start_epilogue' to be
+     listed when flag_debug_asm is set.  */
   output_cfi (cfi, fde, for_eh);
   return cfi;
 }
@@ -3694,7 +3703,6 @@
   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;
diff --exclude=.svn -uNr gcc-4.6-20100815.old_eh_patch/gcc/target.def gcc-4.6-20100815/gcc/target.def
--- gcc-4.6-20100815.old_eh_patch/gcc/target.def	2010-08-16 00:02:03.000000000 -0400
+++ gcc-4.6-20100815/gcc/target.def	2010-08-15 16:44:16.000000000 -0400
@@ -390,15 +390,6 @@
  void, (FILE *file, int size, rtx x),
  NULL)
 
-/* Targets might not need epilogue information in dwarf2 _eh frames.  This
-   hook should return true if the epilogue should be suppressed in such frames.
-   Epilogues will still be emitted in _debug_frames.  */
-DEFHOOK_UNDOC
-(suppress_eh_epilogue_p,
- "",
- bool, (void),
- hook_bool_void_false)
-
 /* Some target machines need to postscan each insn after it is output.  */
 DEFHOOK
 (final_postscan_insn,
diff --exclude=.svn -uNr gcc-4.6-20100815.old_eh_patch/gcc/toplev.c gcc-4.6-20100815/gcc/toplev.c
--- gcc-4.6-20100815.old_eh_patch/gcc/toplev.c	2010-08-16 00:00:52.000000000 -0400
+++ gcc-4.6-20100815/gcc/toplev.c	2010-08-15 16:51:53.000000000 -0400
@@ -1798,9 +1798,9 @@
   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)
diff --exclude=.svn -uNr gcc-4.6-20100815.old_eh_patch/include/dwarf2.h gcc-4.6-20100815/include/dwarf2.h
--- gcc-4.6-20100815.old_eh_patch/include/dwarf2.h	2010-08-16 00:02:03.000000000 -0400
+++ gcc-4.6-20100815/include/dwarf2.h	2010-08-15 16:45:11.000000000 -0400
@@ -854,8 +854,7 @@
     /* GNU extensions.  */
     DW_CFA_GNU_window_save = 0x2d,
     DW_CFA_GNU_args_size = 0x2e,
-    DW_CFA_GNU_negative_offset_extended = 0x2f,
-    DW_CFA_GNU_start_epilogue = 0x30
+    DW_CFA_GNU_negative_offset_extended = 0x2f
   };
 
 #define DW_CIE_ID	  0xffffffff

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

* Re: [Patch, _eh, dawin, Version2]  Allow targets to suppress epilogues in _eh frames.
  2010-08-16  1:18     ` Jack Howarth
@ 2010-08-16  9:20       ` IainS
  0 siblings, 0 replies; 22+ messages in thread
From: IainS @ 2010-08-16  9:20 UTC (permalink / raw)
  To: GCC Patches; +Cc: Jack Howarth, Richard Henderson, mrs

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


On 16 Aug 2010, at 01:18, Jack Howarth wrote:

>   On x86_64-apple-darwin10, I am seeing the following new failures
> compard to your previous version of this patch...
>
> FAIL: g++.dg/eh/async-unwind1.C (test for excess errors)
> FAIL: g++.dg/eh/async-unwind2.C (test for excess errors)


Well, these were already discussed, (and g++.dg/eh/async-unwind2.C was  
failing with the previous version).

I was awaiting Mike's preference as to what to do in the case of a  
failure (which is no longer present).
attached update to the testsuite (just recognize the new warning).

cheers
Iain


testsuite:

	* g++.dg/eh/async-unwind1.C: Recognize warning on darwin.
	* g++.dg/eh/async-unwind2.C: Ditto.


[-- Attachment #2: 163268-asynch-tests.txt --]
[-- Type: text/plain, Size: 1098 bytes --]

Index: gcc/testsuite/g++.dg/eh/async-unwind2.C
===================================================================
--- gcc/testsuite/g++.dg/eh/async-unwind2.C	(revision 163276)
+++ 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 163276)
+++ 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 ();
 

[-- Attachment #3: Type: text/plain, Size: 2 bytes --]




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

* Re: [Patch, _eh, dawin, Version2]  Allow targets to suppress epilogues in _eh frames.
  2010-08-16  7:46       ` Jack Howarth
@ 2010-08-16 10:07         ` IainS
  2010-08-16 11:58           ` Jack Howarth
  0 siblings, 1 reply; 22+ messages in thread
From: IainS @ 2010-08-16 10:07 UTC (permalink / raw)
  To: Jack Howarth; +Cc: Richard Henderson, GCC Patches, mrs


On 16 Aug 2010, at 05:15, Jack Howarth wrote:

>>   I am also seeing the failure...
>>
>> FAIL: gfortran.dg/g77/980701-0.f  -Os  execution test

Hm.  This test does not contain any (overt) exception handling, I  
guess we must be seeing an interaction with the fortran runtime
(or some aspect of the optimize for smallest code which is unexpected  
--- note that m32 forces frame pointers on for optimize_size).

The salient difference between trunk and the patched version is that  
there are no unwind frames emitted for the patched version.
Un-patched trunk sets -fasynchronous-unwind-tables.

The test passes if   either

-fno-omit-frame-pointers

or

-funwind-tables

is given.

this can be replicated on un-patched trunk by passing '-fno- 
asynchronous-frame-tables'.

Given that -fno-asynchronous-frame-tables does not sit well with  
dawin...
I could arrange either to default to  "-funwind-tables" on m64 (at  
least for fortran) but please note the following:

======
apple_local 4.2.1 (5646) gcc/config/i386/i386.c:

   if (TARGET_64BIT)
     {
       /* Mach-O doesn't support omitting the frame pointer for now.  */
       if (flag_omit_frame_pointer == 2)
	flag_omit_frame_pointer = (TARGET_MACHO ? 0 : 1);

but we have recently changed trunk to this:

   if (TARGET_64BIT)
     {
       if (flag_zee == 2)
         flag_zee = 1;
       if (flag_omit_frame_pointer == 2)
	flag_omit_frame_pointer = 1;


------

If you know that the "does not support" is no longer true for Darwin10  
- then, fine, we can arrange the target over-rides to switch it off  
for Darwin10.

Otherwise, I'm not sure how we can justify omitting frame pointers by  
default for m64 OSX
Mike?

cheers,
Iain

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

* Re: [Patch, _eh, dawin, Version2]  Allow targets to suppress epilogues in _eh frames.
  2010-08-16 10:07         ` IainS
@ 2010-08-16 11:58           ` Jack Howarth
  0 siblings, 0 replies; 22+ messages in thread
From: Jack Howarth @ 2010-08-16 11:58 UTC (permalink / raw)
  To: IainS; +Cc: Richard Henderson, GCC Patches, mrs

On Mon, Aug 16, 2010 at 10:52:39AM +0100, IainS wrote:
>
> On 16 Aug 2010, at 05:15, Jack Howarth wrote:
>
>>>   I am also seeing the failure...
>>>
>>> FAIL: gfortran.dg/g77/980701-0.f  -Os  execution test
>
> Hm.  This test does not contain any (overt) exception handling, I guess 
> we must be seeing an interaction with the fortran runtime
> (or some aspect of the optimize for smallest code which is unexpected  
> --- note that m32 forces frame pointers on for optimize_size).
>
> The salient difference between trunk and the patched version is that  
> there are no unwind frames emitted for the patched version.
> Un-patched trunk sets -fasynchronous-unwind-tables.

This seems to be in line with the comments I found in...

http://llvm.org/bugs/show_bug.cgi?id=4945

in particular comment 2 there which indicates that -fasynchronous-unwind-tables
should be defaulted on with -fomit-frame-pointer on x86-32 (for gdb at least).

>
> The test passes if   either
>
> -fno-omit-frame-pointers
>
> or
>
> -funwind-tables
>
> is given.
>
> this can be replicated on un-patched trunk by passing '-fno- 
> asynchronous-frame-tables'.
>
> Given that -fno-asynchronous-frame-tables does not sit well with  
> dawin...
> I could arrange either to default to  "-funwind-tables" on m64 (at least 
> for fortran) but please note the following:
>
> ======
> apple_local 4.2.1 (5646) gcc/config/i386/i386.c:
>
>   if (TARGET_64BIT)
>     {
>       /* Mach-O doesn't support omitting the frame pointer for now.  */
>       if (flag_omit_frame_pointer == 2)
> 	flag_omit_frame_pointer = (TARGET_MACHO ? 0 : 1);
>
> but we have recently changed trunk to this:
>
>   if (TARGET_64BIT)
>     {
>       if (flag_zee == 2)
>         flag_zee = 1;
>       if (flag_omit_frame_pointer == 2)
> 	flag_omit_frame_pointer = 1;
>
>
> ------
>
> If you know that the "does not support" is no longer true for Darwin10 - 
> then, fine, we can arrange the target over-rides to switch it off for 
> Darwin10.
>
> Otherwise, I'm not sure how we can justify omitting frame pointers by  
> default for m64 OSX
> Mike?
>
> cheers,
> Iain

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

* Re: [Patch, _eh, dawin, Version2]  Allow targets to suppress epilogues in _eh frames.
  2010-08-15 20:03   ` [Patch, _eh, dawin, Version2] " IainS
  2010-08-16  1:18     ` Jack Howarth
  2010-08-16  4:15     ` Jack Howarth
@ 2010-08-16 16:21     ` Richard Henderson
  2010-08-16 17:28       ` IainS
  2 siblings, 1 reply; 22+ messages in thread
From: Richard Henderson @ 2010-08-16 16:21 UTC (permalink / raw)
  To: IainS; +Cc: GCC Patches, mrs

On 08/15/2010 12:53 PM, IainS wrote:
> -  enum dwarf_call_frame_info dw_cfi_opc;
> -  dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd1_desc (%1.dw_cfi_opc)")))
> +  unsigned int dw_cfi_opc;
> +  dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd1_desc ((unsigned int)%1.dw_cfi_opc)")))

The cast here is useless.

> @@ -850,6 +868,7 @@ add_fde_cfi (const char *label, dw_cfi_ref cfi)
>  		case DW_CFA_def_cfa_sf:
>  		case DW_CFA_def_cfa_expression:
>  		case DW_CFA_restore_state:
> +		case DW_CFA_INTERNAL_start_epilogue:

Why?  You've already handled that at the top of the function.

> +  else if (cfi->dw_cfi_opc == DW_CFA_INTERNAL_start_epilogue)
> +    {
> +      /* This is a nop unless we want a debug message.  */
> +      if (flag_debug_asm)
> +	fputs (ASM_COMMENT_START"\t\t\t" ASM_COMMENT_START
> +			" DW_CFA_INTERNAL_start_epilogue\n",asm_out_file);

Honestly I don't think you should bother printing these in 
the final version.

> +	  while (cfi->dw_cfi_next) 
> +	    /* Skip to the end.  */
> +	    cfi = cfi->dw_cfi_next;

It's better to move the comment above the while.  As it is, it's hard
to immediately see that the while has one statement.

--

Also, I'm noticing that dwarf2out.c uses flag_asynchronous_unwind_tables
in places where it really means cfun->can_throw_non_call_exceptions.
In fact, almost all occurrences of f_a_u_t in dwarf2out.c are in error.
The only exception that I can see off-hand is in fde_needed_for_eh_p,
which would need an extra check vs cfun->can_throw_non_call_exceptions.

All of which affects the changes that you need to make within toplev.c.


r~

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

* Re: [Patch, _eh, dawin, Version2]  Allow targets to suppress epilogues in _eh frames.
  2010-08-16 16:21     ` Richard Henderson
@ 2010-08-16 17:28       ` IainS
  2010-08-16 18:16         ` Richard Henderson
  0 siblings, 1 reply; 22+ messages in thread
From: IainS @ 2010-08-16 17:28 UTC (permalink / raw)
  To: Richard Henderson; +Cc: GCC Patches, mrs

Hi Richard,

On 16 Aug 2010, at 17:18, Richard Henderson wrote:

<snipped > .... other points taken on board....

> Also, I'm noticing that dwarf2out.c uses  
> flag_asynchronous_unwind_tables
> in places where it really means cfun->can_throw_non_call_exceptions.
> In fact, almost all occurrences of f_a_u_t in dwarf2out.c are in  
> error.
> The only exception that I can see off-hand is in fde_needed_for_eh_p,
> which would need an extra check vs cfun- 
> >can_throw_non_call_exceptions.

How should we proceed on this?

And also there is the remaining question about the GNU-specific code  
(which I fear will get re-enabled when the stuff above is fixed).

... should I expect a dwarf-2-strict target  to be able to interpret  
that code?

... or should I be able to replace it with a (possibly less efficient)  
sequence that is dwarf-2-strict?

> All of which affects the changes that you need to make within  
> toplev.c.

OK - although (independent of this patch) it seems to me that toplev.c  
is overriding a user flag without any diagnostic.

thanks for reviewing,
Iain

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

* Re: [Patch, _eh, dawin, Version2]  Allow targets to suppress epilogues in _eh frames.
  2010-08-16 17:28       ` IainS
@ 2010-08-16 18:16         ` Richard Henderson
  2010-08-17 13:16           ` IainS
  0 siblings, 1 reply; 22+ messages in thread
From: Richard Henderson @ 2010-08-16 18:16 UTC (permalink / raw)
  To: IainS; +Cc: GCC Patches, mrs

On 08/16/2010 09:38 AM, IainS wrote:
> Hi Richard,
> 
> On 16 Aug 2010, at 17:18, Richard Henderson wrote:
> 
> <snipped > .... other points taken on board....
> 
>> Also, I'm noticing that dwarf2out.c uses
>> flag_asynchronous_unwind_tables in places where it really means
>> cfun->can_throw_non_call_exceptions. In fact, almost all
>> occurrences of f_a_u_t in dwarf2out.c are in error. The only
>> exception that I can see off-hand is in fde_needed_for_eh_p, which
>> would need an extra check vs cfun->can_throw_non_call_exceptions.
> 
> How should we proceed on this?

Patch dwarf2out.c?  How else?

> And also there is the remaining question about the GNU-specific code
> (which I fear will get re-enabled when the stuff above is fixed).

What remaining question?  What GNU-specific code?


r~

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

* Re: [Patch, _eh, dawin, Version2]  Allow targets to suppress epilogues in _eh frames.
  2010-08-16 18:16         ` Richard Henderson
@ 2010-08-17 13:16           ` IainS
  2010-08-17 15:00             ` Richard Henderson
  0 siblings, 1 reply; 22+ messages in thread
From: IainS @ 2010-08-17 13:16 UTC (permalink / raw)
  To: Richard Henderson; +Cc: GCC Patches, mrs

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


On 16 Aug 2010, at 18:44, Richard Henderson wrote:

> On 08/16/2010 09:38 AM, IainS wrote:
>> On 16 Aug 2010, at 17:18, Richard Henderson wrote:
>>
>> <snipped > .... other points taken on board....
>>
>>> Also, I'm noticing that dwarf2out.c uses
>>> flag_asynchronous_unwind_tables in places where it really means
>>> cfun->can_throw_non_call_exceptions. In fact, almost all
>>> occurrences of f_a_u_t in dwarf2out.c are in error. The only
>>> exception that I can see off-hand is in fde_needed_for_eh_p, which
>>> would need an extra check vs cfun->can_throw_non_call_exceptions.
>>
>> How should we proceed on this?
> Patch dwarf2out.c?  How else?

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?

>> And also there is the remaining question about the GNU-specific code
>> (which I fear will get re-enabled when the stuff above is fixed).
>
> What remaining question?  What GNU-specific code?

this....

static void
dwarf2out_args_size (const char *label, HOST_WIDE_INT size)
{
   dw_cfi_ref cfi;

   if (size == old_args_size)
     return;

   old_args_size = size;

   cfi = new_cfi ();
   cfi->dw_cfi_opc = DW_CFA_GNU_args_size;  <<<============
   cfi->dw_cfi_oprnd1.dw_cfi_offset = size;
   add_fde_cfi (label, cfi);
}

is a dwarf-2 target supposed to respond to this (i.e. we have a darwin  
bug if it doesn't work) or
... should there be a fall-back for dwarf-2 strict?

=====

I'm attaching a consolidated patch which takes on board the comments  
you've made.
It regtests on i686-darwin9 and java functions on darwin10 with it in  
place, the same patch also restores Java functionality on 4.5.2.

(I've left two debug prints in place for now - accepting that they  
need to be removed before it's applied).

I'll cross-reference this in PR41991 in case someone else wants to  
pick it up but, unfortunately, I don't have time to advance it at the  
moment.

cheers,
Iain


[-- Attachment #2: 163302-unwind-epilogues-v2a-diff.txt --]
[-- Type: text/plain, Size: 12845 bytes --]

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 163302)
+++ 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)")))
@@ -821,9 +833,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 ();
@@ -2894,12 +2912,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
@@ -2941,11 +2969,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)
     {
@@ -2953,6 +2979,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:
@@ -2990,11 +3017,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)
     {
@@ -3090,6 +3115,7 @@ switch_to_frame_table_section (int for_eh, bool ba
 	debug_frame_section = get_section (DEBUG_FRAME_SECTION,
 					   SECTION_DEBUG, NULL);
       switch_to_section (debug_frame_section);
+      TGT_ASM_DBG_SECTION_LABEL (debug_frame_section);
     }
 }
 
@@ -3121,6 +3147,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,
@@ -3303,6 +3331,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;
@@ -3498,6 +3530,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
@@ -3613,13 +3683,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;
 	  }
@@ -3627,7 +3697,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;
@@ -3636,7 +3705,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 163302)
+++ gcc/toplev.c	(working copy)
@@ -1798,9 +1801,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-unwind1.C
===================================================================
--- gcc/testsuite/g++.dg/eh/async-unwind1.C	(revision 163302)
+++ 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/testsuite/g++.dg/eh/async-unwind2.C
===================================================================
--- gcc/testsuite/g++.dg/eh/async-unwind2.C	(revision 163302)
+++ 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/config/darwin.c
===================================================================
--- gcc/config/darwin.c	(revision 163302)
+++ gcc/config/darwin.c	(working copy)
@@ -1866,14 +1897,30 @@ darwin_kextabi_p (void) {
   return flag_apple_kext;
 }
 
+/* Invoked from SUBSUBTARGET_OVERRIDE_OPTIONS from rs6000.c and 
+   i386.c, this is processed after any arch-specific overrides.  */
 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 +1932,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 +1948,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 163302)
+++ gcc/config/i386/darwin.h	(working copy)
@@ -245,6 +245,41 @@ 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)							\
+	/* Emit Unwind tables.  */					\
+	flag_unwind_tables = 1;						\
+      else 								\
+	{								\
+	  if (flag_omit_frame_pointer == 1)				\
+	    /* The user has asked to omit fp - emit Unwind tables.  */  \
+	      flag_unwind_tables = 1;					\
+	  else 								\
+	    /* Use the frame pointer.  */				\
+	    flag_omit_frame_pointer = 0;				\
+	}								\
+    }									\
+  /* Do not allow the subtarget to switch off frame pointers for 	\
+     earlier versions of Darwin.  */					\
+  if (flag_omit_frame_pointer == 2 && !darwin9plus)			\
+    flag_omit_frame_pointer = 0;					\
+  if (flag_omit_frame_pointer >= 1 && flag_unwind_tables == 1)		\
+    target_flags |= MASK_ACCUMULATE_OUTGOING_ARGS;			\
+  /* Do not set asynchronous_unwinding (yet) unless the user		\
+    specifically asks,.  */						\
+  if (flag_asynchronous_unwind_tables == 2)				\
+    flag_asynchronous_unwind_tables = 0;				\
+  if (TARGET_64BIT && MACHO_DYNAMIC_NO_PIC_P)				\
+    target_flags &= ~MASK_MACHO_DYNAMIC_NO_PIC;				\
+} 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

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





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

* Re: [Patch, _eh, dawin, Version2]  Allow targets to suppress epilogues in _eh frames.
  2010-08-17 13:16           ` IainS
@ 2010-08-17 15:00             ` Richard Henderson
  2010-08-17 15:16               ` Jack Howarth
  2010-09-15 14:48               ` IainS
  0 siblings, 2 replies; 22+ messages in thread
From: Richard Henderson @ 2010-08-17 15:00 UTC (permalink / raw)
  To: IainS; +Cc: GCC Patches, mrs

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.


r~

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

* Re: [Patch, _eh, dawin, Version2]  Allow targets to suppress epilogues in _eh frames.
  2010-08-17 15:00             ` Richard Henderson
@ 2010-08-17 15:16               ` Jack Howarth
  2010-09-15 14:48               ` IainS
  1 sibling, 0 replies; 22+ messages in thread
From: Jack Howarth @ 2010-08-17 15:16 UTC (permalink / raw)
  To: Richard Henderson; +Cc: IainS, GCC Patches, mrs

On Tue, Aug 17, 2010 at 07:57:10AM -0700, 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.
> 

Richard,
    In Darwin10, Apple provides two unwinders in libSystem. The
default unwinder in Snow Leopard is the compact unwinder and this
is preselected through the linker. There is a second compatibility
unwinder which can be used by passing -no_compact_unwind to ld.
The compatibility unwinder appears to be based on the original
libgcc 4.2.1 unwinder and does understand the GNU extensions
available up to that release. The compact unwinder is coded to
the standards only and doesn't support those (as can be seen
by linking with -warn_compact_unwind). Currently we are linking
in FSF gcc trunk and 4.5.x with -no_compact_unwind to have
access to all of the GNU extensions for compatibility with
the existing c++ and java eh handling.
     I would rather put off switching FSF gcc on darwin to
the compact unwinder until FSF gcc 4.7 and darwin11. This
will give us a lot of time to test compatibility and file
radar reports against the compact unwinder.
           Jack

> 
> r~

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

* Re: [Patch, _eh, dawin, Version2]  Allow targets to suppress epilogues in _eh frames.
  2010-08-17 15:00             ` Richard Henderson
  2010-08-17 15:16               ` Jack Howarth
@ 2010-09-15 14:48               ` IainS
  2010-09-15 19:49                 ` Jack Howarth
  2010-09-17 14:05                 ` Jack Howarth
  1 sibling, 2 replies; 22+ messages in thread
From: IainS @ 2010-09-15 14:48 UTC (permalink / raw)
  To: Richard Henderson; +Cc: GCC Patches, mrs

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.


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

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

* Re: [Patch, _eh, dawin, Version2]  Allow targets to suppress epilogues in _eh frames.
  2010-09-15 14:48               ` IainS
@ 2010-09-15 19:49                 ` Jack Howarth
  2010-09-17 14:05                 ` Jack Howarth
  1 sibling, 0 replies; 22+ messages in thread
From: Jack Howarth @ 2010-09-15 19:49 UTC (permalink / raw)
  To: IainS; +Cc: Richard Henderson, GCC Patches, mrs

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

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

* Re: [Patch, _eh, dawin, Version2]  Allow targets to suppress epilogues in _eh frames.
  2010-09-15 14:48               ` IainS
  2010-09-15 19:49                 ` Jack Howarth
@ 2010-09-17 14:05                 ` Jack Howarth
  2010-09-17 21:39                   ` IainS
  1 sibling, 1 reply; 22+ messages in thread
From: Jack Howarth @ 2010-09-17 14:05 UTC (permalink / raw)
  To: IainS; +Cc: Richard Henderson, GCC Patches, mrs

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

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

* Re: [Patch, _eh, dawin, Version2]  Allow targets to suppress epilogues in _eh frames.
  2010-09-17 14:05                 ` Jack Howarth
@ 2010-09-17 21:39                   ` IainS
  0 siblings, 0 replies; 22+ messages in thread
From: IainS @ 2010-09-17 21:39 UTC (permalink / raw)
  To: Jack Howarth; +Cc: Richard Henderson, GCC Patches, mrs


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

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

end of thread, other threads:[~2010-09-17 21:06 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-13 20:34 [Patch, _eh, dawin] Allow targets to suppress epilogues in _eh frames IainS
2010-08-14  0:10 ` Richard Henderson
2010-08-15 20:03   ` [Patch, _eh, dawin, Version2] " IainS
2010-08-16  1:18     ` Jack Howarth
2010-08-16  9:20       ` IainS
2010-08-16  4:15     ` Jack Howarth
2010-08-16  7:46       ` Jack Howarth
2010-08-16 10:07         ` IainS
2010-08-16 11:58           ` Jack Howarth
2010-08-16 16:21     ` Richard Henderson
2010-08-16 17:28       ` IainS
2010-08-16 18:16         ` Richard Henderson
2010-08-17 13:16           ` IainS
2010-08-17 15:00             ` Richard Henderson
2010-08-17 15:16               ` Jack Howarth
2010-09-15 14:48               ` IainS
2010-09-15 19:49                 ` Jack Howarth
2010-09-17 14:05                 ` Jack Howarth
2010-09-17 21:39                   ` IainS
2010-08-14  4:23 ` [Patch, _eh, dawin] " Jack Howarth
2010-08-14 15:21   ` IainS
2010-08-15 15:34     ` Mike Stump

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