diff mbox

[Ada,RFA] make sure that multilibs are built with correct s-oscons.ads

Message ID 87B813F3-5B28-4F36-A2BD-C0F919D5BE89@sandoe-acoustics.co.uk
State New
Headers show

Commit Message

Iain Sandoe Oct. 28, 2011, 6:01 p.m. UTC 
The sizes of items represented in s-oscons.ads can (and do) change  
with the multi-lib on targets that support  libada as a multi-lib.

At present, s-oscons.ads is only built once (in gcc/ada) and sym- 
linked to rts*/

This is causing a bunch of failures on i686-darwin9 where the m64  
multi-lib has generally larger structures than the m32 native.

On m64 targets with m32 multi-libs, this tends to be hidden by the  
fact that (generally) the m32 entities are smaller than their m64  
counterparts.  However, it's still wrong (at least insofar as wasting  
memory - if not on any more serious scale).

The attached patch moves the generation and use of the xoscons tool to  
the the library makefile (as suggested by Thomas) and adjusts the  
libada/Makefile dependency to point to this tool.  The remaining  
dependencies should (AFAICT) be handled by the gnatlib target - which  
thence depends on the required objects.

I don't have that many targets to test - and would very much welcome  
any more-Ada-build-system-aware  eyes cast over this.

This DTRT on i686-darwin9 (no unexpected fails at m64 when it's  
applied).

OK for trunk ?
what about 4.6 - given that this is a wrong code scenario?

cheers
Iain


ada:

	* gcc-interface/Makefile.in (stamp-gnatlib-$(RTSDIR)): Don't
	link s-oscons.ads.
	(OSCONS_CPP, OSCONS_EXTRACT): New.
	(./bldtools/oscons/xoscons): New Target.
	($(RTSDIR)/s-oscons.ads): New Target.
	(gnatlib): Depend on  $(RTSDIR)/s-oscons.ads.
	* gcc-interface/Make-lang.in (ada/s-oscons.ads) Remove as dependency.
	* Make-generated.in: Remove machinery to generate xoscons and
	ada/s-oscons.ads.

libada:

	Makefile.in: Change dependency on oscons to depend on the generator
	tool.

Comments

Iain Sandoe Nov. 7, 2011, 11:23 a.m. UTC | #1 
On 28 Oct 2011, at 19:01, Iain Sandoe wrote:

> The sizes of items represented in s-oscons.ads can (and do) change  
> with the multi-lib on targets that support  libada as a multi-lib.
>
> At present, s-oscons.ads is only built once (in gcc/ada) and sym- 
> linked to rts*/
>
> This is causing a bunch of failures on i686-darwin9 where the m64  
> multi-lib has generally larger structures than the m32 native.
>
> On m64 targets with m32 multi-libs, this tends to be hidden by the  
> fact that (generally) the m32 entities are smaller than their m64  
> counterparts.  However, it's still wrong (at least insofar as  
> wasting memory - if not on any more serious scale).
>
> The attached patch moves the generation and use of the xoscons tool  
> to the the library makefile (as suggested by Thomas) and adjusts the  
> libada/Makefile dependency to point to this tool.  The remaining  
> dependencies should (AFAICT) be handled by the gnatlib target -  
> which thence depends on the required objects.
>
> I don't have that many targets to test - and would very much welcome  
> any more-Ada-build-system-aware  eyes cast over this.
>
> This DTRT on i686-darwin9 (no unexpected fails at m64 when it's  
> applied).

http://gcc.gnu.org/ml/gcc-patches/2011-10/msg02695.html
Thomas Quinot Nov. 9, 2011, 9:49 a.m. UTC | #2 
* Iain Sandoe, 2011-11-07 :

> Subject: PING 1 [Patch Ada RFA] make sure that multilibs are built with
>  correct s-oscons.ads

Patch looks fine to me.
Arnaud Charlet Nov. 9, 2011, 9:54 a.m. UTC | #3 
> * Iain Sandoe, 2011-11-07 :
> 
> > Subject: PING 1 [Patch Ada RFA] make sure that multilibs are built with
> >  correct s-oscons.ads
> 
> Patch looks fine to me.

