From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32027 invoked by alias); 15 Dec 2014 13:38:44 -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 32017 invoked by uid 89); 15 Dec 2014 13:38:43 -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 X-HELO: mail-qc0-f178.google.com Received: from mail-qc0-f178.google.com (HELO mail-qc0-f178.google.com) (209.85.216.178) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 15 Dec 2014 13:38:41 +0000 Received: by mail-qc0-f178.google.com with SMTP id b13so9479486qcw.37 for ; Mon, 15 Dec 2014 05:38:39 -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:date :message-id:subject:from:to:cc:content-type; bh=tnbPbmsxUljs3exxLSAGM4lzPveAVAGLZNKHYsJiBSQ=; b=OeMAQa/i57o3j30D2AqoJxuIGI/H3XfZwdhdHo88QIfaELbAkoWkUsJ/dsXDtZ5KHT rBXI15tILlMLnXEmFEMEcbBX+8cXeV13+SRByqoFjMiqGqnL6JJq2VE59P6KeKudLmNt S/sA+w2gL6/FwJHNwP6gsPXGPMk3kq9E9CXL6x/GDDqCQxVX1YDSaJ+p1kVD/d2l1RZJ 3XlZp+WB74vFZzhoSLiTn4xCPdxhn1fsh18bIAVQfXTt67dGarnEthDhEB03v4drkd6z tFd5nhjGB2g65GTpywcM9pH+iUajqbFbmhEXFbHR99nGa88uKYrEgmWKPreFhI0DFN30 iEgg== X-Gm-Message-State: ALoCoQmeFJAvBLwe86IKvAEPBEBbALjgpza5z3/FTf+Itd6mbEBtZ+FpHF5oT62l6O5zMzkw80yP MIME-Version: 1.0 X-Received: by 10.140.46.52 with SMTP id j49mr52475531qga.30.1418650719037; Mon, 15 Dec 2014 05:38:39 -0800 (PST) Received: by 10.140.21.234 with HTTP; Mon, 15 Dec 2014 05:38:38 -0800 (PST) In-Reply-To: <000201d0184d$5d6e6a10$184b3e30$@arm.com> References: <000701cfff29$ea4fbcf0$beef36d0$@arm.com> <000b01cfff4e$aa668090$ff3381b0$@arm.com> <000c01d00310$20bcab60$62360220$@arm.com> <000101d0152b$91105c90$b33115b0$@arm.com> <000201d0184d$5d6e6a10$184b3e30$@arm.com> Date: Mon, 15 Dec 2014 13:48:00 -0000 Message-ID: Subject: Re: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe. From: Christophe Lyon To: David Sherwood Cc: "gcc-patches@gcc.gnu.org" , Marcus Shawcroft , Alan Hayward , Tejas Belagod , Richard Sandiford Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2014-12/txt/msg01245.txt.bz2 On 15 December 2014 at 10:56, David Sherwood wrote: > > >> -----Original Message----- >> From: Christophe Lyon [mailto:christophe.lyon@linaro.org] >> Sent: 11 December 2014 13:47 >> To: David Sherwood >> Cc: gcc-patches@gcc.gnu.org; Marcus Shawcroft; Alan Hayward; Tejas Belagod; Richard Sandiford >> Subject: Re: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe. >> >> On 11 December 2014 at 11:16, David Sherwood wrote: >> > Hi Christophe, >> > >> > Sorry to bother you again. After my clarification email below are you now >> > happy for these patches to go in? >> > >> > Kind Regards, >> > David Sherwood. >> > >> >> -----Original Message----- >> >> From: David Sherwood [mailto:david.sherwood@arm.com] >> >> Sent: 27 November 2014 14:53 >> >> To: 'Christophe Lyon' >> >> Cc: gcc-patches@gcc.gnu.org; Marcus Shawcroft; Alan Hayward; 'Tejas Belagod'; Richard Sandiford >> >> Subject: RE: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe. >> >> >> >> > On 18 November 2014 10:14, David Sherwood wrote: >> >> > > Hi Christophe, >> >> > > >> >> > > Ah sorry. My mistake - it fixes this in bugzilla: >> >> > > >> >> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59810 >> >> > >> >> > I did look at that PR, but since it has no testcase attached, I was unsure. >> >> > And I am still not :-) >> >> > PR 59810 is "[AArch64] LDn/STn implementations are not ABI-conformant >> >> > for bigendian." >> >> > but the advsimd-intrinsics/vldX.c and vldX_lane.c now PASS with Alan's >> >> > patches on aarch64_be, so I thought Alan's patches solve PR59810. >> >> > >> >> > What am I missing? >> >> >> >> Hi Christophe, >> >> >> >> I think probably this is our fault for making our lives way too difficult and >> >> artificially splitting all these patches up. :) >> >> >> >> Alan's patch: >> >> >> >> https://gcc.gnu.org/ml/gcc-patches/2014-10/msg00952.html >> >> >> >> fixes some issues on aarch64_be, but also causes regressions. For example, >> >> >> >> ==== >> >> Tests that now fail, but worked before: >> >> >> >> aarch64_be-elf-aem: gcc.dg/vect/slp-perm-8.c -flto -ffat-lto-objects execution test >> >> aarch64_be-elf-aem: gcc.dg/vect/slp-perm-8.c execution test >> >> aarch64_be-elf-aem: gcc.dg/vect/vect-over-widen-1-big-array.c -flto -ffat-lto-objects execution test >> >> ... >> >> >> >> Tests that now work, but didn't before: >> >> >> >> aarch64_be-elf-aem: gcc.dg/vect/fast-math-vect-complex-3.c execution test >> >> aarch64_be-elf-aem: gcc.dg/vect/if-cvt-stores-vect-ifcvt-18.c execution test >> >> aarch64_be-elf-aem: gcc.dg/vect/no-scevccp-outer-10a.c execution test >> >> ... >> >> ==== >> I didn't notice that because I tested Alan's patch only against the >> advsimd-intrinsics tests. >> In this respect, I don't understand why your ChangeLog entry says >> * config/aarch64/aarch64-simd.md (vec_store_lanes(o/c/x)i, >> vec_load_lanes(o/c/x)i): Fixed to work for Big Endian. >> since the existing advsimd-intrinsics tests already pass with Alan's patch alone >> or is vld1_lane still broken (for which I haven't posted a test yet)? >> > Yes, I think the change log is unclear and I will change it. The only thing that was > broken was not adhering to the ABI, but we don't have any specific regression tests > that prove this. > OK thanks for the clarification. >> >> His patch is only half of the story and must be applied at the same time as the >> >> "[AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe." >> >> patch. With both patches applied the result looks much healthier: >> >> >> >> ==== >> >> # Comparing 1 common sum files >> >> ## /bin/sh ./src/gcc/contrib/compare_tests /tmp/gxx-sum1.10051 /tmp/gxx-sum2.10051 >> >> Tests that now work, but didn't before: >> >> >> >> aarch64_be-elf-aem: gcc.dg/torture/pr52028.c -O3 -fomit-frame-pointer execution test >> >> aarch64_be-elf-aem: gcc.dg/torture/pr52028.c -O3 -fomit-frame-pointer -funroll-all-loops -finline- >> >> functions execution test >> >> aarch64_be-elf-aem: gcc.dg/torture/pr52028.c -O3 -fomit-frame-pointer -funroll-loops execution >> test >> >> ... >> >> ==== >> >> >> >> with no new regressions. After applying both patches the aarch64_be gcc testsuite is >> >> on a parity with the aarch64 testsuite. Furthermore, after applying both of these patches: >> >> >> >> "[AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe" >> >> "[AArch64] [BE] Fix vector load/stores to not use ld1/st1" >> >> >> >> it then becomes safe for us to remove the CCMC macro, which is the cause of >> >> unnecessary spills to the stack for certain auto-vectorised code. So really I >> >> suppose when I posted my second patch >> >> >> >> "[AArch64] [BE] [2/2] Make large opaque integer modes endianness-safe" >> >> >> >> I should have really just called this >> >> >> >> "[AArch64] [BE] Remove CCMC for aarch64" >> >> >> >> in order to make it clear exactly what the purpose of these patches is. >> well, not yet since this very does not remove it :-) >> > Again, this is my fault as I made a mistake in the change log. If you look at the > actual patch the CCMC macro is removed. Let me re-post corrected, more > sensible change logs for both of those changes here: > > "[AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe" > ChangeLog: > > gcc/: > 2014-10-10 David Sherwood > 2014-10-10 Tejas Belagod > > * config/aarch64/aarch64-protos.h (aarch64_simd_attr_length_rglist, > aarch64_reverse_mask): New decls. > * config/aarch64/iterators.md (UNSPEC_REV_REGLIST): New enum. > * config/aarch64/iterators.md (insn_count): New mode_attr. > * config/aarch64/aarch64-simd.md (vec_store_lanesoi, vec_store_lanesci, > vec_store_lanesxi, vec_load_lanesoi, vec_load_lanesci, vec_load_lanesxi) > : Made ABI compliant for Big Endian targets. > * config/aarch64/aarch64-simd.md (aarch64_rev_reglist, aarch64_simd_ld2, > aarch64_simd_ld3, aarch64_simd_ld4, aarch64_simd_st2, aarch64_simd_st3, > aarch64_simd_st4): Added. > * config/aarch64/aarch64.c (aarch64_simd_attr_length_rglist, > aarch64_reverse_mask): Added. > > "[AArch64] [BE] [2/2] Make large opaque integer modes endianness-safe" > ChangeLog: > > gcc/: > 2014-13-10 David Sherwood > 2014-13-10 Tejas Belagod > > * config/aarch64/aarch64.h (CANNOT_CHANGE_MODE_CLASS): Removed. > * config/aarch64/aarch64.c (aarch64_cannot_change_mode_class): Removed. > * config/aarch64/aarch64-protos.h (aarch64_cannot_change_mode_class): > Removed. > > Again, apologies for the confusion, > David. > >> >> >> >> Kind Regards, >> >> David Sherwood. >> > >> > >> > >> > > > > >