This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: libgcc move to the top level


On Fri, Dec 29, 2006 at 12:27:42PM +0100, Paolo Bonzini wrote:
> Why the dependencies?

Stray copy from libiberty.  Fixed.

> This is a good candidate to be set in config.host (only Darwin
> sets it, and it's already converted to toplevel libgcc) instead
> of the Makefile.

OK.

> Consider moving this to configure.ac, after the config.host fragment. 
> You can reuse some code in libffi's configure.ac, like this:

OK.

> >+ifeq ($(LIB2_SIDITI_CONV_FUNCS),)
> >+  lib2funcs += $(subst XX,si,$(swfloatfuncs))
> >+  lib2funcs += $(subst XX,di,$(dwfloatfuncs))
> >+endif
> >...
> >+ifneq ($(LIB2_SIDITI_CONV_FUNCS),)
> >+# Build libgcc2.c for each conversion function, with a specific
> >+# L<func> definition and LIBGCC2_UNITS_PER_WORD setting.  The DImode
> >+# functions are built with a wordsize of 4; the TImode functions are
> >+# built with the same labels, but a wordsize of 8.
> >+
> >+sifuncs = $(subst XX,si,$(swfloatfuncs))
> >+difuncs = $(subst XX,di,$(dwfloatfuncs))
> >+tifuncs = $(subst XX,ti,$(dwfloatfuncs))
> 
> Please extract [sdt]ifuncs out of the if, and use a single 
> ifeq/else/endif for both cases.

I've declined to make this change.  I think it's a bad idea.  The first
if statement can't move lower in the file, because it has to preceed
the filtering of lib2funcs.  The second if statement shouldn't move
higher in the file, because it's custom rules for building a part of
libgcc; it should stay with the other custom rules.  Makefiles are
hard enough to read without mixing up the pieces, IMO.

> >+$(patsubst %,%.vis,$(LIB1ASMFUNCS)): %.vis: %_s.o
> >+	$(gen-hide-list)
> 
> Replace .o with $(objext).

OK, though there's little point - nothing anywhere in all of gcc
sets it to anything else.

> >+libgcc.a libgcov.a libunwind.a libgcc_eh.a:
> >+	-rm -f $@
> >+
> >+	objects="$(objects)";	
> 
> No blank lines (i.e. no lines with the tab only).

It doesn't have a tab.  You don't need one; completely blank lines are
ignored by make.  Do you really want me to remove the blank lines?  I
think they make it more readable.

> >+all: $(HOST_EXTRA)
> >+install: $(HOST_EXTRA_INSTALL)
> >+
> 
> Just add the dependencies in the target fragment.

OK.

I'm combining this with changes for your other message.

1) Re extra_parts, I allowed tmake_file fragments to set it for easier
conversion from gcc, because that's what the GCC subdirectory did.  I'm
trying to keep conversion simple for now.  It's an easy mechanical
cleanup later.

2) The extra-parts variable is necessary because all isn't the only
thing to depend on it - the shared libraries do too and there's an
extra enable_shared conditional there.  It would be possible to rework
this, but I think it's much simpler the way it is.  There's an easy
way to disambiguate it though; I renamed extra-parts: to
libgcc-extra-parts:.

3) I moved INSTALL_PARTS up.

3 again) See above for my opinion on that blank line.

Here's the incremental diff.  I built and inspected libgcc on
x86_64-linux.

-- 
Daniel Jacobowitz
CodeSourcery

2006-12-29  Daniel Jacobowitz  <dan@codesourcery.com>

	* Makefile.in (libgcc.mvars): Don't record ASM_HIDDEN_OP.

2006-12-29  Daniel Jacobowitz  <dan@codesourcery.com>

	* Makefile.in (check, installcheck): Remove dependencies.
	(vis_hide, ASM_HIDDEN_OP): Move to configure script.
	(libgcc-extra-parts): Rename from extra-parts.  Add .PHONY.
	(INSTALL_PARTS): Move higher.
	(all, install): Remove HOST_EXTRA and HOST_EXTRA_INSTALL
	dependencies.
	(LIB1ASMFUNCS, crtbegin.o, crtend.o, crtbeginS.o, crtendS.o)
	(crtbeginT.o): Use $(objext).
	(gcc-extra-parts): Add .PHONY.
	* config/t-slibgcc-darwin (ASM_HIDDEN_OP): Deleted.
	(HOST_EXTRA, HOST_EXTRA_INSTALL): Replace with dependencies.
	* config.host: Add asm_hidden_op.
	* configure.ac: Check for visibility.  Substitute asm_hidden_op.
	* configure: Regenerated.

