diff mbox

[Darwin] Fix PR46904/PR46916

Message ID D67DFCA2-7487-4B35-898C-C861C6BFF9E8@sandoe-acoustics.co.uk
State New
Headers show

Commit Message

Iain Sandoe Dec. 17, 2010, 9:37 a.m. UTC
Hi,
This has now been regstrapped on *-darwin9 and x86_64-darwin10.

There are outstanding issues with -freorder-blocks-and-partition (on  
Darwin, at least) which results in the fail of partition2.C on Darwin10.

IMO it would be better to fix these separately :

  o -freorder-blocks-and-partition - seems to suppress the pubnames  
section (Honza, any ideas?) - this causes a dsymutil fail.
  o -freorder-blocks-and-partition - produces anonymous code atoms  
(chunks introduced only by local labels) which produces linker  
warnings, because it is not keen on adding line number info to  
anonymous code chunks.

Unless people feel those issues should be resolved in this patch ....
OK for trunk?
Iain

gcc:

	Iain Sandoe  <iains...
	Jan Hubicka  <hubika...

	PR middle-end/46916
	PR c++/46904

	* opts.c (finish_options): Enable -freorder-functions
	when -freorder-blocks-and-partition is active.
	* config/darwin.c (generating_for_darwin_version): New var.
	(darwin_text_section): Remove.
	(darwin_rodata_section): Do not check function section here.
	(darwin_emit_unwind_label): Do not emit for Darwin >= 9.
	Emit distinct labels for successive calls for the same decl.
	(darwin_override_options): Set generating_for_darwin_version.
	Suppress automatic asynchronous unwind tables for m32.
	Switch off -freorder-blocks-and-partition when unwind tables
	will be generated.  Update to use generating_for_darwin_version.
	(darwin_function_section): Check for cases that need to be placed
	in coalesced sections.
	* config/darwin-sections.def: Define hot, cold, startup and exit  
sections
	for both coalesced and regular code.
	* config/darwin.h (USE_SELECT_SECTION_FOR_FUNCTIONS): Delete.
	* config/darwin10.h (TARGET_ASM_EMIT_UNWIND_LABEL): Delete.

Comments

Mike Stump Dec. 18, 2010, 6:58 p.m. UTC | #1
On Dec 17, 2010, at 1:37 AM, IainS wrote:
> This has now been regstrapped on *-darwin9 and x86_64-darwin10.
> 
> There are outstanding issues with -freorder-blocks-and-partition (on Darwin, at least) which results in the fail of partition2.C on Darwin10.

:-(  I kinda hate regressions....  and the opt.c change I kinda wish were broken out, either, it goes in first, or it goes in second.

Normally, I'd say since I think the changes for the two remaining problems are well underway, I'd propose to stage in the fixes for those first, before the below goes in, so that when it does go in, there are no regressions, but, the problem with that is this patch is to `fix' regressions, and as I said, I hate regressions....

so...

> OK for trunk?

Ok.


Though, that said, I'd rather have the patch that originally broke, backed out to fix the immediate regression, then the sub problems found fixed first, as prep work for this patch, then this patch and the other fix that was reverted could go in together, all, without regressions.  We are late enough in the cycle, that having regressions is, well, let me put it this way, I'd still like to see a bot that just reverts any patch that causes a regression, no exceptions.  I still think it would be a nice property to have.
Jack Howarth Dec. 18, 2010, 9:09 p.m. UTC | #2
On Sat, Dec 18, 2010 at 10:58:01AM -0800, Mike Stump wrote:
> On Dec 17, 2010, at 1:37 AM, IainS wrote:
> > This has now been regstrapped on *-darwin9 and x86_64-darwin10.
> > 
> > There are outstanding issues with -freorder-blocks-and-partition (on Darwin, at least) which results in the fail of partition2.C on Darwin10.
> 
> :-(  I kinda hate regressions....  and the opt.c change I kinda wish were broken out, either, it goes in first, or it goes in second.

Mike,
    I would consider partition2.C's current passes a bit suspect anyway as we have no idea whether
PR45646 was fixed or went latent. Oddly, I'm not seeing the partition2.C failures here on x86_64-apple-darwin10
with Iain's patch under Xcode 3.2.5.
             Jack

> 
> Normally, I'd say since I think the changes for the two remaining problems are well underway, I'd propose to stage in the fixes for those first, before the below goes in, so that when it does go in, there are no regressions, but, the problem with that is this patch is to `fix' regressions, and as I said, I hate regressions....
> 
> so...
> 
> > OK for trunk?
> 
> Ok.
> 
> 
> Though, that said, I'd rather have the patch that originally broke, backed out to fix the immediate regression, then the sub problems found fixed first, as prep work for this patch, then this patch and the other fix that was reverted could go in together, all, without regressions.  We are late enough in the cycle, that having regressions is, well, let me put it this way, I'd still like to see a bot that just reverts any patch that causes a regression, no exceptions.  I still think it would be a nice property to have.
diff mbox

Patch

Index: gcc/opts.c
===================================================================
--- gcc/opts.c	(revision 167963)
+++ gcc/opts.c	(working copy)
@@ -755,6 +755,10 @@  finish_options (struct gcc_options *opts, struct g
       opts->x_flag_reorder_blocks = 1;
     }
 
+  if (opts->x_flag_reorder_blocks_and_partition
+      && !opts_set->x_flag_reorder_functions)
+    opts->x_flag_reorder_functions = 1;
+
   /* Pipelining of outer loops is only possible when general pipelining
      capabilities are requested.  */
   if (!opts->x_flag_sel_sched_pipelining)
Index: gcc/config/darwin.c
===================================================================
--- gcc/config/darwin.c	(revision 167963)
+++ gcc/config/darwin.c	(working copy)
@@ -90,6 +90,9 @@  int darwin_emit_branch_islands = false;
    functions).  */
 int darwin_running_cxx;
 
