From patchwork Mon Jun 8 09:33:18 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Lawrence X-Patchwork-Id: 481810 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 67EE81400A0 for ; Mon, 8 Jun 2015 19:33:35 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=aa1tBEJF; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type:content-transfer-encoding; q=dns; s= default; b=JA/JCs+lTWrN16dOGYp8TKNIuuWRztc24WYBagcSuOA2XpcHLMiAQ MumZysk34GXaJbig1YXYOZ0Z/YGhfSVPlpI47o338ok93JFq/gKpPFgVXWDXKgkT 71x/jrEtSO5iJ1aAG4yFmoeBRMiyZzfMMS948aLezuIOkeh9OqzXaU= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type:content-transfer-encoding; s=default; bh=C6dMBuslJwOWh8+yCOfcGCWplxA=; b=aa1tBEJFenZ1b8riBNrRKZcBfwwO VTe5QOEkCFqs6bYSPixL7SJvGv+Y5H8sLvGRVHwBLP+bos9n1E7eOZaw/Kb/5BjR +1ZklJtpN3sm6h3Oc9FS6EfVNH31FNFzyHDjs/WCS5Qiql55fLPIH1ujyF2+5AVm 3i6kmO+O9LLSiGw= Received: (qmail 125698 invoked by alias); 8 Jun 2015 09:33:27 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 125687 invoked by uid 89); 8 Jun 2015 09:33:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (146.101.78.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 08 Jun 2015 09:33:23 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by uk-mta-17.uk.mimecast.lan; Mon, 08 Jun 2015 10:33:19 +0100 Received: from [10.2.207.65] ([10.1.2.79]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Mon, 8 Jun 2015 10:33:18 +0100 Message-ID: <5575615E.30509@arm.com> Date: Mon, 08 Jun 2015 10:33:18 +0100 From: Alan Lawrence User-Agent: Thunderbird 2.0.0.24 (X11/20101213) MIME-Version: 1.0 To: Charles Baylis CC: GCC Patches , Tejas Belagod , Marcus Shawcroft , Richard Earnshaw Subject: Re: [PATCH] [AArch64] PR63870 Improve error messages for NEON single lane memory access intrinsics References: In-Reply-To: X-MC-Unique: U_sYAJl3TLu4MvqeEMTMSQ-1 X-IsSubscribed: yes Thanks for working on this! I'd been fiddling around with a patch with some similar elements to this, but many trials with union types, subregs, etc., all worsened the register allocation and led to more unnecessary shuffling / moves. The only real thing I tried which you don't do here, was to introduce a set_dreg expander to clean up some of those macro definitions in arm_neon.h. That could easily follow in a separate patch if desired! So your patch looks good to me. A couple of style nits: also the dg-error messages in the testsuite, do not need to be on the same line as the statement generating the error, because the trailing 0 tells dg that the position/line number doesn't matter (i.e. dg should allow the error to be reported at any line); so these could be brought under 80 chars. Thanks, Alan Charles Baylis wrote: > This is another attempt at fixing this PR63870 for AArch64 (ARM is > still to come). > > As before, the Q register variants are handled by moving the check for > the lane bounds into builtin expansion. The handling of lane numbers > is made consistent wrt endianess with other NEON single lane > operations - lane numbers in RTL are flipped for big-endian, and > flipped back at assembly time. > > The D register variants are now handled by adding new builtins for all > the 64bit operations. These behave identically to Q register variants, > except that the permitted lane bounds are different. > > In the iterators used by the relevant patterns are changed from VQ and > VALLDIF so that the correct vector sizes are used in the endian-flip > at assembly time. > > Finally, a set of machine-generated test cases is added. These do need > to be in separate files, because of testsuite limitations. > > Regression tested on qemu for aarch64-linux-gnu with no regressions > and all new tests pass. > > OK for trunk? > > > gcc/ChangeLog: > > Charles Baylis > > PR target/63870 > * config/aarch64/aarch64-builtins.c (enum aarch64_type_qualifiers): > Add qualifier_struct_load_store_lane_index. > (aarch64_types_loadstruct_lane_qualifiers): Use > qualifier_struct_load_store_lane_index for lane index argument for > last argument. > (aarch64_types_storestruct_lane_qualifiers): Ditto. > (builtin_simd_arg): Add SIMD_ARG_STRUCT_LOAD_STORE_LANE_INDEX. > (aarch64_simd_expand_args): Add new argument describing mode of > builtin. Check lane bounds for arguments with > SIMD_ARG_STRUCT_LOAD_STORE_LANE_INDEX. > (aarch64_simd_expand_builtin): Emit error for incorrect lane indices > if marked with SIMD_ARG_STRUCT_LOAD_STORE_LANE_INDEX. > (aarch64_simd_expand_builtin): Handle arguments with > qualifier_struct_load_store_lane_index. Pass machine mode of builtin to > aarch64_simd_expand_args. > * config/aarch64/aarch64-simd-builtins.def: Declare ld[234]_lane and > vst[234]_lane with BUILTIN_VALLDIF. > * config/aarch64/aarch64-simd.md: > (aarch64_vec_load_lanesoi_lane): Use VALLDIF iterator. Perform > endianness reversal on lane index. > (aarch64_vec_load_lanesci_lane): Ditto. > (aarch64_vec_load_lanesxi_lane): Ditto. > (vec_store_lanesoi_lane): Use VALLDIF iterator. Fix typo > in attribute. > (vec_store_lanesci_lane): Use VALLDIF iterator. > (vec_store_lanesxi_lane): Ditto. > (aarch64_ld2_lane): Use VALLDIF iterator. Remove endianness > reversal of lane index. > (aarch64_ld3_lane): Ditto. > (aarch64_ld4_lane): Ditto. > (aarch64_st2_lane): Ditto. > (aarch64_st3_lane): Ditto. > (aarch64_st4_lane): Ditto. > * config/aarch64/arm_neon.h (__LD2_LANE_FUNC): Rename mode parameter > to qmode. Add new mode parameter. Update uses. > (__LD3_LANE_FUNC): Ditto. > (__LD4_LANE_FUNC): Ditto. > (__ST2_LANE_FUNC): Ditto. > (__ST3_LANE_FUNC): Ditto. > (__ST4_LANE_FUNC): Ditto. > > > Charles Baylis > > * gcc.target/aarch64/simd/vld2_lane_f32_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2_lane_f64_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2_lane_p8_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2_lane_s16_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2_lane_s32_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2_lane_s64_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2_lane_s8_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2_lane_u16_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2_lane_u32_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2_lane_u64_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2_lane_u8_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2q_lane_f32_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2q_lane_f64_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2q_lane_p8_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2q_lane_s16_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2q_lane_s32_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2q_lane_s64_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2q_lane_s8_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2q_lane_u16_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2q_lane_u32_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2q_lane_u64_indices_1.c: New test. > * gcc.target/aarch64/simd/vld2q_lane_u8_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3_lane_f32_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3_lane_f64_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3_lane_p8_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3_lane_s16_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3_lane_s32_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3_lane_s64_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3_lane_s8_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3_lane_u16_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3_lane_u32_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3_lane_u64_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3_lane_u8_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3q_lane_f32_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3q_lane_f64_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3q_lane_p8_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3q_lane_s16_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3q_lane_s32_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3q_lane_s64_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3q_lane_s8_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3q_lane_u16_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3q_lane_u32_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3q_lane_u64_indices_1.c: New test. > * gcc.target/aarch64/simd/vld3q_lane_u8_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4_lane_f32_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4_lane_f64_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4_lane_p8_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4_lane_s16_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4_lane_s32_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4_lane_s64_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4_lane_s8_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4_lane_u16_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4_lane_u32_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4_lane_u64_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4_lane_u8_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4q_lane_f32_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4q_lane_f64_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4q_lane_p8_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4q_lane_s16_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4q_lane_s32_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4q_lane_s64_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4q_lane_s8_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4q_lane_u16_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4q_lane_u32_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4q_lane_u64_indices_1.c: New test. > * gcc.target/aarch64/simd/vld4q_lane_u8_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2_lane_f32_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2_lane_f64_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2_lane_p8_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2_lane_s16_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2_lane_s32_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2_lane_s64_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2_lane_s8_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2_lane_u16_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2_lane_u32_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2_lane_u64_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2_lane_u8_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2q_lane_f32_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2q_lane_f64_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2q_lane_p8_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2q_lane_s16_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2q_lane_s32_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2q_lane_s64_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2q_lane_s8_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2q_lane_u16_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2q_lane_u32_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2q_lane_u64_indices_1.c: New test. > * gcc.target/aarch64/simd/vst2q_lane_u8_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3_lane_f32_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3_lane_f64_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3_lane_p8_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3_lane_s16_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3_lane_s32_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3_lane_s64_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3_lane_s8_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3_lane_u16_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3_lane_u32_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3_lane_u64_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3_lane_u8_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3q_lane_f32_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3q_lane_f64_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3q_lane_p8_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3q_lane_s16_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3q_lane_s32_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3q_lane_s64_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3q_lane_s8_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3q_lane_u16_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3q_lane_u32_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3q_lane_u64_indices_1.c: New test. > * gcc.target/aarch64/simd/vst3q_lane_u8_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4_lane_f32_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4_lane_f64_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4_lane_p8_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4_lane_s16_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4_lane_s32_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4_lane_s64_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4_lane_s8_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4_lane_u16_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4_lane_u32_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4_lane_u64_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4_lane_u8_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4q_lane_f32_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4q_lane_f64_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4q_lane_p8_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4q_lane_s16_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4q_lane_s32_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4q_lane_s64_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4q_lane_s8_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4q_lane_u16_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4q_lane_u32_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4q_lane_u64_indices_1.c: New test. > * gcc.target/aarch64/simd/vst4q_lane_u8_indices_1.c: New test. --- a/gcc/config/aarch64/aarch64-builtins.c +++ b/gcc/config/aarch64/aarch64-builtins.c @@ -128,7 +128,9 @@ enum aarch64_type_qualifiers /* Polynomial types. */ qualifier_poly = 0x100, /* Lane indices - must be in range, and flipped for bigendian. */ - qualifier_lane_index = 0x200 + qualifier_lane_index = 0x200, + /* Lane indices for single lane structure loads and stores */ + qualifier_struct_load_store_lane_index = 0x400 }; should be ...'loads and stores. */'