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: [PATCH, FT32] initial support


This patch is missing pieces such as Texinfo documentation (in
invoke.texi for target-specific options, at least) and config-list.mk
update so automatic builders verify that this target builds OK.  See
"Back End" in sourcebuild.texi and make sure that you have everything
relevant.

It's a good idea to make sure any new port builds cleanly on both 32-bit 
and 64-bit hosts when configured --enable-werror-always and the compiler 
used to build it is the same version of GCC (this provides equivalent 
coverage for being free of compilation warnings as bootstrap does for 
native tools).

> Index: gcc/config/ft32/ft32.md
> ===================================================================
> --- gcc/config/ft32/ft32.md	(revision 0)
> +++ gcc/config/ft32/ft32.md	(revision 0)
> @@ -0,0 +1,965 @@
> +;; Machine description for FT32
> +;; Copyright (C) 2009 Free Software Foundation, Inc.

Copyright years should be <year>-2015.

> +        internal_error ("internal error: 'h' applied to non-register operand");

internal_error already prints "internal compiler error: "; don't
include "internal error: " in the error string.  Use %< and %> as
quotes rather than ''.

> +            internal_error ("internal error: bad alignment: %d", i);

Likewise.

> +        int bf = ft32_as_bitfield(INTVAL(x));

Missing spaces before '(' in function and macro calls; check for any
other instances.

> +      /* if (REGNO (operand) > FT32_R13) internal_error ("internal error: bad register: %d", REGNO (operand));  wtf */

Even commented-out calls (if really needed) should be cleaned up.

> +  if (65536 <= cfun->machine->size_for_adjusting_sp) {
> +    error("Stack frame must be smaller than 64K");

Start diagnostics with a lowercase letter.  Note a missing space again.

> +#if 0
> +/* fixed-bit.h does not define functions for TA and UTA because
> +   that part is wrapped in #if MIN_UNITS_PER_WORD > 4.
> +   This would lead to empty functions for TA and UTA.
> +   Thus, supply appropriate defines as if HAVE_[U]TA == 1.
> +   #define HAVE_[U]TA 1 won't work because avr-modes.def
> +   uses ADJUST_BYTESIZE(TA,8) and fixed-bit.h is not generic enough
> +   to arrange for such changes of the mode size.  */

Don't include large amounts of #if 0 or commented-out code in a new
port (*any* such code needs a good justification).

> +ft32-*-elf)
> +	# tmake_file="ft32/t-ft32 t-softfp-sfdf t-softfp-excl t-softfp"

Why commented-out?  soft-fp is to be preferred to fp-bit (except for
8-bit and 16-bit ports where space is very critical).  Also,
t-softfp-excl shouldn't be needed for new ports; it's only really
relevant when soft-fp is being built for hard-float multilibs for some
reason (t-hardfp is preferred then).

If the issue is that you want to exclude some soft-fp functions from
being built because you have architecture-specific versions of them,
it should be straightforward to add a new softfp_exclusions variable
tht t-softfp respects (in a separate patch, please).

> +	# tm_file="$tm_file ft32/ft32-lib.h"

Unless you're using this file, don't include it in the patch at all.


-- 
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]