Patchwork [build] Define HAVE_GAS_HIDDEN on Darwin

login
register
mail settings
Submitter Rainer Orth
Date May 4, 2011, 12:08 p.m.
Message ID <yddiptqbrdb.fsf@manam.CeBiTec.Uni-Bielefeld.DE>
Download mbox | patch
Permalink /patch/94012/
State New
Headers show

Comments

Rainer Orth - May 4, 2011, 12:08 p.m.
The following patch is a prerequisite for making

	[lto, testsuite] Don't use visibility on targets that don't support it (PR lto/47334)
        http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00295.html

work on Darwin by defining HAVE_GAS_HIDDEN on that target, too.  Instead
of special-casing visibility support on Darwin all over the place, I'd
rather treat it like the other targets that have at least some
visibility support, even if not the full ELF range of visibility types.

Doing this turned out to be astonishingly easy:

* Enable gcc_cv_as_hidden and gcc_cv_ld_hidden in configure.ac.

* Disable rs6000_assemble_visibility if TARGET_MACHO since Darwin has
  (and needs) its own version.

* The code in config/i386/i386.c for USE_HIDDEN_LINKONCE now does the
  right thing even without TARGET_MACHO.

* On the other hand, USE_LINKONCE_INDIRECT in dwarf2asm.c must be
  disabled for Mach-O since the code uses an explicit .hidden, which
  could be considered a bug in itself ;-(

This patch survived bootstrap on i386-apple-darwin9.8.0 (both multilibs)
and powerpc-apple-darwin9.8.0 (32-bit only).  I've compared testresults
with a slightly earlier build (the machines are relatively slow), but a
real regtest is in progress.

I'd appreciate if Darwin maintainers and other Darwin users could try
this themselves since my experience with the platform is rather limited.

While one could argue that HAVE_GAS_HIDDEN is complete misnomer now and
should rather be HAVE_AS_VISIBILITY instead, I'd prefer to do this as a
followup if at all.

Ok for mainline if testing passes and no other problems show up?

Thanks.
	Rainer


2011-04-30  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* configure.ac (gcc_cv_as_hidden): Enable on *-*-darwin*.
	(gcc_cv_ld_hidden): Likewise.
	* configure: Regenerate.
	* config/i386/i386.c (USE_HIDDEN_LINKONCE): Remove TARGET_MACHO.
	* config/rs6000/rs6000.c (rs6000_assemble_visibility)
	[TARGET_MACHO]: Don't define.
	(TARGET_ASM_ASSEMBLE_VISIBILITY): Likewise.
	* dwarf2asm.c (USE_LINKONCE_INDIRECT) [HAVE_GAS_HIDDEN]: Only if
	!TARGET_MACHO.
Mike Stump - May 10, 2011, 12:43 a.m.
On May 4, 2011, at 5:08 AM, Rainer Orth wrote:
> The following patch is a prerequisite for making
> 
> 	[lto, testsuite] Don't use visibility on targets that don't support it (PR lto/47334)
>        http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00295.html

> -# define USE_LINKONCE_INDIRECT (SUPPORTS_ONE_ONLY)
> +# define USE_LINKONCE_INDIRECT (SUPPORTS_ONE_ONLY) && !TARGET_MACHO

Generally speaking, we don't litter the backend with things like this.  We consider this trashy, and we limit the trash to config/...

Now, if you invent a feature (bug) for which this is really testing, and used it instead here, and then put that into the darwin.h file or into an as autoconf test, I think it would be fine.  Ok with that version.  If a build/configure/visibility person wants to object or insist on a better way to do what you want to do, I'd defer to them.
Rainer Orth - May 19, 2011, 5:33 p.m.
Mike Stump <mikestump@comcast.net> writes:

> On May 4, 2011, at 5:08 AM, Rainer Orth wrote:
>> The following patch is a prerequisite for making
>> 
>> 	[lto, testsuite] Don't use visibility on targets that don't support it (PR lto/47334)
>>        http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00295.html
>
>> -# define USE_LINKONCE_INDIRECT (SUPPORTS_ONE_ONLY)
>> +# define USE_LINKONCE_INDIRECT (SUPPORTS_ONE_ONLY) && !TARGET_MACHO
>
> Generally speaking, we don't litter the backend with things like this.
> We consider this trashy, and we limit the trash to config/...
>
> Now, if you invent a feature (bug) for which this is really testing,
> and used it instead here, and then put that into the darwin.h file or
> into an as autoconf test, I think it would be fine.  Ok with that
> version.  If a build/configure/visibility person wants to object or
> insist on a better way to do what you want to do, I'd defer to them.

The problem is the assumption that HAVE_GAS_HIDDEN implies that the
assembler supports the .hidden mnemonic, not visibility in general,
which is no longer true now.

Unfortunately, one cannot use targetm.asm_out.assemble_visibility in
dwarf2asm.c (dw2_output_indirect_constant_1) since the former expects a
tree arg, not an arbitrary string.

The cleanest way to account for this seems to allow overriding
USE_LINKONCE_INDIRECT in target headers (darwin.h in this case).

If Jason or Richard consider this appropriate, I'll modify the patch
accordingly and apply after retesting.

	Rainer
Jason Merrill - May 19, 2011, 6:51 p.m.
On 05/19/2011 01:33 PM, Rainer Orth wrote:
> The cleanest way to account for this seems to allow overriding
> USE_LINKONCE_INDIRECT in target headers (darwin.h in this case).
>
> If Jason or Richard consider this appropriate, I'll modify the patch
> accordingly and apply after retesting.

Not really.  The use in dwarf2asm seems no different from the use in 
i386.c, except that the latter has a special TARGET_MACHO case instead. 
  As you say, dwarf2asm shouldn't be using .hidden directly, and that's 
the bug to fix.

Jason

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -8787,7 +8787,7 @@  ix86_setup_frame_addresses (void)
 }
 
 #ifndef USE_HIDDEN_LINKONCE
