From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31941 invoked by alias); 10 Dec 2015 09:26:45 -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 31931 invoked by uid 89); 10 Dec 2015 09:26:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 X-HELO: mx07-00178001.pphosted.com Received: from mx07-00178001.pphosted.com (HELO mx07-00178001.pphosted.com) (62.209.51.94) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Thu, 10 Dec 2015 09:26:43 +0000 Received: from pps.filterd (m0046668.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.14.5/8.14.5) with SMTP id tBA9PSrO027721; Thu, 10 Dec 2015 10:26:26 +0100 Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35]) by mx07-00178001.pphosted.com with ESMTP id 1ypwfatf8r-1 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT); Thu, 10 Dec 2015 10:26:26 +0100 Received: from zeta.dmz-eu.st.com (zeta.dmz-eu.st.com [164.129.230.9]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id E6FFE3F; Thu, 10 Dec 2015 09:25:48 +0000 (GMT) Received: from Webmail-eu.st.com (safex1hubcas5.st.com [10.75.90.71]) by zeta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 4DB5055C0; Thu, 10 Dec 2015 09:26:24 +0000 (GMT) Received: from [164.129.122.197] (164.129.122.197) by webmail-eu.st.com (10.75.90.13) with Microsoft SMTP Server (TLS) id 8.3.389.2; Thu, 10 Dec 2015 10:26:23 +0100 Subject: Re: [PATCH, ARM] PR68674 Fix LTO support for neon builtins and error catching To: Kyrill Tkachov , "ramana.radhakrishnan@foss.arm.com" , "gcc-patches@gcc.gnu.org" References: <5666D2BC.5030105@st.com> <566865A6.4020307@arm.com> From: Christian Bruel X-No-Archive: yes Message-ID: <5669453F.5010100@st.com> Date: Thu, 10 Dec 2015 09:26:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <566865A6.4020307@arm.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:5.15.21,1.0.33,0.0.0000 definitions=2015-12-10_05:2015-12-08,2015-12-10,1970-01-01 signatures=0 X-IsSubscribed: yes X-SW-Source: 2015-12/txt/msg01090.txt.bz2 Hi Kyrill, On 12/09/2015 06:32 PM, Kyrill Tkachov wrote: > Hi Christian, > > On 08/12/15 12:53, Christian Bruel wrote: >> Hi, >> >> The order of the NEON builtins construction has led to complications since the attribute target support. This was not a problem when driven from the command line, but was causing various issues when the builtins was mixed between fpu >> configurations or when used with LTO. >> >> Firstly the builtin functions was not initialized before the parsing of functions, leading to wrong type initializations. >> >> Then error catching code when a builtin was used without the proper fpu flags was incomprehensible for the user, for instance >> >> #include "arm_neon.h" >> >> int8x8_t a, b; >> int16x8_t e; >> >> void >> main() >> { >> e = (int16x8_t)__builtin_neon_vaddlsv8qi (a, b); >> } >> >> compiled with default options (without -mfpu=neon -mfloat-abi=hard) gave pages of >> >> /arm-none-eabi/6.0.0/include/arm_neon.h:39:9: error: unknown type name '__simd64_int8_t' >> typedef __simd64_int8_t int8x8_t; >> ... >> ... >> arm_neon.h:4724:3: error: can't convert a vector of type 'poly64x2_t {aka __vector(4) int}' to type 'int' which has different size >> return (poly64x2_t)__builtin_neon_vsli_nv2di ((int64x2_t) __a, (int64x2_t) __b, __c); >> ^~~~~~ >> ... >> ... and one for each arm_neon.h lines.. >> >> by postponing the check into arm_expand_builtin, we now emit something more useful: >> >> testo.c: In function 'main': >> testo.c:9:7: error: '__builtin_neon_vaddlsv8qi' neon builtin is not supported in this configuration. >> e = (int16x8_t)__builtin_neon_vaddlsv8qi (a, b); >> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> >> One small side effect to note: The total memory allocated is 370k bigger when neon is not used, so this support will have a follow-up to make their initialization lazy. But I'd like first to stabilize the stuff for stage3 (or get it >> pre-approved if the memory is an issue) >> >> tested without new failures with {,-mfpu=vfp,-mfpu=neon}{,-march=armv7-a\} >> (a few tests that was fail are now unsupported) >> > > I agree, the vector types (re)initialisation is a tricky part. > I've seen similar issues in the aarch64 work for target attributes > > bool > arm_vector_mode_supported_p (machine_mode mode) > { > - /* Neon also supports V2SImode, etc. listed in the clause below. */ > - if (TARGET_NEON && (mode == V2SFmode || mode == V4SImode || mode == V8HImode > + if (mode == V2SFmode || mode == V4SImode || mode == V8HImode > || mode == V4HFmode || mode == V16QImode || mode == V4SFmode > - || mode == V2DImode || mode == V8HFmode)) > - return true; > - > - if ((TARGET_NEON || TARGET_IWMMXT) > - && ((mode == V2SImode) > - || (mode == V4HImode) > - || (mode == V8QImode))) > + || mode == V2DImode || mode == V8HFmode > + || mode == V2SImode || mode == V4HImode || mode == V8QImode) > return true; > > > So this allows vector modes unconditionally for all targets/fpu configurations? > I was tempted to do that in aarch64 when I was encountering similar issues. > In the end what worked for me was re-laying out the vector types in SET_CURRENT_FUNCTION > if necessary (https://gcc.gnu.org/ml/gcc-patches/2015-08/msg01084.html) yes my assumption was that arm_init_neon_builtins () is now called for all targets, since the check is done at expand time and that the builtins need to be known by lto, with the vector type initialization, before they are expanded. However at that time, lto streaming-in have not yet processed the attributes and TARGET_NEON is not set for the function. I had a look at your re-layout, but I'm not sure. it feels like a hack. I think this should be solved first place during the builtin construction. Also set_current_function is too late, builtin_expand that will explode because of the unknown modes. But raise the point. In fact I was not really happy with this arm_vector_mode_supported_p neither as I was not sure about other contexts it can be called from and I cannot clearly claim that this change is always correct. I'd like to think about other way to set the vector modes from arm_init_neon_builtins before the target flags are known. I'm thinking about the lazy initialization at expand time, or using a contextual boolean flags. how does that sound ? many thanks, Christian > > Kyrill >