From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23520 invoked by alias); 1 May 2013 15:26:07 -0000 Mailing-List: contact libc-ports-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: libc-ports-owner@sourceware.org Received: (qmail 23509 invoked by uid 89); 1 May 2013 15:26:07 -0000 X-Spam-SWARE-Status: No, score=-3.3 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,TW_CP,TW_HW autolearn=ham version=3.3.1 Received: from mail-ie0-f170.google.com (HELO mail-ie0-f170.google.com) (209.85.223.170) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Wed, 01 May 2013 15:26:06 +0000 Received: by mail-ie0-f170.google.com with SMTP id at1so2026556iec.1 for ; Wed, 01 May 2013 08:26:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:cc:content-type:x-gm-message-state; bh=sJwvrDzdkpbpo4h4QuFnrvKww5TTaeNo6vYYnvSwbzE=; b=QXBcVqAZ7uFINOiANSJkfn5YOLAOv9BRVESqN2ZbHOkRSa8KP5LZYV+4W0EiF1Vx4O zHNzRUMzoxle6g9VFM+C2MgUikxGcwqPVqoVwTo3JJjj0ci2AkxnhRceaG7sWiU7gKHO H3gZ2jENn54EYOCq18Z44bWvBbn20WHD8PD5jm/LfXCig7EnT88AisMJBaufwJPU+6wF Q7h5640jM62fl5sP/TWIDzsweFqT729iWSwbXc6E471ARYsi3MDxRrnvVygBTNdLUY7g T6tSmIJ6j42mk0/Hfkyh/xmNTilaGlF5PglY+P7anVP6sCLUniPwwjmMM71yYVobUls4 mZnw== MIME-Version: 1.0 X-Received: by 10.42.161.4 with SMTP id r4mr1669930icx.8.1367421964981; Wed, 01 May 2013 08:26:04 -0700 (PDT) Received: by 10.64.170.142 with HTTP; Wed, 1 May 2013 08:26:04 -0700 (PDT) In-Reply-To: <20130430171818.697972C08A@topped-with-meat.com> References: <517FF73E.5020509@linaro.org> <20130430171818.697972C08A@topped-with-meat.com> Date: Wed, 01 May 2013 15:26:00 -0000 Message-ID: Subject: Re: [PATCH, v5] ARM: Add Cortex-A15 optimized NEON and VFP memcpy routines, with IFUNC. From: Will Newton To: Roland McGrath Cc: libc-ports@sourceware.org, Patch Tracking Content-Type: text/plain; charset=ISO-8859-1 X-Gm-Message-State: ALoCoQkyYktekesdoZszGIP+AWqxozCFFIoOm/KAKY3FucRKPM7P1cVUp4EZUy+B6BtkaLmdozYs X-SW-Source: 2013-05/txt/msg00014.txt.bz2 On 30 April 2013 18:18, Roland McGrath wrote: Hi Roland, Thanks for the review! >> +++ b/ports/sysdeps/arm/armv7/multiarch/aeabi_memcpy.c >> @@ -0,0 +1,33 @@ >> +/* Copyright (C) 2005-2013 Free Software Foundation, Inc. > > The first line of each new file should be a descriptive comment. > >> +void *__memcpy_arm (void *dest, const void *src, size_t n); >> + >> +/* Copy memory like memcpy, but no return value required. Can't alias >> + to memcpy because it's not defined in the same translation >> + unit. */ > > Why not just define the aliases in memcpy.S instead? > (Then this can be an empty file just to override arm/aeabi_memcpy.c.) > > You should also add some comments about why it's important that the > __aeabi_* functions use __memcpy_arm rather than memcpy. Done. >> +++ b/ports/sysdeps/arm/armv7/multiarch/ifunc-impl-list.c >> @@ -0,0 +1,44 @@ >> +/* Enumerate available IFUNC implementations of a function. arm version. > > ARM in caps. Done. >> + IFUNC_IMPL_ADD (array, i, memcpy, hwcap & HWCAP_ARM_VFPv3, >> + __memcpy_vfp) > > HWCAP_ARM_VFP. Yep, not sure how that slipped back in. >> +ENTRY(memcpy) >> + .type memcpy, %gnu_indirect_function >> + ldr r1, .Lmemcpy_arm >> + tst r0, #HWCAP_ARM_VFP >> + ldrne r1, .Lmemcpy_vfp > > If __SOFTFP__ is predefined by the compiler, then the compiler is presuming > VFP support anyway. So you can make this: > > #ifdef __SOFTFP__ > ldr r1, .Lmemcpy_arm > tst r0, #HWCAP_ARM_VFP > ldrne r1, .Lmemcpy_vfp > #else > ldr r1, .Lmemcpy_vfp > #endif > > (and also conditionalize .Lmemcpy_arm, below). I'm not sure I follow the logic here, could you elaborate? >> +# If we are configuring for armv7 we need binutils 2.21 to ensure that >> +# NEON alignments are assembled correctly. >> +if test $machine = arm/armv7; then >> + AC_CHECK_PROG_VER(AS, $AS, --version, >> + [GNU assembler.* \([0-9]*\.[0-9.]*\)], >> + [2.1[0-9][0-9]*|2.[2-9][1-9]*|[3-9].*|[1-9][0-9]*], AS=: critic_missing="$critic_missing as") >> +fi > > Just put this in sysdeps/arm/armv7/configure.in and don't test $machine. > Whenever possible, it is far better to test for an actual relevant detail > empirically rather than testing version numbers. If you can show an > example of an instruction sequence that is misassembled by binutils 2.20 > then we can help you construct a more precise configure check. I've moved the configure fragment as you suggest. I'll look into the creating a test for the assembler breakage. -- Will Newton Toolchain Working Group, Linaro