From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52b.google.com (mail-ed1-x52b.google.com [IPv6:2a00:1450:4864:20::52b]) by sourceware.org (Postfix) with ESMTPS id A79FE3858437 for ; Thu, 19 Aug 2021 08:10:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A79FE3858437 Received: by mail-ed1-x52b.google.com with SMTP id cn28so7518313edb.6 for ; Thu, 19 Aug 2021 01:10:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=TZsRPQ29qJobqdnndDlxso/oDvuPGLEKSrvBVacZvpI=; b=LIyEQHil5rVAuLWXNrmtXRXybb1E6GlDIaBasuaP3XOTNmcTEpBfpyQzSYCA+PLwse SckrDAaQL5ETHh2Np6LP6wj+WKxTbJNMECugnMWhHEcuXJMlQsLjTI8XHY9J1yhxPP32 kg6qAy78nPilAwJKPJ0J59gcP0OMwpKMf4cG51/MIeGODEOf7fCoowWIa9oPQoJtt3Vu GCHrP0n1X0eyyhlTaxcdPOiT4mXlBUTqdLuRBMRbefbP7B13FJjNx4DUQcXCheVMzLxk Lc8DTAPKq43fAFe2dewtbVCCNK1V0viidHKOpe5OogQMk69FJpYzGCk4F25iiwpkZ1LD cNQQ== X-Gm-Message-State: AOAM530xYSdNChpbVIYYO6PVAIVGzEmXzfXk5q1CngGb79MH3caRW0Q6 bVqmCcpvevp8NbcEkReJ5oDM3jOFak4w7QOsXmNxSw== X-Google-Smtp-Source: ABdhPJwddfSSkwCmiM/9YG3vTX4fpFDHpsEukG5yajny2VFCpi6yk5lCGrZN7i8N71ow6PAigLFXwKbSbpnXeZRbJ/k= X-Received: by 2002:aa7:de14:: with SMTP id h20mr14970277edv.43.1629360645044; Thu, 19 Aug 2021 01:10:45 -0700 (PDT) MIME-Version: 1.0 References: <9587be75-ec2a-a997-3074-1811228245d3@gmail.com> <0cf362fc-328f-5256-d289-e6b6fda838cd@gmail.com> <62f7087a-8adf-0a83-1fdf-d24ac49d04d2@foss.arm.com> <034b8502-73ba-205b-b267-236688df2709@gmail.com> <8ae370ba-3730-c28c-1378-4645e74ee2bc@gmail.com> In-Reply-To: <8ae370ba-3730-c28c-1378-4645e74ee2bc@gmail.com> From: Prathamesh Kulkarni Date: Thu, 19 Aug 2021 13:40:07 +0530 Message-ID: Subject: Re: [RFC] Adding a new attribute to function param to mark it as constant To: Martin Sebor Cc: Richard Earnshaw , Richard Biener , GCC Development Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_ASCII_DIVIDERS, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, 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: Thu, 19 Aug 2021 08:10:57 -0000 On Wed, 18 Aug 2021 at 20:10, Martin Sebor wrote: > > On 8/18/21 12:52 AM, Prathamesh Kulkarni wrote: > > On Fri, 13 Aug 2021 at 22:44, Martin Sebor wrote: > >> > >> On 8/12/21 2:32 AM, Prathamesh Kulkarni wrote: > >>> On Sat, 7 Aug 2021 at 02:09, Martin Sebor wrote: > >>>> > >>>> On 8/6/21 4:51 AM, Richard Earnshaw wrote: > >>>>> > >>>>> > >>>>> On 06/08/2021 01:06, Martin Sebor via Gcc wrote: > >>>>>> On 8/4/21 3:46 AM, Richard Earnshaw wrote: > >>>>>>> > >>>>>>> > >>>>>>> On 03/08/2021 18:44, Martin Sebor wrote: > >>>>>>>> On 8/3/21 4:11 AM, Prathamesh Kulkarni via Gcc wrote: > >>>>>>>>> On Tue, 27 Jul 2021 at 13:49, Richard Biener > >>>>>>>>> wrote: > >>>>>>>>>> > >>>>>>>>>> On Mon, Jul 26, 2021 at 11:06 AM Prathamesh Kulkarni via Gcc > >>>>>>>>>> wrote: > >>>>>>>>>>> > >>>>>>>>>>> On Fri, 23 Jul 2021 at 23:29, Andrew Pinski > >>>>>>>>>>> wrote: > >>>>>>>>>>>> > >>>>>>>>>>>> On Fri, Jul 23, 2021 at 3:55 AM Prathamesh Kulkarni via Gcc > >>>>>>>>>>>> wrote: > >>>>>>>>>>>>> > >>>>>>>>>>>>> Hi, > >>>>>>>>>>>>> Continuing from this thread, > >>>>>>>>>>>>> https://gcc.gnu.org/pipermail/gcc-patches/2021-July/575920.= html > >>>>>>>>>>>>> The proposal is to provide a mechanism to mark a parameter = in a > >>>>>>>>>>>>> function as a literal constant. > >>>>>>>>>>>>> > >>>>>>>>>>>>> Motivation: > >>>>>>>>>>>>> Consider the following intrinsic vshl_n_s32 from arrm/arm_n= eon.h: > >>>>>>>>>>>>> > >>>>>>>>>>>>> __extension__ extern __inline int32x2_t > >>>>>>>>>>>>> __attribute__ ((__always_inline__, __gnu_inline__, > >>>>>>>>>>>>> __artificial__)) > >>>>>>>>>>>>> vshl_n_s32 (int32x2_t __a, const int __b) > >>>>>>>>>>>>> { > >>>>>>>>>>>>> return (int32x2_t)__builtin_neon_vshl_nv2si (__a, __b)= ; > >>>>>>>>>>>>> } > >>>>>>>>>>>>> > >>>>>>>>>>>>> and it's caller: > >>>>>>>>>>>>> > >>>>>>>>>>>>> int32x2_t f (int32x2_t x) > >>>>>>>>>>>>> { > >>>>>>>>>>>>> return vshl_n_s32 (x, 1); > >>>>>>>>>>>>> } > >>>>>>>>>>>> > >>>>>>>>>>>> Can't you do similar to what is done already in the aarch64 > >>>>>>>>>>>> back-end: > >>>>>>>>>>>> #define __AARCH64_NUM_LANES(__v) (sizeof (__v) / sizeof (__v= [0])) > >>>>>>>>>>>> #define __AARCH64_LANE_CHECK(__vec, __idx) \ > >>>>>>>>>>>> __builtin_aarch64_im_lane_boundsi (sizeof(__vec), > >>>>>>>>>>>> sizeof(__vec[0]), __idx) > >>>>>>>>>>>> > >>>>>>>>>>>> ? > >>>>>>>>>>>> Yes this is about lanes but you could even add one for min/m= ax > >>>>>>>>>>>> which > >>>>>>>>>>>> is generic and such; add an argument to say the intrinsics n= ame > >>>>>>>>>>>> even. > >>>>>>>>>>>> You could do this as a non-target builtin if you want and re= use it > >>>>>>>>>>>> also for the aarch64 backend. > >>>>>>>>>>> Hi Andrew, > >>>>>>>>>>> Thanks for the suggestions. IIUC, we could use this approach = to > >>>>>>>>>>> check > >>>>>>>>>>> if the argument > >>>>>>>>>>> falls within a certain range (min / max), but I am not sure h= ow it > >>>>>>>>>>> will help to determine > >>>>>>>>>>> if the arg is a constant immediate ? AFAIK, vshl_n intrinsics > >>>>>>>>>>> require > >>>>>>>>>>> that the 2nd arg is immediate ? > >>>>>>>>>>> > >>>>>>>>>>> Even the current RTL builtin checking is not consistent acros= s > >>>>>>>>>>> optimization levels: > >>>>>>>>>>> For eg: > >>>>>>>>>>> int32x2_t f(int32_t *restrict a) > >>>>>>>>>>> { > >>>>>>>>>>> int32x2_t v =3D vld1_s32 (a); > >>>>>>>>>>> int b =3D 2; > >>>>>>>>>>> return vshl_n_s32 (v, b); > >>>>>>>>>>> } > >>>>>>>>>>> > >>>>>>>>>>> With pristine trunk, compiling with -O2 results in no errors = because > >>>>>>>>>>> constant propagation replaces 'b' with 2, and during expansio= n, > >>>>>>>>>>> expand_builtin_args is happy. But at -O0, it results in the e= rror - > >>>>>>>>>>> "argument 2 must be a constant immediate". > >>>>>>>>>>> > >>>>>>>>>>> So I guess we need some mechanism to mark a parameter as a > >>>>>>>>>>> constant ? > >>>>>>>>>> > >>>>>>>>>> I guess you want to mark it in a way that the frontend should = force > >>>>>>>>>> constant evaluation and error if that's not possible? C++ do= esn't > >>>>>>>>>> allow to declare a parameter as 'constexpr' but something like > >>>>>>>>>> > >>>>>>>>>> void foo (consteval int i); > >>>>>>>>>> > >>>>>>>>>> since I guess you do want to allow passing constexpr arguments > >>>>>>>>>> in C++ or in C extended forms of constants like > >>>>>>>>>> > >>>>>>>>>> static const int a[4]; > >>>>>>>>>> > >>>>>>>>>> foo (a[1]); > >>>>>>>>>> > >>>>>>>>>> ? But yes, this looks useful to me. > >>>>>>>>> Hi Richard, > >>>>>>>>> Thanks for the suggestions and sorry for late response. > >>>>>>>>> I have attached a prototype patch that implements consteval att= ribute. > >>>>>>>>> As implemented, the attribute takes at least one argument(s), w= hich > >>>>>>>>> refer to parameter position, > >>>>>>>>> and the corresponding parameter must be const qualified, failin= g > >>>>>>>>> which, the attribute is ignored. > >>>>>>>> > >>>>>>>> I'm curious why the argument must be const-qualified. If it's > >>>>>>>> to keep it from being changed in ways that would prevent it from > >>>>>>>> being evaluated at compile-time in the body of the function then > >>>>>>>> to be effective, the enforcement of the constraint should be on > >>>>>>>> the definition of the function. Otherwise, the const qualifier > >>>>>>>> could be used in a declaration of a function but left out from > >>>>>>>> a subsequent definition of it, letting it modify it, like so: > >>>>>>>> > >>>>>>>> __attribute__ ((consteval (1))) void f (const int); > >>>>>>>> > >>>>>>>> inline __attribute__ ((always_inline)) void f (int i) { ++i= ; } > >>>>>>> > >>>>>>> In this particular case it's because the inline function is > >>>>>>> implementing an intrinsic operation in the architecture and the > >>>>>>> instruction only supports a literal constant value. At present w= e > >>>>>>> catch this while trying to expand the intrinsic, but that can lea= d to > >>>>>>> poor diagnostics because we really want to report against the lin= e of > >>>>>>> code calling the intrinsic. > >>>>>> > >>>>>> Presumably the intrinsics can accept (or can be made to accept) an= y > >>>>>> constant integer expressions, not just literals. E.g., the aarch6= 4 > >>>>>> builtin below accepts them. For example, this is accepted in C++: > >>>>>> > >>>>>> __Int64x2_t void f (__Int32x2_t a) > >>>>>> { > >>>>>> constexpr int n =3D 2; > >>>>>> return __builtin_aarch64_vshll_nv2si (a, n + 1); > >>>>>> } > >>>>>> > >>>>>> Making the intrinscis accept constant arguments in constexpr-like > >>>>>> functions and introducing a constexpr-lite attribute (for C code) > >>>>>> was what I was suggesting bythe constexpr comment below. I'd find > >>>>>> that a much more general and more powerful design. > >>>>>> > >>>>> > >>>>> Yes, it would be unfortunate if the rule made it impossible to avoi= d > >>>>> idiomatic const-exprs like those you would write in C++, or even th= ose > >>>>> you'd write naturally in C: > >>>>> > >>>>> #define foo (1u << 5) > >>>>> > >>>>> > >>>>>> But my comment above was to highlight that if requiring the functi= on > >>>>>> argument referenced by the proposed consteval attribute to be cons= t > >>>>>> is necessary to prevent it from being modified then the requiremen= t > >>>>>> needs to be enforced not on the declaration but on the definition. > >>>>>> > >>>>>> You may rightly say: "but we get to define the inline arm function > >>>>>> wrappers so we'll make sure to never declare them that way." I do= n't > >>>>>> have a problem with that. What I am saying is that a consteval > >>>>>> attribute that required the function argument to be declared const > >>>>>> to be effective would be flawed as a general-purpose feature witho= ut > >>>>>> enforcing the requirement on the definition. > >>>>> > >>>>> I'm not totally sure I understand what you're saying. But the poin= t of > >>>>> putting the attribute on the declaration is to allow for reporting > >>>>> errors at the point of invocation, so if I call a function with an > >>>>> invalid 'must-be-const-expr' value, I'll get a diagnostic at the po= int > >>>>> in the source where that is done, not at the point in the (presumab= ly > >>>>> inlined) function where the value ends up being used. Most of our > >>>>> builtins are wrapped into slightly more user-friendly function name= s so > >>>>> that they conform to the ACLE specification, yet ultimately map ont= o > >>>>> names into __builtin namespace per gcc's internal standards. If I = have: > >>>>> > >>>>> __extension__ extern __inline int8x8_t > >>>>> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__)= ) > >>>>> vshr_n_s8 (int8x8_t __a, const int __b) > >>>>> { > >>>>> return (int8x8_t)__builtin_neon_vshrs_nv8qi (__a, __b); > >>>>> } > >>>>> > >>>>> > >>>>> and then use that with > >>>>> > >>>>> extern int8x8_t x; > >>>>> extern int y; > >>>>> > >>>>> f() { > >>>>> int8x8_t z =3D vshr_n_s8 (x, y); // Error y must be a const i= nt expr. > >>>>> ... > >>>>> } > >>>>> > >>>>> I want the error to be reported on the line in f() where the proble= m > >>>>> really lies, not on the line in the inline function where the probl= em > >>>>> eventually manifests itself. > >>>> > >>>> Yes, I understand that. What I'm pointing out is that > >>>> the description of the attribute above > >>>> > >>>> "the attribute takes at least one argument(s), which refer to > >>>> parameter position, and the corresponding parameter must be con= st > >>>> qualified, failing which, the attribute is ignored." > >>>> > >>>> doesn't match the POC patch because it silently accepts the followin= g > >>>> two declarations: > >>>> > >>>> int __attribute__ ((consteval (1))) > >>>> f (const int); // attribute consteval accepted... > >>>> > >>>> int f (int a) // ...even though the argument is not const > >>>> { > >>>> return ++a; > >>>> } > >>>> > >>>> The attribute handler should check that the const qualifier on > >>>> the argument is in the definition of the function. > >>>> > >>>> I'm also pointing out that from the POV of the user of the attribute= , > >>>> the const shouldn't be necessary: the attribute consteval alone shou= ld > >>>> be sufficient. Hence my original question: why must the argument be > >>>> const-qualified? (Why can't you print equally good error messages > >>>> without it?) > >>> Hi Martin, > >>> While writing the POC patch, my expectation was to treat consteval > >>> param as a more "restricted version" > >>> of const param -- in addition to being readonly, it should only be > >>> passed compile-time constant > >>> values, and so I thought it probably made sense to require it to be > >>> const qualified. > >>> But if that doesn't seem like a good idea, we can drop it to be const= qualified. > >> > >> I don't think depending on the const qualifier is necessary or > >> a good idea. > >> > >>> > >>> I was wondering how do we take this forward ? > >>> For a start would this be OK ? > >>> 1] Since we also need range info, define consteval attribute to take = 3 > >>> arguments -- param-pos, min-val, max-val. > >>> and requiring the parameter to be an integral type (unlike the one > >>> implemented in POC patch). > >> > >> Specifying a range for a variable of any kind (including globals > >> and members) would be useful in general, independent of this > >> enhancement. I would suggest to introduce a separate attribute > >> for that. (With the effects of assigning out-of-range values > >> being undefined and a warning issued if/when detected.) > > Hi Martin, > > Sorry for late response. > > > > For vshl_n case, we need two constraints on the argument: > > (1) It has to be a constant. > > (2) The constant value has to be within the permissible range > > corresponding to the width. > > For eg, vshl_n_s32, accepts a constant value only between [0, 31] range= . > > > > I agree that the range-info can be more generally useful beyond > > argument checking. > > So, do you think we should add two attributes: > > (1) consteval(param-pos), which just tells the compiler that the > > corresponding parameter > > expects an ICE (similar to POC patch). > > > > (2) range-info attribute for integral types similar to mode. > > For eg: > > typedef int __attribute__((range_info(0, 31))) S32; > > S32 x; // x is int with [0, 31] range > > > > So, vshl_n_s32 could be declared as: > > > > __extension__ extern __inline int32x2_t > > __attribute__ ((__always_inline__, __gnu_inline__, __artificial__, > > consteval(2))) > > vshl_n_s32 (int32x2_t __a, const S32 __b) > > { > > return (int32x2_t)__builtin_neon_vshl_nv2si (__a, __b); > > } > > > > which would tell the compiler that 'b' needs to be constant within [0, > > 31] range, > > and emit an error otherwise during parsing. > > Yes, that's what I was thinking (without the const qualifier). > The range_info attribute (I'd suggest naming it value_range) > could be used on parameters as well to avoid having to > introduce typedefs: > > __attribute__ ((..., consteval(2))) > vshl_n_s32 (int32x2_t a, int b __attribute__ ((value_range (0, 31))) > { ... } > > (I think accepting the attribute on types as in your example would > be useful as well, as would be accepting multiple attributes to > represent multi-ranges). > > >> > >>> 2] Diagnose a function call, if the corresponding argument (after > >>> invoking c_fully_fold) is not INTEGER_CST. > >>> > >>> My intent is to currently accept ICE's that are also acceptable in > >>> other language features like case statement. > >> > >> So something like this: > >> > >> __attribute__ ((always_inline, artificial, consteval (1))) > >> inline int f (int i) > >> { > >> switch (i) case i: return i; // or a built-in that expects > >> return 0; // a constant expression argument > >> } > >> > >> int g (void) { return f (7); } // valid > >> > >> but: > >> > >> int h (int i) { return f (i); } // invalid > >> > >> and a nice error for the call to h (as opposed to f): > >> > >> In function 'h': > >> error: argument 1 to 'f' is not a constant integer > >> note: 'f' declared with attribute 'consteval (1)' > >> > >>> As you suggest above, we could add "constexpr-lite" attribute to C, > >>> for accepting more complicated integral constant expressions, > >>> that can also be used in other language features that need ICE's. > >>> > >>> My concern with this approach tho, is that currently we accept ICE's > >>> for intrinsics and rely on the optimizer to resolve them into > >>> constants > >>> before expansion. > >> > >> So calls to the intrinsics don't compile without optimization? > >> (That's not what I saw in my very limited testing but I know > >> almost nothing about the ARM builtins so I could be missing > >> something.) > > I think it's only the case for _n intrinsics that require an ICE. > > The actual checking for constant currently happens in > > expand_builtin_args, in the backend during expansion, > > so the following function: > > > > int32x2_t f(int32_t *restrict a) > > { > > int32x2_t v =3D vld1_s32 (a); > > int b =3D 2; > > return vshl_n_s32 (v, b); > > } > > > > compiles at -O1+ because ccp turns 'b' into a constant, but fails at > > -O0 with following diagnostic: > > In file included from foo.c:1: > > In function =E2=80=98vshl_n_s32=E2=80=99, > > inlined from =E2=80=98f=E2=80=99 at foo.c:9:10: > > ../armhf-build/gcc/include/arm_neon.h:4904:10: error: argument 2 must > > be a constant immediate > > 4904 | return (int32x2_t)__builtin_neon_vshl_nv2si (__a, __b); > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > I meant simply this: > > vshl_n_s32 (v, 2); > > With vshl_n_s32() an inline wrapper around the __builtin, I'd expect > the call not to compile for the reason we're discussing (i.e., it's > not an constant integer expression). > > >> > >>> So code that passes ICE to vshl_n, would get > >>> compiled at higher optimization levels but fail to compile with -O0. > >>> I am wondering if there's any existing code that relies on this > >>> behavior and will fail to compile if we impose the above restriction = ? > >> > >> I don't know. > >> > >> But as it turns out, an argument to a C++ constexpr function isn't > >> considered a constant expression within the body of the function > >> even if it is one at the call site. So the extension you describe > >> would also apply to constexpr functions: > >> > >> __attribute__ ((consteval (1))) > >> constexpr int f (int i) > >> { > >> switch (i) case i: return i; // potentially valid with constev= al > >> return 0; > >> } > >> > >> constexpr int x =3D f (1); // valid with consteval (not without) > >> int y =3D f (y); // not valid > > I guess that would be a more useful extension, but I was merely suggest= ing, > > to add constexpr to C for computing more complex ICE's. > > For eg: > > int a =3D 1; > > int b =3D a + 1; > > > > switch() { case b: ... } // error > > > > With constexpr: > > constexpr int a =3D 1; > > constexpr int b =3D a + 1; > > > > switch() { case b: ... } // OK > > I was just noting that the consteval attribute would be useful even > in C++ where a constant integer expression argument of a constexpr > function isn't considered constexpr. (Function arguments can't > be declared constexpr in C++ as far as I can tell.) > > A "constexpr" for variables only (or attribute consteval) be useful > too in C. That would suggest making it a variable attribute rather > than a positional function attribute. > > FWIW, there was a proposal for C2x along these lines some time ago: > http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2067.pdf > AFAIK, it didn't go anywhere in WG14 for a variety of reasons but > the basic idea behind it was a good one. Thanks for the suggestions. To summarize we'd need (1) consteval (let's just call it constexpr?) attribute on function params. (2) value_range attribute on variables and/or types. For extending a consteval/constexpr param to be accepted as ICE like in case statement, won't it be necessary tho that the function be always inlined ? As mentioned above, my concern with using the attribute(s) on the _n intrinsics, is that it will break code that relies on the optimizer to resolve expressions to constants like: int a =3D 1; vshl_n_s32 (x, a); @Richard Earnshaw would that be OK ? Thanks, Prathamesh Thanks, Prathamesh > > Martin > > > > > Thanks, > > Prathamesh > >> > >> Martin > >> > >>> > >>> Thanks, > >>> Prathamesh > >>> > >>> > >>>> > >>>> Martin > >>>> > >>>>> > >>>>> R. > >>>>> > >>>>>> > >>>>>> Martin > >>>>>> > >>>>>> > >>>>>>> > >>>>>>> R. > >>>>>>>> > >>>>>>>> That said, if compile-time function evaluation is the goal then > >>>>>>>> a fully general solution is an attribute that applies to the who= le > >>>>>>>> function, not just a subset of its arguments. That way argument= s > >>>>>>>> can also be assigned to local variables within the function that > >>>>>>>> can then be modified while still evaluated at compile time and > >>>>>>>> used where constant expressions are expected. I.e., the design > >>>>>>>> goal is [a subset of] C++ constexpr. (Obviously a much bigger > >>>>>>>> project.) > >>>>>>>> > >>>>>>>> A few notes on the prototype patch: conventionally GCC warnings > >>>>>>>> about attributes do not mention when an attribute is ignored. > >>>>>>>> It may be a nice touch to add to all of them but I'd recommend > >>>>>>>> against doing that in individual handlers. Since the attribute > >>>>>>>> allows pointer constants the warning issued when an argument is > >>>>>>>> not one should be generalized (i.e., not refer to just integer > >>>>>>>> constants). > >>>>>>>> > >>>>>>>> (Other than that, C/C++ warnings should start in lowercase and > >>>>>>>> not end in a period). > >>>>>>>> > >>>>>>>> Martin > >>>>>>>> > >>>>>>>>> > >>>>>>>>> The patch does type-checking for arguments in > >>>>>>>>> check_function_consteval_attr, which > >>>>>>>>> simply does a linear search to see if the corresponding argumen= t > >>>>>>>>> number is included in consteval attribute, > >>>>>>>>> and if yes, it checks if the argument is CONSTANT_CLASS_P. > >>>>>>>>> > >>>>>>>>> This works for simple constants and the intrinsics use-case, bu= t > >>>>>>>>> rejects "extended constants" as in your above example. > >>>>>>>>> I guess we can special case to detect cases like a[i] where 'a'= is > >>>>>>>>> const and 'i' is an immediate, > >>>>>>>>> but I am not sure if there's a way to force constant evaluation= in > >>>>>>>>> FE ? > >>>>>>>>> I tried using c_fully_fold (argarray[i], false, &maybe_const) b= ut that > >>>>>>>>> didn't seem to work. > >>>>>>>>> Do you have any suggestions on how to detect those in the front= -end ? > >>>>>>>>> > >>>>>>>>> Example: > >>>>>>>>> > >>>>>>>>> __attribute__((consteval(1, 2))) > >>>>>>>>> void f(const int x, int *p) > >>>>>>>>> {} > >>>>>>>>> > >>>>>>>>> Calling it with: > >>>>>>>>> 1) f(0, (int *) 0) is accepted. > >>>>>>>>> > >>>>>>>>> 2) > >>>>>>>>> void g(int *p) > >>>>>>>>> { > >>>>>>>>> f (0, p); > >>>>>>>>> } > >>>>>>>>> > >>>>>>>>> emits the following error: > >>>>>>>>> test.c: In function =E2=80=98g=E2=80=99: > >>>>>>>>> test.c:7:9: error: argument 2 is not a constant. > >>>>>>>>> 7 | f (0, p); > >>>>>>>>> | ^ > >>>>>>>>> test.c:2:6: note: Function =E2=80=98f=E2=80=99 has parameter 2 = with consteval > >>>>>>>>> attribute. > >>>>>>>>> 2 | void f(const int x, int *const p) > >>>>>>>>> | ^ > >>>>>>>>> > >>>>>>>>> Thanks, > >>>>>>>>> Prathamesh > >>>>>>>>>> > >>>>>>>>>> Richard. > >>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> Thanks, > >>>>>>>>>>> Prathamesh > >>>>>>>>>>>> > >>>>>>>>>>>> Thanks, > >>>>>>>>>>>> Andrew Pinski > >>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> The constraint here is that, vshl_n intrinsics requir= e > >>>>>>>>>>>>> that the > >>>>>>>>>>>>> second arg (__b), > >>>>>>>>>>>>> should be an immediate value. > >>>>>>>>>>>>> Currently, this check is performed by arm_expand_builtin_ar= gs, > >>>>>>>>>>>>> and if > >>>>>>>>>>>>> a non-constant > >>>>>>>>>>>>> value gets passed, it emits the following diagnostic: > >>>>>>>>>>>>> > >>>>>>>>>>>>> ../armhf-build/gcc/include/arm_neon.h:4904:10: error: argum= ent > >>>>>>>>>>>>> 2 must > >>>>>>>>>>>>> be a constant immediate > >>>>>>>>>>>>> 4904 | return (int32x2_t)__builtin_neon_vshl_nv2si (_= _a, __b); > >>>>>>>>>>>>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~= ~~~~~~~~ > >>>>>>>>>>>>> > >>>>>>>>>>>>> However, we're trying to replace builtin calls with gcc's C= vector > >>>>>>>>>>>>> extensions where > >>>>>>>>>>>>> possible (PR66791), because the builtins are opaque to the > >>>>>>>>>>>>> optimizers. > >>>>>>>>>>>>> > >>>>>>>>>>>>> Unfortunately, we lose type checking of immediate value if = we > >>>>>>>>>>>>> replace > >>>>>>>>>>>>> the builtin > >>>>>>>>>>>>> with << operator: > >>>>>>>>>>>>> > >>>>>>>>>>>>> __extension__ extern __inline int32x2_t > >>>>>>>>>>>>> __attribute__ ((__always_inline__, __gnu_inline__, > >>>>>>>>>>>>> __artificial__)) > >>>>>>>>>>>>> vshl_n_s32 (int32x2_t __a, const int __b) > >>>>>>>>>>>>> { > >>>>>>>>>>>>> return __a << __b; > >>>>>>>>>>>>> } > >>>>>>>>>>>>> > >>>>>>>>>>>>> So, I was wondering if we should have an attribute for a > >>>>>>>>>>>>> parameter to > >>>>>>>>>>>>> specifically > >>>>>>>>>>>>> mark it as a constant value with optional range value info = ? > >>>>>>>>>>>>> As Richard suggested, sth like: > >>>>>>>>>>>>> void foo(int x __attribute__((literal_constant (min_val, > >>>>>>>>>>>>> max_val))); > >>>>>>>>>>>>> > >>>>>>>>>>>>> Thanks, > >>>>>>>>>>>>> Prathamesh > >>>>>>>> > >>>>>> > >>>> > >> >