This is the mail archive of the gcc@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: MPE Port


  In message <4.1.19990906102304.00c72c80@garfield.dis.com>you write:
  > Sat Sep  4 18:00:00 PDT 1999 Mark Klein (mklein@dis.com)
  > 
  >         * pa/pa-mpeix.h: New file for MPE port.
Several comments :-)


  > *** egcs/gcc/config/pa/pa-mpeix.h	Wed Dec 31 16:00:00 1969
  > --- egcs-ss/gcc/config/pa/pa-mpeix.h	Mon Aug 30 21:52:06 1999
  > ***************
  > *** 0 ****
  > --- 1,240 ----
  > + /* Definitions of target machine for GNU compiler, for MPEiX.
  > +    Contributed by Mark Klein <mklein@dis.com>
  > +    Copyright (C) 1998 Free Software Foundation, Inc.
Make sure to add 1999 to the copyright header.


  > + You should have received a copy of the GNU General Public License
  > + along with GNU CC; see the file COPYING.  If not, write to
  > + the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.  */
  > + 
This appears to be the old FSF address.  You need to update the files to have
the new FSF address:

You should have received a copy of the GNU General Public License
along with GNU CC; see the file COPYING.  If not, write to
the Free Software Foundation, 59 Temple Place - Suite 330,
Boston, MA 02111-1307, USA.  */


  > + #undef TARGET_DEFAULT
  > + #define TARGET_DEFAULT (MASK_GAS | MASK_JUMP_IN_DELAY | MASK_USE_MILLICOD
  > E)
  > + 
  > + /* MPE (as of MPE/iX 6.5) still uses older millicode than HP-UX. */
  > + #undef  TARGET_NEW_MILLICODE
  > + #define TARGET_NEW_MILLICODE 0
In general try to avoid names like NEW/OLD, since there will always be
something newer.  For example, PA64 has a yet newer millicode scheme.  Do we
call it NEW_NEW_MILLICODE :-)? 

Is there some better way to describe it?  ie, what are the characteristics of
this scheme that make it different from the scheme found with PA32 SOM HPUX
conventions?

Or better yet, is there a name for it?

  > + #undef CPP_PREDEFINES
  > + #define CPP_PREDEFINES "-Dhppa -Dhp3000s900 -D__hp3000s900 -Dhp3k9
  > -Dmpeix -Dhp3000 -DREVARGV -Asystem(mpeix) -Acpu(hppa) -Amachine(hppa)"
You should move the -D definitions into a CPP_SPEC string to avoid ANSI
namespace pollution.


  > + /* Ugly hack, but I don't see any other way.
  > +  * Gotta enable the <Control Y> traps, otherwise SIG_INT won't work.
  > +  */
:-)   As I mentioned before, I do not think this belongs in gcc.  A cleaner
way to do this is to provide this functionality in a library routine that
users can link with.  To ensure it runs before main you can declare the
magic function as a constructor (using the constructor attribute).

On a style note we write multi-line comments like:

  /* blah blah blah
     boo hoo
     This is a sentence.  */

Nearly every one of your comments used the wrong style.

  > + 
  > + /* HPUX needs gcc_plt_call for gdb. Since gdb is not yet running on
  > +  * MPE and I think that gcc_plt_call will need an MPE variant, use
  > +  * the following in lieu of the DO_GLOBAL_DTORS_BODY declared in pa.h
  > +  */
What you'll need to do to find out is determine whether or not your runtime
linker allows a return location from a call to be a stack address.

If it does, then you will not need a stub.  If it does not, then you will need
a stub.  The stub is most likely going to be identical to __gcc_plt_call since
it's only purpose is to sit in the text segment to shut up the overly pedantic
runtime linker.


  > + #undef  ASM_DECLARE_FUNCTION_NAME
  > + #define ASM_DECLARE_FUNCTION_NAME(FILE, NAME, DECL) \
  > +     do { tree fntype = TREE_TYPE (TREE_TYPE (DECL));		
  > 	\
  > + 	 tree tree_type = TREE_TYPE (DECL);				\
  > + 	 tree parm;							\
  > + 	 int i;								\
  > + 	 if (TREE_PUBLIC (DECL) || TARGET_GAS)				\
  > + 	   { extern int current_function_varargs;			\
  > + 	     if (TREE_PUBLIC (DECL))					\
  > + 	       {							\
  > + 		 fputs ("\t.EXPORT ", FILE);				\
  > + 		 assemble_name (FILE, NAME);				\
  > + 		 fputs (",ENTRY,PRIV_LEV=3", FILE);			\
  > + 	       }							\
  > + 	     else							\
  > + 	       {							\
  > + 		 fputs ("\t.PARAM ", FILE);				\
  > + 		 assemble_name (FILE, NAME);				\
  > + 		 fputs (",PRIV_LEV=3", FILE);				\
If the only difference is that you need a PRIV_LEV=3 on a static function, I
see no reason we can't promote that into som.h.  I believe that would
eliminate the need for a separate ASM_DECLARE_FUNCTION_NAME, right?

Even if you need to be different we can easily conditionalize the code in
som.h a little, which would avoid a lot of code duplication.

  > + 	       }							\
  > + 	     if (TARGET_PORTABLE_RUNTIME)				\
  > + 	       {							\
  > + 		 fputs (",ARGW0=NO,ARGW1=NO,ARGW2=NO,ARGW3=NO,", FILE);	\
  > + 		 fputs ("RTNVAL=NO\n", FILE);				\
  > + 		 break;							\
  > + 	       }							\
If you end up with a separate ASM_DECLARE_FUNCTION_NAME you certainly do
not need TARGET_PORTABLE_RUNTIME support ;-)


  > + /* This is another ugly hack. Compilation units must be named
  > +  * if we ever want to extract them by name. Since making libgcc
  > +  * does this, I added a new pseudo command .compiler that is used
  > +  * to communicate the names to gas.
  > +  */
  > + #undef ASM_IDENTIFY_GCC
  > + #define ASM_IDENTIFY_GCC(FILE) 					
  > 	\
  > + 	do {								\
  > + 		fputs ("; gcc_compiled.:\n", FILE);			\
  > + 		fprintf (FILE, "\t.compiler \"%s %s %s\"\n",		\
  > + 			 main_input_filename,				\
  > + 			 "GNU_C_Compiler",				\
  > + 			 version_string);				\
  > + 	} while (0)
Only problem with this is formatting.  I strongly recommend you read the GNU
coding standards.

  do {
    fputs ("...");
    fprintf (...");
  } while (0)

Two space indention for nesting levels please, not tabs, not four spaces, but
two.  


  > + /*
  > +  * Long double support for MPE. The Quad functions are part of the PRO
  > +  * ABI, but it appears that I'm the only one interested in Quad support
  > +  * on PA, so I'll declare and support it only for MPE.
  > +  */
  > + #undef LONG_DOUBLE_TYPE_SIZE
  > + #define LONG_DOUBLE_TYPE_SIZE 128
Presumably you had to fix up other code to handle this :-)  I know it doesn't
"just work" :-)  I started down the 128bit long double road with the PA64
port and realized that it wasn't all that simple ;-)

  > + /* This is how to output an assembler line defining a `long double'
  > constant. */
/* This is how to output an assembler line defining a `long double'
   constant.  */

Note the indention.  Also two spaces after a period ending a sentence.

Basically it was OK -- most of the issues are style nits that you should be
able to clean up quickly and resubmit.

Thanks,
jeff


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