From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6248 invoked by alias); 3 Feb 2015 22:51:30 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 6237 invoked by uid 89); 3 Feb 2015 22:51:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 03 Feb 2015 22:51:28 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-01.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1YImJX-0003e2-TO from joseph_myers@mentor.com ; Tue, 03 Feb 2015 14:51:24 -0800 Received: from digraph.polyomino.org.uk (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.3.224.2; Tue, 3 Feb 2015 22:51:22 +0000 Received: from jsm28 (helo=localhost) by digraph.polyomino.org.uk with local-esmtp (Exim 4.82) (envelope-from ) id 1YImJV-00087H-0i; Tue, 03 Feb 2015 22:51:21 +0000 Date: Tue, 03 Feb 2015 22:51:00 -0000 From: Joseph Myers To: James Bowman CC: "gcc-patches@gcc.gnu.org" Subject: Re: [PATCH, FT32] initial support In-Reply-To: Message-ID: References: User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-SW-Source: 2015-02/txt/msg00172.txt.bz2 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 -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