--- gcc/Makefile.in	(revision 120294)
+++ gcc/Makefile.in	(local)
@@ -1457,7 +1457,6 @@ libgcc.mvars: config.status Makefile $(L
 	echo SHLIB_MKMAP_OPTS = '$(SHLIB_MKMAP_OPTS)' >> tmp-libgcc.mvars
 	echo SHLIB_MAPFILES = '$(call srcdirify,$(SHLIB_MAPFILES))' >> tmp-libgcc.mvars
 	echo SHLIB_NM_FLAGS = '$(SHLIB_NM_FLAGS)' >> tmp-libgcc.mvars
-	echo ASM_HIDDEN_OP = '$(ASM_HIDDEN_OP)' >> tmp-libgcc.mvars
 	echo LIBGCC2_CFLAGS = '$(LIBGCC2_CFLAGS)' >> tmp-libgcc.mvars
 	echo CRTSTUFF_CFLAGS = '$(CRTSTUFF_CFLAGS)' >> tmp-libgcc.mvars
 	echo CRTSTUFF_T_CFLAGS = '$(CRTSTUFF_T_CFLAGS)' >> tmp-libgcc.mvars
--- libgcc/Makefile.in	(revision 120294)
+++ libgcc/Makefile.in	(local)
@@ -107,8 +107,8 @@ all-multi:
 	@: $(MAKE) ; exec $(MULTIDO) $(FLAGS_TO_PASS) multi-do DO=all
 
 .PHONY: check installcheck
-check: check-subdir
-installcheck: installcheck-subdir
+check:
+installcheck:
 
 .PHONY: all clean
 
@@ -233,30 +233,16 @@ ifeq ($(enable_shared),yes)
     install-libunwind = install-libunwind
   endif
 
-# Test -fvisibility=hidden.  We need both a -fvisibility=hidden on
+# For -fvisibility=hidden.  We need both a -fvisibility=hidden on
 # the command line, and a #define to prevent libgcc2.h etc from
-# overriding that with #pragmas.  The dance with @ is to prevent
-# echo from seeing anything it might take for an option.
-# echo turns the \$\$\$\$ into $$$$ and when make sees it it
-# becomes $$ and the shell substitutes the pid. Makes for a
-# slightly safer temp file.
-vis_hide := $(strip $(subst @,-,\
-    $(shell if echo 'void foo(void); void foo(void) {}' | \
-          $(gcc_compile_bare) -fvisibility=hidden -Werror \
-          -c -xc - -o vis_temp_file$$$$.o 2> /dev/null; \
-          then echo @fvisibility=hidden @DHIDE_EXPORTS; \
-          rm vis_temp_file$$$$.o 2> /dev/null; \
-          fi)))
+# overriding that with #pragmas.
+vis_hide = @vis_hide@
 
 ifneq (,$(vis_hide))
 
 # If we have -fvisibility=hidden, then we need to generate hide
-# lists for object files implemented in assembly.  The default
-# pseudo-op for this is ".hidden", but can be overridden with
-# ASM_HIDDEN_OP.
-ifeq ($(ASM_HIDDEN_OP),)
-ASM_HIDDEN_OP = .hidden
-endif
+# lists for object files implemented in assembly.
+ASM_HIDDEN_OP = @asm_hidden_op@
 
 define gen-hide-list
 $(NM) -pg $< | \
@@ -275,10 +261,12 @@ gen-hide-list = echo > \$@
 endif
 
 ifneq ($(EXTRA_PARTS),)
-  extra-parts = extra-parts
+  extra-parts = libgcc-extra-parts
+  INSTALL_PARTS = $(EXTRA_PARTS)
 else
 ifneq ($(GCC_EXTRA_PARTS),)
   extra-parts = gcc-extra-parts
+  INSTALL_PARTS = $(GCC_EXTRA_PARTS)
 endif
 endif
 
@@ -311,10 +299,6 @@ endif
 # unwinder info.
 LIB2_DIVMOD_FUNCS = _divdi3 _moddi3 _udivdi3 _umoddi3 _udiv_w_sdiv _udivmoddi4
 
-
-all: $(HOST_EXTRA)
-install: $(HOST_EXTRA_INSTALL)
-
 # Remove any objects from lib2funcs and LIB2_DIVMOD_FUNCS that are
 # defined as optimized assembly code in LIB1ASMFUNCS or as C code
 # in LIB2FUNCS_EXCLUDE.
@@ -329,7 +313,7 @@ lib1asmfuncs-o = $(patsubst %,%$(objext)
 $(lib1asmfuncs-o): %$(objext): $(gcc_srcdir)/config/$(LIB1ASMSRC) %.vis
 	$(gcc_compile) -DL$* -xassembler-with-cpp \
 	  -c $(gcc_srcdir)/config/$(LIB1ASMSRC) -include $*.vis
-$(patsubst %,%.vis,$(LIB1ASMFUNCS)): %.vis: %_s.o
+$(patsubst %,%.vis,$(LIB1ASMFUNCS)): %.vis: %_s$(objext)
 	$(gen-hide-list)
 libgcc-objects += $(lib1asmfuncs-o)
 
@@ -658,20 +642,20 @@ ALL_CRT_CFLAGS = $(CFLAGS) $(CRTSTUFF_CF
 crt_compile = $(CC) $(ALL_CRT_CFLAGS) -o $@ $(compile_deps)
 
 ifeq ($(CUSTOM_CRTSTUFF),)
-crtbegin.o: $(gcc_srcdir)/crtstuff.c
+crtbegin$(objext): $(gcc_srcdir)/crtstuff.c
 	$(crt_compile) $(CRTSTUFF_T_CFLAGS) \
 	  -c $(gcc_srcdir)/crtstuff.c -DCRT_BEGIN
 
-crtend.o: $(gcc_srcdir)/crtstuff.c
+crtend$(objext): $(gcc_srcdir)/crtstuff.c
 	$(crt_compile) $(CRTSTUFF_T_CFLAGS) \
 	  -c $(gcc_srcdir)/crtstuff.c -DCRT_END
 
 # These are versions of crtbegin and crtend for shared libraries.
-crtbeginS.o: $(gcc_srcdir)/crtstuff.c
+crtbeginS$(objext): $(gcc_srcdir)/crtstuff.c
 	$(crt_compile) $(CRTSTUFF_T_CFLAGS_S) \
 	  -c $(gcc_srcdir)/crtstuff.c -DCRT_BEGIN -DCRTSTUFFS_O
 
-crtendS.o: $(gcc_srcdir)/crtstuff.c
+crtendS$(objext): $(gcc_srcdir)/crtstuff.c
 	$(crt_compile) $(CRTSTUFF_T_CFLAGS_S) \
 	  -c $(gcc_srcdir)/crtstuff.c -DCRT_END -DCRTSTUFFS_O
 
@@ -682,7 +666,8 @@ crtbeginT.o: $(gcc_srcdir)/crtstuff.c
 endif
 
 # Build extra startfiles in the libgcc directory.
-extra-parts: $(EXTRA_PARTS)
+.PHONY: libgcc-extra-parts
+libgcc-extra-parts: $(EXTRA_PARTS)
 ifneq ($(GCC_EXTRA_PARTS),)
 ifneq ($(sort $(EXTRA_PARTS)),$(GCC_EXTRA_PARTS))
 	# If the gcc directory specifies which extra parts to
@@ -709,6 +694,7 @@ endif
 
 # Build extra startfiles in the gcc directory, for unconverted
 # targets.
+.PHONY: gcc-extra-parts
 gcc-extra-parts:
 	# Recursively invoke make in the GCC directory to build any
 	# startfiles (for now).  We must do this just once, passing
@@ -767,12 +753,6 @@ install-shared:
 		@shlib_base_name@,libgcc_s,$(subst \
 		@shlib_slibdir_qual@,$(MULTIOSSUBDIR),$(SHLIB_INSTALL))))
 
-ifneq ($(EXTRA_PARTS),)
-  INSTALL_PARTS = $(EXTRA_PARTS)
-else
-  INSTALL_PARTS = $(GCC_EXTRA_PARTS)
-endif
-
 install: $(install-shared) $(install-libunwind)
 	$(mkinstalldirs) $(DESTDIR)$(inst_libdir)
 
--- libgcc/config/t-slibgcc-darwin	(revision 120294)
+++ libgcc/config/t-slibgcc-darwin	(local)
@@ -26,9 +26,6 @@ SHLIB_MKMAP = $(gcc_srcdir)/mkmap-flat.a
 SHLIB_MKMAP_OPTS = -v leading_underscore=1
 SHLIB_MAPFILES += $(gcc_srcdir)/libgcc-std.ver
 
-# Must use a different directive for hidden visibility in assembly sources.
-ASM_HIDDEN_OP = .private_extern
-
 INSTALL_FILES=libgcc_s.10.4.dylib libgcc_s.10.5.dylib libgcc_s.1.dylib
 
 # For the toplevel multilib, build a fat archive including all the multilibs.
@@ -40,8 +37,8 @@ SHLIB_INSTALL = \
 	  $(DESTDIR)$(slibdir)/$(SHLIB_SONAME)
 
 ifeq ($(enable_shared),yes)
-HOST_EXTRA = $(INSTALL_FILES)
-HOST_EXTRA_INSTALL = install-darwin-libgcc-stubs
+all: $(INSTALL_FILES)
+install: install-darwin-libgcc-stubs
 endif
 
 # In order to support -mmacosx-version-min, you need to have multiple
--- libgcc/config.host	(revision 120294)
+++ libgcc/config.host	(local)
@@ -39,6 +39,10 @@
 # This file sets the following shell variables for use by the
 # autoconf-generated configure script:
 #
+#  asm_hidden_op	The assembler pseudo-op to use for hide
+#			lists for object files implemented in
+#			assembly (with -fvisibility=hidden for C).
+#			The default is ".hidden".
 #  cpu_type		The name of the cpu, if different from the first
 #			chunk of the canonical host name.
 #  extra_parts		List of extra object files that should be compiled
@@ -51,6 +55,7 @@
 #			makefile-fragments, if different from
 #			"$cpu_type/t-$cpu_type".
 
+asm_hidden_op=.hidden
 extra_parts=
 tmake_file=
 
@@ -137,6 +142,7 @@ esac
 # Common parts for widely ported systems.
 case ${host} in
 *-*-darwin*)
