| Message ID | yddiptqbrdb.fsf@manam.CeBiTec.Uni-Bielefeld.DE |
|---|---|
| State | New |
| Headers | show |
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.
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
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
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
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.