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: New port: Renesas RL78


On Wed, 2 Nov 2011, DJ Delorie wrote:

> -      unsigned int __max_iter = 1000000;
> +      unsigned int __max_iter = 65536U;

Doesn't that need to be 65535U for your purpose?

> Index: libgcc/config/rl78/lib2shift.c
> ===================================================================
> --- libgcc/config/rl78/lib2shift.c	(revision 0)
> +++ libgcc/config/rl78/lib2shift.c	(revision 0)
> @@ -0,0 +1,103 @@
> +/* Shift functions for the GCC support library for the Renesas RL78 processors.
> +   Copyright (C) 2011 Free Software Foundation, Inc.
> +   Contributed by Red Hat.
> +
> +   This file is part of GCC.
> +
> +   GCC is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3, or (at your option)
> +   any later version.
> +
> +   GCC is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with GCC; see the file COPYING3.  If not see
> +   <http://www.gnu.org/licenses/>.  */

Should have the runtime license exception.

> +FPBIT = fp-bit.c
> +$(gcc_objdir)/fp-bit.c: $(gcc_srcdir)/config/fp-bit.c
> +	echo '#define FLOAT'		     > $@
> +	cat $(gcc_srcdir)/config/fp-bit.c   >> $@
> +
> +DPBIT = dp-bit.c
> +$(gcc_objdir)/dp-bit.c: $(gcc_srcdir)/config/fp-bit.c
> +	cat $(gcc_srcdir)/config/fp-bit.c   >> $@

This should not be needed; just using t-fdpbit should suffice for almost 
all targets.

> \ No newline at end of file

As already noted, avoid this.

> +static int register_sizes[] =

const?

There are a lot of functions and variables without comments explaining 
them and their parameters.  (If a function just implements a standard 
hook, a comment identifying which hook suffices without needing to explain 
the parameters of that hook again.)

> +      if (letter == 'q' || letter == 'Q')
> +	error ("q/Q modifiers invalid for symbol referencs");

"referencs" should be "reference" or "references" or similar.  I think 
this should call output_operand_lossage not error, so that it becomes an 
error or an ICE as appropriate depending on whether the source of the 
error is the machine description or an asm.

> +      if (letter == 'q' || letter == 'Q')
> +	error ("q/Q modifiers invalid for symbol referencs");

Likewise.

I don't see updates to md.texi, contrib.texi or contrib/config-list.mk.  
(As regards the last, you should also check that the back end does build 
cleanly starting from a current native trunk compiler used to build a 
cross compiler configured with --enable-werror-always, on both 32-bit and 
64-bit hosts if possible.  contrib/config-list.mk is used for automatic 
checks of that sort of thing across all targets.)

I think the target-independent changes outside the back end (the ones that 
are meant as separate bug fixes required by the port) should be posted in 
separate messages, each with its own rationale.

The web site changes listed in sourcebuild.texi, "Back End", will also be 
needed.

-- 
Joseph S. Myers
joseph@codesourcery.com


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