From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1XVjgo-0007d7-VH for mharc-grub-devel@gnu.org; Sun, 21 Sep 2014 12:08:42 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35557) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XVjgi-0007XV-SQ for grub-devel@gnu.org; Sun, 21 Sep 2014 12:08:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XVjge-0006Cq-5X for grub-devel@gnu.org; Sun, 21 Sep 2014 12:08:36 -0400 Received: from mail-wg0-x22d.google.com ([2a00:1450:400c:c00::22d]:64955) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XVjgd-0006CA-RY for grub-devel@gnu.org; Sun, 21 Sep 2014 12:08:32 -0400 Received: by mail-wg0-f45.google.com with SMTP id x13so1060377wgg.16 for ; Sun, 21 Sep 2014 09:08:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:subject:references :in-reply-to:content-type; bh=T359w8GePvV8+Bh6+q5MXoOBic7ZpAK0i//G8hXCPoM=; b=Z5r7yzNYq/HspviH1IJNnkw3g9cyUmPHlrFdP6E7gTSwjebkbpbWXbpRZdIGet+qOU /FTAanJYYy+D7kdX8m3Rs9df3SyGbKZ1QuVpR7TalYASGCkarMpBjWP2dHAb9sTAE9MA bcCyiP88Msnw/1XYIHsES1zevvSn+QrcENJQqLUM3yDrX/54D6K9s8SyZQ+aMJRC9bXs vy3YxbMmtX9/2sU3exoUIqvH2upWVoNzaMzGF7SmyZYlHe2OQv4zi58kOZtwRaFMqBj3 MCxNBv/F0LgCrWI7Qp7rHVX/gX/TwgT2hj21vHm1vCB9cJtTk/wDaXCxqnb8e8W2zFAY C14g== X-Received: by 10.180.83.39 with SMTP id n7mr15667614wiy.0.1411315706140; Sun, 21 Sep 2014 09:08:26 -0700 (PDT) Received: from [192.168.42.4] (160-228.197-178.cust.bluewin.ch. [178.197.228.160]) by mx.google.com with ESMTPSA id u10sm9123148wix.2.2014.09.21.09.08.18 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 21 Sep 2014 09:08:25 -0700 (PDT) Message-ID: <541EF7A9.1020005@gmail.com> Date: Sun, 21 Sep 2014 18:07:05 +0200 From: =?UTF-8?B?VmxhZGltaXIgJ8+GLWNvZGVyL3BoY29kZXInIFNlcmJpbmVua28=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.0 MIME-Version: 1.0 To: The development of GNU GRUB Subject: Re: [RFC PATCH 2/3] Files reorganization and include some libgcc fuctions References: <1409255765-3209-1-git-send-email-pfsmorigo@linux.vnet.ibm.com> <1409255765-3209-3-git-send-email-pfsmorigo@linux.vnet.ibm.com> <20140908021620.GA9140@riva.ucam.org> <20140917214336.GA13292@beren.chocolate> In-Reply-To: <20140917214336.GA13292@beren.chocolate> Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="xD2tGgChGouClLW01g4hUF3pHQtjUdXSC" X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:400c:c00::22d X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 21 Sep 2014 16:08:41 -0000 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --xD2tGgChGouClLW01g4hUF3pHQtjUdXSC Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 17.09.2014 23:43, Paulo Flabiano Smorigo wrote: > Colin, I changed the patches following your suggestions and making it > more likely to the no-libgcc branch from Vladimir. In this branch, > phcoder added compiler-rt.{c,h,S} with the necessary code in it. >=20 compiler-rt.{c,S} fixes bunch of possible problems when libgcc doesn't match the intended platform (e.g. because of SSE flags). So I think we should have whole thing go in. Biggest problem is the lack of tests for those functions. > My approach is very minimalist and only for powerpc. I tried to avoid > change the behavior for other architecture since we are in code freeze.= >=20 > In the future we can think in spread this approach for all archs and us= e > all implementation that phcoder is doing in the no-libgcc branch. >=20 > Mon, Sep 08, 2014 at 03:16:21AM +0100, Colin Watson wrote: >> On Thu, Aug 28, 2014 at 04:56:04PM -0300, Paulo Flabiano Smorigo wrote= : >>> diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c >>> index c5c815d..353a207 100644 >>> --- a/grub-core/kern/misc.c >>> +++ b/grub-core/kern/misc.c >>> @@ -1342,3 +1342,110 @@ grub_real_boot_time (const char *file, >>> grub_error_pop (); >>> } >>> #endif >>> + >>> +#if defined (NO_LIBGCC) >> >> Should this perhaps be restricted to __powerpc__ as well? >> Alternatively, the prototypes in include/grub/compiler.h (or >> include/grub/misc.h; see below) should be used on other architectures >> too. Either way, the declarations and definitions should match. >=20 > Yes, now I only use __powerpc__ >=20 >> >>> diff --git a/include/grub/compiler.h b/include/grub/compiler.h >>> index c9e1d7a..a9a684c 100644 >>> --- a/include/grub/compiler.h >>> +++ b/include/grub/compiler.h >>> @@ -48,4 +48,65 @@ >>> # define WARN_UNUSED_RESULT >>> #endif >>> =20 >>> +#include "types.h" >> >> Shouldn't this be #include , assuming that you need this= >> for grub_uint*_t? Also, includes should generally be grouped at the t= op >> of the file. >=20 > All in the compile-rt.c now >=20 >> >>> +union component64 >>> +{ >>> + grub_uint64_t full; >>> + struct >>> + { >>> +#ifdef GRUB_CPU_WORDS_BIGENDIAN >>> + grub_uint32_t high; >>> + grub_uint32_t low; >>> +#else >>> + grub_uint32_t low; >>> + grub_uint32_t high; >>> +#endif >>> + }; >>> +}; >> >> This is only used by grub-core/kern/misc.c. Please move it there rath= er >> than putting it somewhere that's included by everything in GRUB. >> >>> +#if defined (__powerpc__) >> >> Should this be #if defined (__powerpc__) && defined (NO_LIBGCC) or >> something similar, to match the general way things are set up in >> configure.ac? (Also see comment above about declarations matching >> definitions.) >=20 > Fixed. >=20 >> >> Relatedly, have you tested this patch set with a native build on a >> 32-bit BE powerpc system, as opposed to 32-bit BE built on a 64-bit LE= >> system? This looks like a potential problem there. >> >=20 > Yes. I tested it in both BE and LE systems. No problem found. I did > another round of test with this changes and will do some more this week= =2E >=20 >>> +grub_uint64_t EXPORT_FUNC (__lshrdi3) (grub_uint64_t u, int b); >>> +grub_uint64_t EXPORT_FUNC (__ashrdi3) (grub_uint64_t u, int b); >>> +grub_uint64_t EXPORT_FUNC (__ashldi3) (grub_uint64_t u, int b); >>> +int EXPORT_FUNC(__ucmpdi2) (grub_uint64_t a, grub_uint64_t b); >>> +void EXPORT_FUNC (_restgpr_14_x) (void); >> [...] >> >> These aren't compiler features, so don't belong in >> include/grub/compiler.h. Other architectures seem to have this kind o= f >> thing in include/grub/misc.h inside a big #ifndef GRUB_UTIL conditiona= l, >> so please move all this to there. >=20 > Yes, I follow what phcoder did in no-libgcc branch. This lines are in > the compiler-rt.c file. >=20 >> >>> diff --git a/include/grub/libgcc.h b/include/grub/libgcc.h >>> index 8e93b67..5bdb8fb 100644 >>> --- a/include/grub/libgcc.h >>> +++ b/include/grub/libgcc.h >>> @@ -16,73 +16,6 @@ >>> * along with GRUB. If not, see . >>> */ >>> =20 >>> -/* We need to include config-util.h.in for HAVE_*. */ >>> -#ifndef __STDC_VERSION__ >>> -#define __STDC_VERSION__ 0 >>> -#endif >>> -#include >>> - >>> -/* On x86 these functions aren't really needed. Save some space. */= >>> -#if !defined (__i386__) && !defined (__x86_64__) >>> -# ifdef HAVE___ASHLDI3 >>> -void EXPORT_FUNC (__ashldi3) (void); >>> -# endif >>> -# ifdef HAVE___ASHRDI3 >>> -void EXPORT_FUNC (__ashrdi3) (void); >>> -# endif >>> -# ifdef HAVE___LSHRDI3 >>> -void EXPORT_FUNC (__lshrdi3) (void); >>> -# endif >>> -# ifdef HAVE___UCMPDI2 >>> -void EXPORT_FUNC (__ucmpdi2) (void); >>> -# endif >>> -# ifdef HAVE___BSWAPSI2 >>> -void EXPORT_FUNC (__bswapsi2) (void); >>> -# endif >>> -# ifdef HAVE___BSWAPDI2 >>> -void EXPORT_FUNC (__bswapdi2) (void); >>> -# endif >>> -#endif >>> - >>> -#ifdef HAVE__RESTGPR_14_X >>> -void EXPORT_FUNC (_restgpr_14_x) (void); >>> -void EXPORT_FUNC (_restgpr_15_x) (void); >>> -void EXPORT_FUNC (_restgpr_16_x) (void); >>> -void EXPORT_FUNC (_restgpr_17_x) (void); >>> -void EXPORT_FUNC (_restgpr_18_x) (void); >>> -void EXPORT_FUNC (_restgpr_19_x) (void); >>> -void EXPORT_FUNC (_restgpr_20_x) (void); >>> -void EXPORT_FUNC (_restgpr_21_x) (void); >>> -void EXPORT_FUNC (_restgpr_22_x) (void); >>> -void EXPORT_FUNC (_restgpr_23_x) (void); >>> -void EXPORT_FUNC (_restgpr_24_x) (void); >>> -void EXPORT_FUNC (_restgpr_25_x) (void); >>> -void EXPORT_FUNC (_restgpr_26_x) (void); >>> -void EXPORT_FUNC (_restgpr_27_x) (void); >>> -void EXPORT_FUNC (_restgpr_28_x) (void); >>> -void EXPORT_FUNC (_restgpr_29_x) (void); >>> -void EXPORT_FUNC (_restgpr_30_x) (void); >>> -void EXPORT_FUNC (_restgpr_31_x) (void); >>> -void EXPORT_FUNC (_savegpr_14) (void); >>> -void EXPORT_FUNC (_savegpr_15) (void); >>> -void EXPORT_FUNC (_savegpr_16) (void); >>> -void EXPORT_FUNC (_savegpr_17) (void); >>> -void EXPORT_FUNC (_savegpr_18) (void); >>> -void EXPORT_FUNC (_savegpr_19) (void); >>> -void EXPORT_FUNC (_savegpr_20) (void); >>> -void EXPORT_FUNC (_savegpr_21) (void); >>> -void EXPORT_FUNC (_savegpr_22) (void); >>> -void EXPORT_FUNC (_savegpr_23) (void); >>> -void EXPORT_FUNC (_savegpr_24) (void); >>> -void EXPORT_FUNC (_savegpr_25) (void); >>> -void EXPORT_FUNC (_savegpr_26) (void); >>> -void EXPORT_FUNC (_savegpr_27) (void); >>> -void EXPORT_FUNC (_savegpr_28) (void); >>> -void EXPORT_FUNC (_savegpr_29) (void); >>> -void EXPORT_FUNC (_savegpr_30) (void); >>> -void EXPORT_FUNC (_savegpr_31) (void); >>> -#endif >>> - >>> #if defined (__arm__) >>> void EXPORT_FUNC (__aeabi_lasr) (void); >>> void EXPORT_FUNC (__aeabi_llsl) (void); >> >> I don't think you should touch this file at all. I don't know precise= ly >> how it's used, but it only seems to be used to generate symbol lists; >> furthermore, you're removing some things from it that are not clearly >> powerpc-specific and either making them powerpc-specific (__ashldi3, >> __ashrdi3, __lshrdi3, __ucmpdi2) or dropping them altogether >> (__bswapsi2, __bswapdi2). This is worrisome and suggests the >> possibility that this will break other architectures. Does your patch= >> work if you just leave include/grub/libgcc.h unmodified? >=20 > Please, check this new patchset. libgcc.h is intact. >=20 >> >> Thanks, >> >> --=20 >> Colin Watson [cjwatson@ubuntu.co= m] >> >> _______________________________________________ >> Grub-devel mailing list >> Grub-devel@gnu.org >> https://lists.gnu.org/mailman/listinfo/grub-devel >> >=20 >=20 >=20 > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel >=20 --xD2tGgChGouClLW01g4hUF3pHQtjUdXSC Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iF4EAREKAAYFAlQe96kACgkQmBXlbbo5nOtHPgEAkJ1j0UB1Gibhc5pgkG3etQLI w8uFK0K9JruoArtvQBcA/3SnvVQOPtHxw0Y0RBwaMa9b456VyqeHm4XqB/Unx9x/ =K0i8 -----END PGP SIGNATURE----- --xD2tGgChGouClLW01g4hUF3pHQtjUdXSC--