From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32438 invoked by alias); 13 Nov 2015 11:49:50 -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 32427 invoked by uid 89); 13 Nov 2015 11:49:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 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) (207.82.80.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 13 Nov 2015 11:49:48 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-36-t0B4Ov5bR0yXdrIHfXUg_g-1; Fri, 13 Nov 2015 11:49:42 +0000 Received: from [10.2.206.200] ([10.1.2.79]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Fri, 13 Nov 2015 11:49:42 +0000 Message-ID: <5645CE55.2060302@arm.com> Date: Fri, 13 Nov 2015 11:49:00 -0000 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Christian Bruel CC: Ramana Radhakrishnan , "gcc-patches@gcc.gnu.org" Subject: Re: [PATCH 4/4] [ARM] Add attribute/pragma target fpu= References: <55F6D9FF.4030600@st.com> <55F7F75E.4070800@st.com> <55FBD3B4.9050709@arm.com> <5600096E.4030403@st.com> <56162EE8.5010209@arm.com> <5644A826.9040606@st.com> In-Reply-To: <5644A826.9040606@st.com> X-MC-Unique: t0B4Ov5bR0yXdrIHfXUg_g-1 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-11/txt/msg01674.txt.bz2 Hi Christian, On 12/11/15 14:54, Christian Bruel wrote: > Hi Kyril, > >> ... >> The parts in this patch look ok to me. >> However, I think we need some more functionality >> In aarch64 we support compiling a file with no simd, including arm_neon.= h and using arm_neon.h intrinsics >> within functions tagged with simd support. >> We want to support such functionality on arm i.e. compile a file with -m= fpu=3Dvfp and use arm_neon.h intrinsics >> in a function tagged with an fpu=3Dneon attribute. >> For that we'd need to wrap the intrinsics in arm_neon.h in appropriate p= ragmas, like in the aarch64 version of arm_neon.h > > As discussed, here is arm_neon.h for aarch32/neon with the same programmi= ng model than aarch64/simd. As you said lets use one of the fpu=3Dneon attr= ibutes even if the file is compiled with -mfpu=3Dvfp. > > The drawback for this is that now we unconditionally makes available ever= y neon intrinsics, introducing a small legacy change with regards to error = checking (that you didn't have with aarch64). Then it's worth to stress tha= t: > > - One cannot check #include "arm_neon.h" to check if the compiler can us= e neon instruction. Instead use #ifndef __ARM_NEON__. (Found in target-supp= orts.exp) Checking the macro is the 'canonical' way to check for NEON support, so I reckon we can live with that. > > > - Types cannot be checked. For instance: > > #include > > poly128_t > foo (poly128_t* ptr) > { > return vldrq_p128 (ptr); > } > > compiled with -mfpu=3Dneon used to be rejected with > > error: unknown type name 'poly128_t' ... > > Now the error, as a side effect from the inlining rules between incompat= ible modes, becomes > > error: inlining failed in call to always_inline 'vldrq_p128': target sp= ecific option mismatch ... Well, the previous message is misleading anyway since the user error there = is not a type issue but failure to specify the correct -mfpu option. > > I found this more confusing, so I was a little bit reluctant to implement= this, but the code is correctly rejected and the message makes sense, afte= r all. Just a different check. > > This patch applies on top of the preceding attribute/pragma target fpu=3D= series. Tested with arm-none-eabi configured with default and --with-cpu= =3Dcortex-a9 --with-fp --with-float=3Dhard Do you mean --with-fpu=3D? > > Also fixes a few macro that depends on fpu=3D, that I forgot to redefine. Can you please split those changes into a separate patch and ChangeLog and = commit the separately? That part is preapproved. This patch is ok then with above comment about splitting the arm-c.c change= s separately. Thanks for doing this! I believe all patches in this series are approved then so you can go ahead and start committing. Kyrill > > Christian >