It's an official 'OK' then.
Iain Sandoe Nov. 12, 2011, 3:17 p.m. UTC | #4 
On 9 Nov 2011, at 09:54, Arnaud Charlet wrote:

>> * Iain Sandoe, 2011-11-07 :
>>
>>> Subject: PING 1 [Patch Ada RFA] make sure that multilibs are built  
>>> with
>>> correct s-oscons.ads
>>
>> Patch looks fine to me.
>
> It's an official 'OK' then.

Thanks, done (r181319).

May I repeat my question re. 4.6?
Since this is a wrong-code situation, I would have thought it eligible  
for a back-port?

thanks
Iain
Arnaud Charlet Nov. 12, 2011, 4:01 p.m. UTC | #5 
> May I repeat my question re. 4.6?
> Since this is a wrong-code situation, I would have thought it eligible for
> a back-port?

Well, it's not a wrong-code as in "wrong code generated by the back-end",
and it's not a regression.

In any case, backporting to 4.6 is fine with me.

Arno
diff mbox

Patch

Index: gcc/ada/gcc-interface/Makefile.in
===================================================================
--- gcc/ada/gcc-interface/Makefile.in	(revision 180619)
+++ gcc/ada/gcc-interface/Makefile.in	(working copy)
@@ -2498,21 +2498,50 @@  install-gnatlib: ../stamp-gnatlib-$(RTSDIR)
 	                $(RTSDIR)/$(word 1,$(subst <, ,$(PAIR)));)
 # Copy tsystem.h
 	$(CP) $(srcdir)/tsystem.h $(RTSDIR)
-# Copy generated target dependent sources
-	$(RM) $(RTSDIR)/s-oscons.ads
-	(cd $(RTSDIR); $(LN_S) ../s-oscons.ads s-oscons.ads)
 	$(RM) ../stamp-gnatlib-$(RTSDIR)
 	touch ../stamp-gnatlib1-$(RTSDIR)
 
 # GNULLI End #############################################################
 
+ifeq ($(strip $(filter-out alpha64 ia64 dec hp vms% openvms% alphavms%,$(subst -, ,$(host)))),)
+OSCONS_CPP=../../$(DECC) -E /comment=as_is -DNATIVE \
+             -DTARGET='""$(target)""' $(fsrcpfx)ada/s-oscons-tmplt.c
+
+OSCONS_EXTRACT=../../$(DECC) -DNATIVE \
+                 -DTARGET='""$(target)""' $(fsrcpfx)ada/s-oscons-tmplt.c ; \
+  ld -o s-oscons-tmplt.exe s-oscons-tmplt.obj; \
+  ./s-oscons-tmplt.exe > s-oscons-tmplt.s
+
+else
+# GCC_FOR_TARGET has paths relative to the gcc directory, so we need to adjust
+# for running it from $(RTSDIR)
+OSCONS_CC=`echo "$(GCC_FOR_TARGET)" \
+  | sed -e 's^\./xgcc^../../xgcc^' -e 's^-B./^-B../../^'`
+OSCONS_CPP=$(OSCONS_CC) $(GNATLIBCFLAGS) -E -C \
+  -DTARGET=\"$(target)\" $(fsrcpfx)ada/s-oscons-tmplt.c > s-oscons-tmplt.i
+OSCONS_EXTRACT=$(OSCONS_CC) -S s-oscons-tmplt.i
+endif
+
+./bldtools/oscons/xoscons: xoscons.adb xutil.ads xutil.adb
+	-$(MKDIR) ./bldtools/oscons
+	$(RM) $(addprefix ./bldtools/oscons/,$(notdir $^))
+	$(CP) $^ ./bldtools/oscons
+	(cd ./bldtools/oscons ; gnatmake -q xoscons)
+
+$(RTSDIR)/s-oscons.ads: ../stamp-gnatlib1-$(RTSDIR) s-oscons-tmplt.c gsocket.h ./bldtools/oscons/xoscons
+	$(RM) $(RTSDIR)/s-oscons-tmplt.i $(RTSDIR)/s-oscons-tmplt.s
+	(cd $(RTSDIR) ; \
+	    $(OSCONS_CPP) ; \
+	    $(OSCONS_EXTRACT) ; \
+	    ../bldtools/oscons/xoscons)
+
 # Don't use semicolon separated shell commands that involve list expansions.
 # The semicolon triggers a call to DCL on VMS and DCL can't handle command
 # line lengths in excess of 256 characters.
 # Example: cd $(RTSDIR); ar rc libfoo.a $(LONG_LIST_OF_OBJS)
 # is guaranteed to overflow the buffer.
 