-# if (defined(HAVE_GAS_HIDDEN) && (SUPPORTS_ONE_ONLY - 0)) || TARGET_MACHO
+# if defined(HAVE_GAS_HIDDEN) && (SUPPORTS_ONE_ONLY - 0)
 #  define USE_HIDDEN_LINKONCE 1
 # else
 #  define USE_HIDDEN_LINKONCE 0
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -916,7 +916,7 @@  static bool legitimate_lo_sum_address_p 
 static struct machine_function * rs6000_init_machine_status (void);
 static bool rs6000_assemble_integer (rtx, unsigned int, int);
 static bool no_global_regs_above (int, bool);
-#ifdef HAVE_GAS_HIDDEN
+#if defined (HAVE_GAS_HIDDEN) && !defined (TARGET_MACHO)
 static void rs6000_assemble_visibility (tree, int);
 #endif
 static int rs6000_ra_ever_killed (void);
@@ -1381,7 +1381,7 @@  static const struct default_options rs60
 #undef TARGET_ASM_INTEGER
 #define TARGET_ASM_INTEGER rs6000_assemble_integer
 
-#ifdef HAVE_GAS_HIDDEN
+#if defined (HAVE_GAS_HIDDEN) && !defined (TARGET_MACHO)
 #undef TARGET_ASM_ASSEMBLE_VISIBILITY
 #define TARGET_ASM_ASSEMBLE_VISIBILITY rs6000_assemble_visibility
 #endif
@@ -16710,7 +16710,7 @@  rs6000_assemble_integer (rtx x, unsigned
   return default_assemble_integer (x, size, aligned_p);
 }
 
-#ifdef HAVE_GAS_HIDDEN
+#if defined (HAVE_GAS_HIDDEN) && !defined (TARGET_MACHO)
 /* Emit an assembler directive to set symbol visibility for DECL to
    VISIBILITY_TYPE.  */
 
diff --git a/gcc/configure.ac b/gcc/configure.ac
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -2174,6 +2174,12 @@  EOF
     gcc_cv_as_hidden=yes
     ;;
 esac])
+case "${target}" in
+  *-*-darwin*)
+    # Darwin as has some visibility support, though with a different syntax.
+    gcc_cv_as_hidden=yes
+    ;;
+esac
 
 # gnu_indirect_function type is an extension proposed at
 # http://groups.google/com/group/generic-abi/files. It allows dynamic runtime
@@ -2273,6 +2279,10 @@  else
     fi
   else
     case "${target}" in
+      *-*-darwin*)
+	# Darwin ld has some visibility support.
+	gcc_cv_ld_hidden=yes
+        ;;
       hppa64*-*-hpux* | ia64*-*-hpux*)
 	gcc_cv_ld_hidden=yes
 	;;
diff --git a/gcc/dwarf2asm.c b/gcc/dwarf2asm.c
--- a/gcc/dwarf2asm.c
+++ b/gcc/dwarf2asm.c
@@ -1,5 +1,5 @@ 
 /* Dwarf2 assembler output helper routines.
-   Copyright (C) 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2009, 2010
+   Copyright (C) 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2009, 2010, 2011
    Free Software Foundation, Inc.
 
 This file is part of GCC.
@@ -799,7 +799,7 @@  static GTY((param1_is (char *), param2_i
 static GTY(()) int dw2_const_labelno;
 
 #if defined(HAVE_GAS_HIDDEN)
-# define USE_LINKONCE_INDIRECT (SUPPORTS_ONE_ONLY)
+# define USE_LINKONCE_INDIRECT (SUPPORTS_ONE_ONLY) && !TARGET_MACHO
 #else
 # define USE_LINKONCE_INDIRECT 0
 #endif