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]

Re: MachTen Egcs Patches


Jeffery,

	Ill get the proper copyright and any other disclaimers that you
need done asap.  The changes were generated from a one-day port of our GNU
efforts to EGCS.  Ill make a pass over the submission with your message in
mind.  Some of the problems are simply ignornace of how you want things
done.  We will correct this.  There are three principal reasons for the
changes:
	1. remove the builtin alloca functions and force them to use our
external versions
	2. provide for backward compatibilty in some strict directory
assignments that
	   were setup for our older M68K and early PowerPC versions of the
system and at
	   the same time evolve to standard directory assignments.
	3. Handle a set of pragma options that - again inhereited - are
littered throughout
	   mass quantities of Apple and our include files.  These options
are important in
	   a transition from older and very poor for power pc aligned data
structures to more
	  modern "naturally" aligned data structures.

As far as the coding style is concerned.  Thats fine - well take a look at
the gnu documents and make the code conform.

Thanks for taking a look at this so quickly.

Regards,
Steve Holmgren

At 10:26 PM -0800 7/6/98, Jeffrey A Law wrote:
>  In message <199807021827.LAA09959@mail.tenon.com>you write:
>  > Hello,
>  >
>  > We have wanted to get our gcc patches into the standard release for a
>  > long time.  We were hoping that with the forthcoming freeze of egcs 1.1
>  > that we might have chance to be included.
>First note, I do not see a copyright assignment on file for you; you
>will need to fill one out and probably an employer disclaimer before
>we can do anything with these patches.  See:
>
>http://www.cygnus.com/egcs/contribute.html.
>
>Also note, this applies to any other folks that have contributed to
>this code.
>
>This is a rough first pass over the code.  I'm sure as we iterate on
>the changes we'll notice more things that need to be addressed.
>
>
>  > environment.  Since this system must reside equally well in real memory
>  > and virtual memory environments we have had to make modifications to the
>  > alloca, __builtin_alloca and __builtin_va_alist parts of gcc.
>Can you please explain this?  Those functions really don't care about
>whether your're running with or without virtual memory.  The va_alist
>change in particular sounds totally bogus.
>
>
>  > gcc/config/m68k/machten68k.h
>  >
>  > /* Definitions of target machine for GNU compiler.  Macintosh 68000
>version
>  > .
>  >    Copyright (C) 1987, 1988 Free Software Foundation, Inc.
>You'll need to add 1998 to the copyright list -- similarly for the
>other files you added.
>
>
>  > /* Use MachTen include file search path.  */
>  > /* Even for a cross compiler with MachTen as target! */
>  > #define INCLUDE_DEFAULTS				\
>  >   {							\
>  >     { GPLUSPLUS_INCLUDE_DIR, 1, 1},			\
>  >     { MACHINE_INCLUDE_DIR, 0, 1},			\
>  >     { LOCAL_INCLUDE_DIR, 0, 1},				\
>  >     { STANDARD_INCLUDE_DIR, 0, 1},			\
>  >     { 0, 0, 0}						\
>  >   }
>This sounds wrong.  You shouldn't be modifying the INCLUDE_DEFAULTS
>when building a cross compiler on MachTen, only when building a
>MachTen target compiler.
>
>  > /* Define __HAVE_68881 in preprocessor only if -m68881 is specified.
>  >    This will control the use of inline 68881 insns in certain macros.
>  >    Also inform the program which CPU this is for.  */
>  >
>  > #ifndef CPP_SPEC
>  > #define CPP_SPEC "\
>  > %{!m68040-lc:%{!mc68040-lc:%{m680*: -Dmc680%*}%{mc680*: -Dmc680%*}}}\
>  > %{m68040-lc: -Dmc68LC040}%{mc68040-lc: -Dmc68LC040}\
>  > %{!m680*:%{!mc680*: -Dmc68020}}\
>  > %{m68881:%{!msoft-float: -D__HAVE_68881__}}"
>  > #endif
>This may need updating -- some of this stuff has changed relatively
>recently.
>
>
>  > #ifndef ASM_SPEC
>  > #define ASM_SPEC "\
>  > %{!m68040-lc:%{!mc68040-lc:%{m680*: -mc680%*}%{mc680*: -mc680%*}}}\
>  > %{m68040-lc: -mc68040}%{mc68040-lc: -mc68040}\
>  > %{!m680*:%{!mc680*: -mc68020}} %{mno-68*} %{pic}"
>  > #endif
>Similarly.
>
>  > /* Names to predefine in the preprocessor for this target machine.  */
>  > /* __MACHTEN__ is needed inside cpp for direct calls to it; gcc will
>  >    compose it from -DMACHTEN; TENON is a silent holdover.  */
>  > /* TENON is *deprecated*, use __MACHTEN__ instead */
>  > #ifndef CPP_PREDEFINES
>  > #define CPP_PREDEFINES "-Dunix -D__MACHTEN__ -D__MACHTEN_68K__ -DTENON\
>  >  -Asystem(unix) -Asystem(bsd) -Asystem(machten)\
>  >  -Acpu(m68000) -Acpu(mc68000) -Acpu(m68k)\
>  >  -Amachine(m68000) -Amachine(mc68000) -Amachine(m68k) -Amachine(mac)"
>  > #endif
>If at all possible, we should avoid -Dunix and -DTENON since they
>pollute the ansi namespace.
>
>  > #ifndef	CC1_SPEC
>  > #define CC1_SPEC "-fno-builtin-alloca -fno-defer-pop"
>  > #endif
>Why are you turning off alloca & defer pop?  That seems wrong.
>
>  > #ifndef	CC1PLUS_SPEC
>  > #define CC1PLUS_SPEC "-fno-builtin-alloca -fno-defer-pop
>-felide-constructo rs"
>  > #endif
>Similarly.
>
>  >
>  > #ifndef LINK_SPEC
>  > #define LINK_SPEC "-x"
>  > #endif
>What does this do?
>
>  > # Strip installed executables.. please!
>  > INSTALL_PROGRAM = $(INSTALL) -s
>This is wrong.  It should be driven by users, not by compiler writers.
>
>  > # Common prefix for installation directories.
>  > prefix = /usr
>Ugh.  This is wrong too.  prefix is a user option and if the user
>does not specify --prefix, then it should default to /usr/local.
>
>  > # One might argue that GCC_O_CPPFLAGS and CCCP_O_CPPFLAGS belong in
>  > # x-machten68k, however, their presence here is convenient for making both
>  > # a native m68k compiler and a cross compiler running on PowerMacs.
>  > GCC_O_CPPFLAGS=\
>  > 	-DSTANDARD_STARTFILE_PREFIX=\"/usr/mac68k/lib/\" \
>  > 	-DSTANDARD_EXEC_PREFIX=\"/usr/mac68k/bin/\" \
>  > 	-DDEFAULT_TARGET_VERSION=\"$(version)\" \
>  > 	-DDEFAULT_TARGET_MACHINE=\"$(target)\" \
>  > 	-DTOOLDIR_BASE_PREFIX=\"/usr/local/\"
>  >
>  > CCCP_O_CPPFLAGS=\
>  > 	-DGCC_INCLUDE_DIR=\"\" \
>  > 	-DGPLUSPLUS_INCLUDE_DIR=\"/usr/include/g++\" \
>  > 	-DLOCAL_INCLUDE_DIR=\"/usr/local/include\" \
>  > 	-DCROSS_INCLUDE_DIR=\"\" \
>  > 	-DTOOL_INCLUDE_DIR=\"\" \
>  > 	-DMACHINE_INCLUDE_DIR=\"/usr/mac68k/include\"
>See the comment above this code.  this is wrong.
>
>  >
>  > # Even tho gcc is the compiler on MachTen, refer to it by the
>historical na
>  > me
>  > # so that USE_ALLOCA and USE_HOST_ALLOCA are properly set (as alloca.o)!
>  > CC = cc
>Remove this.  It is not needed or wanted anymore.
>
>  > ALL= native xgcc specs lang.start.encap lang.rest.encap
>  > LANGUAGES = c c++
>  > STMP_FIXPROTO=
>Kill ALL & LANGUAGES.  These should not be set in an x-* file.
>
>  > X_CFLAGS= -fno-builtin-alloca -fno-defer-pop
>  >
>  > # Extra compile/load switches specific to MachTen
>  > ALLOCA= alloca.o
>  > ALLOCA_FLAGS= -DSTACK_DIRECTION=-1
>  >
>  > XGCC_LDFLAGS= -stack 32768 ${LDFLAGS}
>  > CCCP_LDFLAGS= -stack 49152 ${LDFLAGS}
>  > CC1_LDFLAGS= -stack 49152 ${LDFLAGS}
>  > CC1OBJ_LDFLAGS= -stack 49152 ${LDFLAGS}
>  > CC1PLUS_LDFLAGS= -stack 49152 ${LDFLAGS}
>  > F771_LDFLAGS= -stack 49152 ${LDFLAGS}
>Is there a better way to do this?
>
>  > /* Note presence of selected function in host library */
>  > #define HAVE_PUTENV		/* putenv() */
>  > #define HAVE_STRERROR		/* strerror() */
>  > #define HAVE_VPRINTF		/* vprintf() */
>Most of this stuff should not be necessary anymore due to autoconf.
>
>
>I worry about the alloca code you added to the rs6000 directory.  I think
>you need to explain the need to override alloca in more detail.
>
>
>  > /* No structure field wants to be aligned rounder than this.  */
>  > #undef BIGGEST_FIELD_ALIGNMENT
>  > //
>  > // wanted to make this the default.  simply to many things started
>  > // breaking - therefore had to return to default
>  > #define   BIGGEST_FIELD_ALIGNMENT 16
>Do not put C++ style comments in gcc code.  Use C comments instead.
>
>
>Comments should be complete sentences ending with a period followed by
>two spaces.  Many of them need updating to this convention.
>
>
>  > USER_H =
>  > INSTALL_ASSERT_H =
>This is wrong.  It will cause problems with C++.  You should find a way
>to work with the header files gcc installs.
>
>  > CFLAGS = -malignppc -O3 -pipe -fstrength-reduce -fomit-frame-pointer
>-finli
>  > ne-functions -finhibit-size-directive -DREAL_MEMORY_TARGET
>  > CXXFLAGS = -malignppc -O3 -pipe -fstrength-reduce -fomit-frame-pointer
>-fin
>  > line-functions -finhibit-size-directive -DREAL_MEMORY_TARGET
>Remove this -- you shouldn't be passing most/all of these flags when
>building the compiler.
>
>  > # Common prefix for installation directories.
>  > prefix = /usr
>See earlier comments abbout prefix.
>
>  > # One might argue that GCC_O_CPPFLAGS and CCCP_O_CPPFLAGS belong in
>  > # x-machtenppc, however, their presence here is convenient for making both
>  > # a native PPC compiler and a cross compiler running on m68k-Macs.
>  >
>  >
>  > GCC_O_CPPFLAGS =\
>  > 	-DSTANDARD_STARTFILE_PREFIX=\"$(prefix)/lib/gcc-lib/\"\
>  > 	-DSTANDARD_EXEC_PREFIX=\"$(prefix)/lib/gcc-lib/\"\
>  > 	-DDEFAULT_TARGET_VERSION=\"$(version)\"\
>  > 	-DDEFAULT_TARGET_MACHINE=\"$(target)\"\
>  > 	-DTOOLDIR_BASE_PREFIX=\"/usr/lib/gcc-lib/\"
>  >
>  > CCCP_O_CPPFLAGS =\
>  > 	-DGCC_INCLUDE_DIR=\"$(libsubdir)/include\"\
>  > 	-DGPLUSPLUS_INCLUDE_DIR=\"/usr/include/g++\"\
>  > 	-DLOCAL_INCLUDE_DIR=\"/usr/local/include\"\
>  > 	-DCROSS_INCLUDE_DIR=\"$(libsubdir)/sys-include\"\
>  > 	-DTOOL_INCLUDE_DIR=\"$(tooldir)/include\"\
>  > 	-DMACHINE_INCLUDE_DIR=\"/usr/macppc/include\"
>Same comment as for the m68k files.
>
>
>  > # Directory in which the compiler finds g++ includes.
>  > gxx_include_dir = /usr/include/g++
>You shouldn't be setting this variable either.
>
>  > ALL= native xgcc specs lang.start.encap lang.rest.encap
>  > LANGUAGES = c
>  > STMP_FIXPROTO=
>Setting ALL and LANGUAGES is wrong.
>
>
>  > /* It would be better to get this (actually BSD4_4) from <sys/param.h>,
>  >    however, this is all we really need.  */
>  > #define bsd4_4	1
>This should not be necessary anymore.
>
>  > /* Note presence of selected function in host library */
>  > //#define HAVE_PUTENV		/* putenv() */
>  > //#define HAVE_STRERROR		/* strerror() */
>  > //#define HAVE_VPRINTF		/* vprintf() */
>Delete all this stuff.
>
>  > + #ifdef REAL_MEMORY_TARGET
>All these ifdefs need to be rethought or explained better -- why can't
>you use the builtin alloca stuff?
>
>  > + #ifdef __MACHTEN__
>  > +       else if( type == ps_align ) {
>  > +           if( state == ps_right )
>  > +             maximum_field_alignment = align * 8;
>  > +           else
>  > +             warning( "malformed `#pragma align`" );
>  > +       } else if( type == ps_options ) {
>  > +           /* a great deal of the source already supports this form
>of pra
>  > gma */
>  > +           /* simply cheaper to modify the compiler than to fix
>everything
>  >  */
>  > +           if( state == ps_value ) {
>  > +                 if( strcmp( value,"native" ) == 0  ||
>  > +                     strcmp( value,"power" ) == 0 )
>  > +                         maximum_field_alignment = 32;
>  > +                 else
>  > +                 if( strcmp( value,"mac68k" ) == 0  ||
>  > +                     strcmp( value,"m68k" ) == 0 )
>  > +                         maximum_field_alignment = 16;
>  > +                 else
>  > +                 if( strcmp( value,"reset" ) == 0 )
>  > +                         maximum_field_alignment = TARGET_ALIGN_M68K
>? 16
>  > : 32;
>  > +                 else
>  > +                         maximum_field_alignment = 32;
>  > +           } else
>  > +             warning( "malformed `#pragma options align =
><reset|mac68k|na
>  > tive|power>`" );
>  > +       }
>  > + #endif
>ifdefs on __MACHTEN__ are not appropriate in the generic parts of the
>compiler.  You need to find a better (more generic) way to solve these
>problems.
>
>Also note you have many formatting problems.  I would recommend you
>read the GNU coding standards and update your code to conform.
>
>
>  > +                # On MachTen, the headers are already ok.
>  > +                fixincludes=Makefile.in
>Don't be so sure :-)  We recommend running fixincludes even if you
>think your headers are OK.



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