-gnatlib: ../stamp-gnatlib1-$(RTSDIR) ../stamp-gnatlib2-$(RTSDIR)
+gnatlib: ../stamp-gnatlib1-$(RTSDIR) ../stamp-gnatlib2-$(RTSDIR) $(RTSDIR)/s-oscons.ads
 	$(MAKE) -C $(RTSDIR) \
 		CC="`echo \"$(GCC_FOR_TARGET)\" \
 		| sed -e 's,\./xgcc,../../xgcc,' -e 's,-B\./,-B../../,'`" \
Index: gcc/ada/gcc-interface/Make-lang.in
===================================================================
--- gcc/ada/gcc-interface/Make-lang.in	(revision 180619)
+++ gcc/ada/gcc-interface/Make-lang.in	(working copy)
@@ -568,7 +568,7 @@  canadian-gnattools: force
 	$(MAKE) -C ada $(ADA_TOOLS_FLAGS_TO_PASS) gnattools1-re
 	$(MAKE) -C ada $(ADA_TOOLS_FLAGS_TO_PASS) gnattools2
 
-gnatlib gnatlib-sjlj gnatlib-zcx gnatlib-shared: ada/s-oscons.ads force
+gnatlib gnatlib-sjlj gnatlib-zcx gnatlib-shared: force
 	$(MAKE) -C ada $(COMMON_FLAGS_TO_PASS)  \
 	   GNATLIBFLAGS="$(GNATLIBFLAGS)" \
 	   GNATLIBCFLAGS="$(GNATLIBCFLAGS)" \
Index: gcc/ada/Make-generated.in
===================================================================
--- gcc/ada/Make-generated.in	(revision 180619)
+++ gcc/ada/Make-generated.in	(working copy)
@@ -64,37 +64,6 @@  $(ADA_GEN_SUBDIR)/nmake.ads :  $(ADA_GEN_SUBDIR)/s
 	$(CP) $^ $(ADA_GEN_SUBDIR)/bldtools/nmake_s
 	(cd $(ADA_GEN_SUBDIR)/bldtools/nmake_s; gnatmake -q xnmake ; ./xnmake -s ../../nmake.ads )
 
-ifeq ($(strip $(filter-out alpha64 ia64 dec hp vms% openvms% alphavms%,$(subst -, ,$(host)))),)
-OSCONS_CPP=../../../$(DECC) -E /comment=as_is -DNATIVE \
-             -DTARGET='""$(target)""' s-oscons-tmplt.c
-
-OSCONS_EXTRACT=../../../$(DECC) -DNATIVE \
-                 -DTARGET='""$(target)""' s-oscons-tmplt.c ; \
-  ld -o s-oscons-tmplt.exe s-oscons-tmplt.obj; \
-  ./s-oscons-tmplt.exe > s-oscons-tmplt.s
-
-else
-# GCC_FOR_TARGET has paths relative to the gcc directory, so we need to adjust
-# for running it from $(ADA_GEN_SUBDIR)/bldtools/oscons
-OSCONS_CC=`echo "$(GCC_FOR_TARGET)" \
-  | sed -e 's^\./xgcc^../../../xgcc^' -e 's^-B./^-B../../../^'`
-OSCONS_CPP=$(OSCONS_CC) $(GNATLIBCFLAGS) -E -C \
-  -DTARGET=\"$(target)\" s-oscons-tmplt.c > s-oscons-tmplt.i
-OSCONS_EXTRACT=$(OSCONS_CC) -S s-oscons-tmplt.i
-endif
-
-$(ADA_GEN_SUBDIR)/s-oscons.ads : $(ADA_GEN_SUBDIR)/s-oscons-tmplt.c $(ADA_GEN_SUBDIR)/gsocket.h $(ADA_GEN_SUBDIR)/xoscons.adb $(ADA_GEN_SUBDIR)/xutil.ads $(ADA_GEN_SUBDIR)/xutil.adb
-	-$(MKDIR) $(ADA_GEN_SUBDIR)/bldtools/oscons
-	$(RM) $(addprefix $(ADA_GEN_SUBDIR)/bldtools/oscons/,$(notdir $^))
-	$(CP) $^ $(ADA_GEN_SUBDIR)/bldtools/oscons
-	(cd $(ADA_GEN_SUBDIR)/bldtools/oscons ; gnatmake -q xoscons ; \
-		$(RM) s-oscons-tmplt.i s-oscons-tmplt.s ; \
-		$(OSCONS_CPP) ; \
-		$(OSCONS_EXTRACT) ; \
-		./xoscons ; \
-		$(RM) ../../s-oscons.ads ; \
-		$(CP) s-oscons.ads s-oscons.h ../../)
-
 $(ADA_GEN_SUBDIR)/sdefault.adb: $(ADA_GEN_SUBDIR)/stamp-sdefault ; @true
 $(ADA_GEN_SUBDIR)/stamp-sdefault : $(srcdir)/version.c Makefile
 	$(ECHO) "pragma Style_Checks (Off);" >tmp-sdefault.adb
