From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 6B3823857832 for ; Wed, 4 Aug 2021 13:00:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6B3823857832 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D9A9112FC; Wed, 4 Aug 2021 06:00:43 -0700 (PDT) Received: from [192.168.1.19] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3F9713F719; Wed, 4 Aug 2021 06:00:43 -0700 (PDT) Subject: Re: [RFC] Adding a new attribute to function param to mark it as constant To: Segher Boessenkool , Prathamesh Kulkarni Cc: GCC Development References: <20210803215538.GU1583@gate.crashing.org> <20210804101722.GD1583@gate.crashing.org> <20210804124619.GF1583@gate.crashing.org> From: Richard Earnshaw Message-ID: <07900439-bc29-1f24-2522-4d2d79a86d4f@foss.arm.com> Date: Wed, 4 Aug 2021 14:00:42 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20210804124619.GF1583@gate.crashing.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3491.7 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, NICE_REPLY_A, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 Aug 2021 13:00:48 -0000 On 04/08/2021 13:46, Segher Boessenkool wrote: > On Wed, Aug 04, 2021 at 05:20:58PM +0530, Prathamesh Kulkarni wrote: >> On Wed, 4 Aug 2021 at 15:49, Segher Boessenkool >> wrote: >>> Both __builtin_constant_p and __is_constexpr will not work in your use >>> case (since a function argument is not a constant, let alone an ICE). >>> It only becomes a constant value later on. The manual (for the former) >>> says: >>> You may use this built-in function in either a macro or an inline >>> function. However, if you use it in an inlined function and pass an >>> argument of the function as the argument to the built-in, GCC never >>> returns 1 when you call the inline function with a string constant or >>> compound literal (see Compound Literals) and does not return 1 when you >>> pass a constant numeric value to the inline function unless you specify >>> the -O option. >> Indeed, that's why I was thinking if we should use an attribute to mark param as >> a constant, so during type-checking the function call, the compiler >> can emit a diagnostic if the passed arg >> is not a constant. > > That will depend on the vagaries of what optimisations the compiler > managed to do :-( > >> Alternatively -- as you suggest, we could define a new builtin, say >> __builtin_ice(x) that returns true if 'x' is an ICE. > > (That is a terrible name, it's not clear at all to the reader, just > write it out? It is fun if you know what it means, but infuriating > otherwise.) > >> And wrap the intrinsic inside a macro that would check if the arg is an ICE ? > > That will work yeah. Maybe not as elegant as you'd like, but not all > that bad, and it *works*. Well, hopefully it does :-) > >> For eg: >> >> __extension__ extern __inline int32x2_t >> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__)) >> vshl_n_s32_1 (int32x2_t __a, const int __b) >> { >> return __builtin_neon_vshl_nv2si (__a, __b); >> } >> >> #define vshl_n_s32(__a, __b) \ >> ({ typeof (__a) a = (__a); \ >> _Static_assert (__builtin_constant_p ((__b)), #__b " is not an >> integer constant"); \ >> vshl_n_s32_1 (a, (__b)); }) >> >> void f(int32x2_t x, const int y) >> { >> vshl_n_s32 (x, 2); >> vshl_n_s32 (x, y); >> >> int z = 1; >> vshl_n_s32 (x, z); >> } >> >> With this, the compiler rejects vshl_n_s32 (x, y) and vshl_n_s32 (x, >> z) at all optimization levels since neither 'y' nor 'z' is an ICE. > > You used __builtin_constant_p though, which works differently, so the > test is not conclusive, might not show what you want to show. > >> Instead of __builtin_constant_p, we could use __builtin_ice. >> Would that be a reasonable approach ? > > I think it will work, yes. > >> But this changes the semantics of intrinsic from being an inline >> function to a macro, and I am not sure if that's a good idea. > > Well, what happens if you call the actual builtin directly, with some > non-constant parameter? That just fails with a more cryptic error, > right? So you can view this as some syntactic sugar to make these > intrinsics easier to use. > > Hrm I now remember a place I could have used this: > > #define mtspr(n, x) do { asm("mtspr %1,%0" : : "r"(x), "n"(n)); } while (0) > #define mfspr(n) ({ \ > u32 x; asm volatile("mfspr %0,%1" : "=r"(x) : "n"(n)); x; \ > }) > > It is quite similar to your builtin code really, and I did resort to > macros there, for similar reasons :-) > > > Segher > We don't want to have to resort to macros. Not least because at some point we want to replace the content of arm_neon.h with a single #pragma directive to remove all the parsing of the header that's needed. What's more, if we had a suitable pragma we'd stand a fighting chance of being able to extend support to other languages as well that don't use the pre-processor, such as Fortran or Ada (not that that is on the cards right now). R.