+  asm_hidden_op=.private_extern
   tmake_file="t-darwin ${cpu_type}/t-darwin t-slibgcc-darwin"
   ;;
 *-*-freebsd[12] | *-*-freebsd[12].* | *-*-freebsd*aout*)
--- libgcc/configure.ac	(revision 120294)
+++ libgcc/configure.ac	(local)
@@ -95,10 +95,30 @@ AC_CACHE_CHECK([whether decimal floating
 decimal_float=$libgcc_cv_dfp
 AC_SUBST(decimal_float)
 
-
 # Collect host-machine-specific information.
 . ${srcdir}/config.host
 
+# Check for visibility support.  This is after config.host so that
+# we can check for asm_hidden_op.
+AC_CACHE_CHECK([for __attribute__((visibility("hidden")))],
+    libgcc_cv_hidden_visibility_attribute, [
+	echo 'int __attribute__ ((visibility ("hidden"))) foo (void) { return 1; }' > conftest.c
+	libgcc_cv_hidden_visibility_attribute=no
+	if AC_TRY_COMMAND(${CC-cc} -Werror -S conftest.c -o conftest.s 1>&AS_MESSAGE_LOG_FD); then
+	    if grep "\\$asm_hidden_op.*foo" conftest.s >/dev/null; then
+		libgcc_cv_hidden_visibility_attribute=yes
+	    fi
+	fi
+	rm -f conftest.*
+    ])
+
+if test $libgcc_cv_hidden_visibility_attribute = yes; then
+    vis_hide='-fvisibility=hidden -DHIDE_EXPORTS'
+else
+    vis_hide=
+fi
+AC_SUBST(vis_hide)
+
 # Conditionalize the makefile for this target machine.
 tmake_file_=
 for f in ${tmake_file}
@@ -113,6 +133,7 @@ AC_SUBST(tmake_file)
 
 # Substitute configuration variables
 AC_SUBST(extra_parts)
+AC_SUBST(asm_hidden_op)
 
 # We need multilib support.
 AC_CONFIG_FILES([Makefile])


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]