Index: libada/Makefile.in
===================================================================
--- libada/Makefile.in	(revision 180619)
+++ libada/Makefile.in	(working copy)
@@ -64,6 +64,7 @@  target_noncanonical:=@target_noncanonical@
 version := $(shell cat $(srcdir)/../gcc/BASE-VER)
 libsubdir := $(libdir)/gcc/$(target_noncanonical)/$(version)$(MULTISUBDIR)
 ADA_RTS_DIR=$(GCC_DIR)/ada/rts$(subst /,_,$(MULTISUBDIR))
+ADA_RTS_SUBDIR=./rts$(subst /,_,$(MULTISUBDIR))
 
 # exeext should not be used because it's the *host* exeext.  We're building
 # a *target* library, aren't we?!?  Likewise for CC.  Still, provide bogus
@@ -90,10 +91,10 @@  LIBADA_FLAGS_TO_PASS = \
         "CFLAGS=$(CFLAGS)"
 
 # Rules to build gnatlib.
-.PHONY: gnatlib gnatlib-plain gnatlib-sjlj gnatlib-zcx gnatlib-shared oscons
+.PHONY: gnatlib gnatlib-plain gnatlib-sjlj gnatlib-zcx gnatlib-shared osconstool
 gnatlib: @default_gnatlib_target@
 
-gnatlib-plain: oscons $(GCC_DIR)/ada/Makefile
+gnatlib-plain: osconstool $(GCC_DIR)/ada/Makefile
 	test -f stamp-libada || \
 	$(MAKE) -C $(GCC_DIR)/ada $(LIBADA_FLAGS_TO_PASS) gnatlib \
 	&& touch stamp-libada
@@ -102,7 +103,7 @@  gnatlib: @default_gnatlib_target@
 	$(LN_S) $(ADA_RTS_DIR) adainclude
 	$(LN_S) $(ADA_RTS_DIR) adalib
 
-gnatlib-sjlj gnatlib-zcx gnatlib-shared: oscons $(GCC_DIR)/ada/Makefile
+gnatlib-sjlj gnatlib-zcx gnatlib-shared: osconstool $(GCC_DIR)/ada/Makefile
 	test -f stamp-libada || \
 	$(MAKE) -C $(GCC_DIR)/ada $(LIBADA_FLAGS_TO_PASS) $@ \
 	&& touch stamp-libada
@@ -111,8 +112,8 @@  gnatlib: @default_gnatlib_target@
 	$(LN_S) $(ADA_RTS_DIR) adainclude
 	$(LN_S) $(ADA_RTS_DIR) adalib
 
-oscons:
-	$(MAKE) -C $(GCC_DIR) $(LIBADA_FLAGS_TO_PASS) ada/s-oscons.ads
+osconstool:
+	$(MAKE) -C $(GCC_DIR)/ada $(LIBADA_FLAGS_TO_PASS) ./bldtools/oscons/xoscons
 
 install-gnatlib: $(GCC_DIR)/ada/Makefile
 	$(MAKE) -C $(GCC_DIR)/ada $(LIBADA_FLAGS_TO_PASS) install-gnatlib