+/* Some code-gen now depends on OS major version numbers (at least).  */
+int generating_for_darwin_version ;
+
 /* Section names.  */
 section * darwin_sections[NUM_DARWIN_SECTIONS];
 
@@ -1145,19 +1148,6 @@  darwin_mark_decl_preserved (const char *name)
 }
 
 static section *
-darwin_text_section (int reloc, int weak)
-{
-  if (reloc)
-    return (weak
-	    ? darwin_sections[text_unlikely_coal_section]
-	    : unlikely_text_section ());
-  else
-    return (weak
-	    ? darwin_sections[text_coal_section]
-	    : text_section);
-}
-
-static section *
 darwin_rodata_section (int weak, bool zsize)
 {
   return (weak
@@ -1267,17 +1257,7 @@  machopic_select_section (tree decl,
   switch (categorize_decl_for_section (decl, reloc))
     {
     case SECCAT_TEXT:
-      {
-	struct cgraph_node *node;
-	if (decl && TREE_CODE (decl) == FUNCTION_DECL
-	    && (node = cgraph_get_node (decl)) != NULL)
-	  base_section = darwin_function_section (decl,
-						  node->frequency,
-						  node->only_called_at_startup,
-						  node->only_called_at_exit);
-	if (!base_section)
-          base_section = darwin_text_section (reloc, weak);
-      }
+      gcc_unreachable ();
       break;
 
     case SECCAT_RODATA:
@@ -1684,13 +1664,38 @@  darwin_handle_weak_import_attribute (tree *node, t
 void
 darwin_emit_unwind_label (FILE *file, tree decl, int for_eh, int empty)
 {
-  char *lab;
-
-  if (! for_eh)
+  char *lab ;
+  char buf[32];
+  static int invok_count = 0;
+  static tree last_fun_decl = NULL_TREE;
+  
+  /* We use the linker to emit the .eh labels for Darwin 9 and above.  */
+  if (! for_eh || generating_for_darwin_version >= 9)
     return;
 
-  lab = concat (IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), ".eh", NULL);
+  /* FIXME: This only works when the eh for all sections of a function is 
+     emitted at the same time.  If that changes, we would need to use a lookup
+     table of some form to determine what to do.  Also, we should emit the
+     unadorned label for the partition containing the public label for a
+     function.  This is of limited use, probably, since we do not currently
+     enable partitioning.  */
+  strcpy (buf, ".eh");
+  if (decl && TREE_CODE (decl) == FUNCTION_DECL) 
+    {
+      if (decl == last_fun_decl)
+        {
+	  invok_count++;
+	  snprintf (buf, 31, "$$part$$%d.eh", invok_count);
+	}
+      else
+	{
+	  last_fun_decl = decl;
+	  invok_count = 0;
+	}
+    }
 
+  lab = concat (IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), buf, NULL);
+
   if (TREE_PUBLIC (decl))
     {
       targetm.asm_out.globalize_label (file, lab);
@@ -2551,21 +2556,49 @@  darwin_kextabi_p (void) {
 void
 darwin_override_options (void)
 {
-  bool darwin9plus = (darwin_macosx_version_min
-		      && strverscmp (darwin_macosx_version_min, "10.5") >= 0);
+  /* Keep track of which (major) version we're generating code for.  */
+  if (darwin_macosx_version_min)
+    {
+      if (strverscmp (darwin_macosx_version_min, "10.6") >= 0)
+	generating_for_darwin_version = 10;
+      else if (strverscmp (darwin_macosx_version_min, "10.5") >= 0)
+	generating_for_darwin_version = 9;
 
+      /* Earlier versions are not specifically accounted, until required.  */
+    }
+
   /* Don't emit DWARF3/4 unless specifically selected.  This is a 
      workaround for tool bugs.  */
   if (!global_options_set.x_dwarf_strict) 
     dwarf_strict = 1;
 
-  /* 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))
+  /* Do not allow unwind tables to be generated by default for m32.  
+     fnon-call-exceptions will override this, regardless of what we do.  */
+  if (generating_for_darwin_version < 10
+      && !global_options_set.x_flag_asynchronous_unwind_tables
+      && !TARGET_64BIT)
+    global_options.x_flag_asynchronous_unwind_tables = 0;
+
+  /* Disable -freorder-blocks-and-partition when unwind tables are being emitted
+     for Darwin < 10 (OSX 10.6).  
+     The strategy is, "Unless the User has specifically set/unset an unwind flag
+     we will switch off -freorder-blocks-and-partition when unwind tables will be
+     generated".  If the User specifically sets flags... we assume (s)he knows
+     why...  */
+   if (generating_for_darwin_version < 10
+       && global_options_set.x_flag_reorder_blocks_and_partition
+       && ((global_options.x_flag_exceptions 		/* User, c++, java */
+	    && !global_options_set.x_flag_exceptions) 	/* User specified... */
+	   || (global_options.x_flag_unwind_tables
+		&& !global_options_set.x_flag_unwind_tables)
+	   || (global_options.x_flag_non_call_exceptions
+		&& !global_options_set.x_flag_non_call_exceptions)
+	   || (global_options.x_flag_asynchronous_unwind_tables
+		&& !global_options_set.x_flag_asynchronous_unwind_tables)))
     {
       inform (input_location,
-              "-freorder-blocks-and-partition does not work with exceptions "
-              "on this architecture");
+	      "-freorder-blocks-and-partition does not work with exceptions "
+	      "on this architecture");
       flag_reorder_blocks_and_partition = 0;
       flag_reorder_blocks = 1;
     }
@@ -2590,7 +2623,7 @@  darwin_override_options (void)
     }
 
   if (flag_var_tracking
-      && darwin9plus
+      && (generating_for_darwin_version >= 9)
       && debug_info_level >= DINFO_LEVEL_NORMAL
       && debug_hooks->var_location != do_nothing_debug_hooks.var_location)
     flag_var_tracking_uninit = 1;
@@ -2608,7 +2641,7 @@  darwin_override_options (void)
     }
 
   /* It is assumed that branch island stubs are needed for earlier systems.  */
-  if (!darwin9plus)
+  if (generating_for_darwin_version < 9)
     darwin_emit_branch_islands = true;
   else
     emit_aligned_common = true; /* Later systems can support aligned common.  */
@@ -2968,33 +3001,56 @@  section *
 darwin_function_section (tree decl, enum node_frequency freq,
 			  bool startup, bool exit)
 {
+  /* Decide if we need to put this in a coalescable section.  */
+  bool weak = (decl 
+	       && DECL_WEAK (decl)
+	       && (!DECL_ATTRIBUTES (decl)
+		   || !lookup_attribute ("weak_import", 
+					  DECL_ATTRIBUTES (decl))));
+
+  /* If there is a specified section name, we should not be trying to
+     override.  */
+  if (decl && DECL_SECTION_NAME (decl) != NULL_TREE)
+    return get_named_section (decl, NULL, 0);
+
+  /* Default when there is no function re-ordering.  */
   if (!flag_reorder_functions)
-    return NULL;
+    return (weak)
+	    ? darwin_sections[text_coal_section]
+	    : text_section;
+
   /* Startup code should go to startup subsection unless it is
      unlikely executed (this happens especially with function splitting
-     where we can split away unnecesary parts of static constructors.  */
+     where we can split away unnecesary parts of static constructors).  */
   if (startup && freq != NODE_FREQUENCY_UNLIKELY_EXECUTED)
-    return get_named_text_section
-	     (decl, "__TEXT,__startup,regular,pure_instructions", "_startup");
+    return (weak)
+	    ? darwin_sections[text_startup_coal_section]
+	    : darwin_sections[text_startup_section];
 
   /* Similarly for exit.  */
   if (exit && freq != NODE_FREQUENCY_UNLIKELY_EXECUTED)
-    return get_named_text_section (decl,
-				   "__TEXT,__exit,regular,pure_instructions",
-				   "_exit");
+    return (weak)
+	    ? darwin_sections[text_exit_coal_section]
+	    : darwin_sections[text_exit_section];
 
   /* Group cold functions together, similarly for hot code.  */
   switch (freq)
     {
       case NODE_FREQUENCY_UNLIKELY_EXECUTED:
-	return get_named_text_section
-		 (decl,
-	          "__TEXT,__unlikely,regular,pure_instructions", "_unlikely");
+	return (weak)
+		? darwin_sections[text_cold_coal_section]
+		: darwin_sections[text_cold_section];
+	break;
       case NODE_FREQUENCY_HOT:
-	return get_named_text_section
-		 (decl, "__TEXT,__hot,regular,pure_instructions", "_hot");
+	return (weak)
+		? darwin_sections[text_hot_coal_section]
+		: darwin_sections[text_hot_section];
+	break;
       default:
-	return NULL;
+	return (weak)
+		? darwin_sections[text_coal_section]
+		: text_section;
+	break;
     }
 }
 
Index: gcc/config/darwin-sections.def
===================================================================
--- gcc/config/darwin-sections.def	(revision 167963)
+++ gcc/config/darwin-sections.def	(working copy)
@@ -34,6 +34,24 @@  DEF_SECTION (text_unlikely_coal_section, SECTION_C
 	     ".section __TEXT,__text_unlikely_coal,"
 	     "coalesced,pure_instructions", 0)
 
+DEF_SECTION (text_hot_section, SECTION_CODE,
+	     ".section __TEXT,__text_hot,regular,pure_instructions", 0)
+DEF_SECTION (text_cold_section, SECTION_CODE,
+	     ".section __TEXT,__text_cold,regular,pure_instructions", 0)
+DEF_SECTION (text_startup_section, SECTION_CODE,
+	     ".section __TEXT,__text_startup,regular,pure_instructions", 0)
+DEF_SECTION (text_exit_section, SECTION_CODE,
+	     ".section __TEXT,__text_exit,regular,pure_instructions", 0)
+
+DEF_SECTION (text_hot_coal_section, SECTION_CODE,
+	     ".section __TEXT,__text_hot_coal,coalesced,pure_instructions", 0)
+DEF_SECTION (text_cold_coal_section, SECTION_CODE,
+	     ".section __TEXT,__text_cold_coal,coalesced,pure_instructions", 0)
+DEF_SECTION (text_startup_coal_section, SECTION_CODE,
+	     ".section __TEXT,__text_stt_coal,coalesced,pure_instructions", 0)
+DEF_SECTION (text_exit_coal_section, SECTION_CODE,
+	     ".section __TEXT,__text_exit_coal,coalesced,pure_instructions", 0)
+
 /* const */
 DEF_SECTION (const_section, 0, ".const", 0)
 DEF_SECTION (const_coal_section, SECTION_NO_ANCHOR,
Index: gcc/config/darwin.h
===================================================================
--- gcc/config/darwin.h	(revision 167963)
+++ gcc/config/darwin.h	(working copy)
@@ -664,7 +669,7 @@  extern GTY(()) section * darwin_sections[NUM_DARWI
 
 #undef	TARGET_ASM_SELECT_SECTION
 #define TARGET_ASM_SELECT_SECTION machopic_select_section
-#define USE_SELECT_SECTION_FOR_FUNCTIONS
+
 #undef	TARGET_ASM_FUNCTION_SECTION
 #define TARGET_ASM_FUNCTION_SECTION darwin_function_section
 
Index: gcc/config/darwin10.h
===================================================================
--- gcc/config/darwin10.h	(revision 167963)
+++ gcc/config/darwin10.h	(working copy)
@@ -23,8 +23,3 @@  unwinder in libSystem is fixed to digest new epilo
 
 #undef LIB_SPEC
 #define LIB_SPEC "%{!static:-no_compact_unwind -lSystem}"
-
-/* Unwind labels are no longer required in darwin10.  */
-
-#undef TARGET_ASM_EMIT_UNWIND_LABEL
-#define TARGET_ASM_EMIT_UNWIND_LABEL default_emit_unwind_label