From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 98338 invoked by alias); 25 Nov 2016 15:03:41 -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 98320 invoked by uid 89); 25 Nov 2016 15:03:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=occurred X-HELO: mail-qk0-f174.google.com Received: from mail-qk0-f174.google.com (HELO mail-qk0-f174.google.com) (209.85.220.174) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 25 Nov 2016 15:03:30 +0000 Received: by mail-qk0-f174.google.com with SMTP id x190so80653830qkb.0 for ; Fri, 25 Nov 2016 07:03:30 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=sOdAYxh8XAFwbkv6r3FZVLeLDmazsijxS5exJw8X288=; b=CgtJpoXsZwRe9fveeiChJ43JjVd3t5UDCex55xEdrUfccDmGeWQ9vEWNuGWmp0mhsG I2+/Kve0p6uR7oWMl9qJt3ZolaxN1BQRrAPFD2OR/utC4nYc9zKdd5lfYmVFWOKAABfe Qgr6hqVAH1p7t5UFnjb+vCa6L1UFhG76BkIoyhKlKfAVx/2yLwPaYoNF+IOQdIAi/HKj YoWuMV861IPlhqWRoMM+pPgRN5yywaiZx4ggSA1IPKwKcgwzGhxw4ixP1CTIE84j+uFX 6JJ6G10qw6ATQ8mkXaihLVfzNiWnnrWDD+mGsCBf3MvL+HpbSxC7rL7LNkSYX+tRyHb2 KEiQ== X-Gm-Message-State: AKaTC02mBx+fUkJhQ7oL3MfjneoNn0DaZEfBbA8CmC5kJGHOZ3NBDS9ILcfdjWTHmjNgVEHb0SzMLu3ryX0QHHcG X-Received: by 10.55.8.20 with SMTP id 20mr8173506qki.3.1480086208972; Fri, 25 Nov 2016 07:03:28 -0800 (PST) MIME-Version: 1.0 Received: by 10.140.28.226 with HTTP; Fri, 25 Nov 2016 07:03:28 -0800 (PST) In-Reply-To: References: From: Christophe Lyon Date: Fri, 25 Nov 2016 15:03:00 -0000 Message-ID: Subject: Re: [AArch64][ARM][GCC][PATCHv2 3/3] Add tests for missing Poly64_t intrinsics to GCC To: Tamar Christina Cc: GCC Patches , "christophe.lyon@st.com" , Marcus Shawcroft , Richard Earnshaw , James Greenhalgh , Kyrylo Tkachov , nd Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2016-11/txt/msg02639.txt.bz2 On 25 November 2016 at 15:53, Christophe Lyon wrote: > Hi Tamar, > > On 24 November 2016 at 12:45, Tamar Christina wrote: >> Hi Christoph, >> >> I have combined most of the tests in p64_p128 except for the >> vreinterpret_p128 and vreinterpret_p64 ones because I felt the number >> of code that would be have to be added to p64_p128 vs having them in those >> files isn't worth it. Since a lot of the test setup would have to be copied. >> > > A few comments about this new version: > * arm-neon-ref.h: why do you create CHECK_RESULTS_NAMED_NO_FP16_NO_POLY64? > Can't you just add calls to CHECK_CRYPTO in the existing > CHECK_RESULTS_NAMED_NO_FP16? > > * p64_p128: > From what I can see ARM and AArch64 differ on the vceq variants > available with poly64. > For ARM, arm_neon.h contains: uint64x1_t vceq_p64 (poly64x1_t __a, > poly64x1_t __b) > For AArch64, I can't see vceq_p64 in arm_neon.h? ... Actually I've just noticed > the other you submitted while I was writing this, where you add vceq_p64 for > aarch64, but it still returns uint64_t. > Why do you change the vceq_64 test to return poly64_t instead of uint64_t? > > Why do you add #ifdef __aarch64 before vldX_p64 tests and until vsli_p64? > > The comment /* vget_lane_p64 tests. */ is wrong before VLDX_LANE tests > > You need to protect the new vmov, vget_high and vget_lane tests with > #ifdef __aarch64__. > Actually, vget_high_p64 exists on arm, so no need for the #fidef for it. > Christophe > >> Kind regards, >> Tamar >> ________________________________________ >> From: Tamar Christina >> Sent: Tuesday, November 8, 2016 11:58:46 AM >> To: Christophe Lyon >> Cc: GCC Patches; christophe.lyon@st.com; Marcus Shawcroft; Richard Earnshaw; James Greenhalgh; Kyrylo Tkachov; nd >> Subject: RE: [AArch64][ARM][GCC][PATCHv2 3/3] Add tests for missing Poly64_t intrinsics to GCC >> >> Hi Christophe, >> >> Thanks for the review! >> >>> >>> A while ago I added p64_p128.c, to contain all the poly64/128 tests except for >>> vreinterpret. >>> Why do you need to create p64.c ? >> >> I originally created it because I had a much smaller set of intrinsics that I wanted to >> add initially, this grew and It hadn't occurred to me that I can use the existing file now. >> >> Another reason was the effective-target arm_crypto_ok as you mentioned below. >> >>> >>> Similarly, adding tests for vcreate_p64 etc... in p64.c or p64_p128.c might be >>> easier to maintain than adding them to vcreate.c etc with several #ifdef >>> conditions. >> >> Fair enough, I'll move them to p64_p128.c. >> >>> For vdup-vmod.c, why do you add the "&& defined(__aarch64__)" >>> condition? These intrinsics are defined in arm/arm_neon.h, right? >>> They are tested in p64_p128.c >> >> I should have looked for them, they weren't being tested before so I had >> Mistakenly assumed that they weren't available. Now I realize I just need >> To add the proper test option to the file to enable crypto. I'll update this as well. >> >>> Looking at your patch, it seems some tests are currently missing for arm: >>> vget_high_p64. I'm not sure why I missed it when I removed neont- >>> testgen... >> >> I'll adjust the test conditions so they run for ARM as well. >> >>> >>> Regarding vreinterpret_p128.c, doesn't the existing effective-target >>> arm_crypto_ok prevent the tests from running on aarch64? >> >> Yes they do, I was comparing the output against a clean version and hasn't noticed >> That they weren't running. Thanks! >> >>> >>> Thanks, >>> >>> Christophe