On Wed, 18 Aug 2021 at 20:10, Martin Sebor <mse...@gmail.com> wrote:
>
> On 8/18/21 12:52 AM, Prathamesh Kulkarni wrote:
> > On Fri, 13 Aug 2021 at 22:44, Martin Sebor <mse...@gmail.com> wrote:
> >>
> >> On 8/12/21 2:32 AM, Prathamesh Kulkarni wrote:
> >>> On Sat, 7 Aug 2021 at 02:09, Martin Sebor <mse...@gmail.com> 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
> >>>>>>>>> <richard.guent...@gmail.com> wrote:
> >>>>>>>>>>
> >>>>>>>>>> On Mon, Jul 26, 2021 at 11:06 AM Prathamesh Kulkarni via Gcc
> >>>>>>>>>> <gcc@gcc.gnu.org> wrote:
> >>>>>>>>>>>
> >>>>>>>>>>> On Fri, 23 Jul 2021 at 23:29, Andrew Pinski <pins...@gmail.com>
> >>>>>>>>>>> wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>> On Fri, Jul 23, 2021 at 3:55 AM Prathamesh Kulkarni via Gcc
> >>>>>>>>>>>> <gcc@gcc.gnu.org> 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_neon.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/max
> >>>>>>>>>>>> which
> >>>>>>>>>>>> is generic and such; add an argument to say the intrinsics name
> >>>>>>>>>>>> even.
> >>>>>>>>>>>> You could do this as a non-target builtin if you want and reuse 
> >>>>>>>>>>>> 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 how 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 across
> >>>>>>>>>>> optimization levels:
> >>>>>>>>>>> For eg:
> >>>>>>>>>>> int32x2_t f(int32_t *restrict a)
> >>>>>>>>>>> {
> >>>>>>>>>>>      int32x2_t v = vld1_s32 (a);
> >>>>>>>>>>>      int b = 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 expansion,
> >>>>>>>>>>> expand_builtin_args is happy. But at -O0, it results in the error 
> >>>>>>>>>>> -
> >>>>>>>>>>> "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++ doesn'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 
> >>>>>>>>> attribute.
> >>>>>>>>> As implemented, the attribute takes at least one argument(s), which
> >>>>>>>>> refer to parameter position,
> >>>>>>>>> and the corresponding parameter must be const qualified, failing
> >>>>>>>>> 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 we
> >>>>>>> catch this while trying to expand the intrinsic, but that can lead to
> >>>>>>> poor diagnostics because we really want to report against the line of
> >>>>>>> code calling the intrinsic.
> >>>>>>
> >>>>>> Presumably the intrinsics can accept (or can be made to accept) any
> >>>>>> constant integer expressions, not just literals.  E.g., the aarch64
> >>>>>> builtin below accepts them.  For example, this is accepted in C++:
> >>>>>>
> >>>>>>      __Int64x2_t void f (__Int32x2_t a)
> >>>>>>      {
> >>>>>>        constexpr int n = 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 avoid
> >>>>> idiomatic const-exprs like those you would write in C++, or even those
> >>>>> you'd write naturally in C:
> >>>>>
> >>>>> #define foo (1u << 5)
> >>>>>
> >>>>>
> >>>>>> But my comment above was to highlight that if requiring the function
> >>>>>> argument referenced by the proposed consteval attribute to be const
> >>>>>> is necessary to prevent it from being modified then the requirement
> >>>>>> 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 don'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 without
> >>>>>> enforcing the requirement on the definition.
> >>>>>
> >>>>> I'm not totally sure I understand what you're saying.  But the point 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 point
> >>>>> in the source where that is done, not at the point in the (presumably
> >>>>> inlined) function where the value ends up being used.  Most of our
> >>>>> builtins are wrapped into slightly more user-friendly function names so
> >>>>> that they conform to the ACLE specification, yet ultimately map onto
> >>>>> 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 = vshr_n_s8 (x, y);  // Error y must be a const int 
> >>>>> expr.
> >>>>>      ...
> >>>>> }
> >>>>>
> >>>>> I want the error to be reported on the line in f() where the problem
> >>>>> really lies, not on the line in the inline function where the problem
> >>>>> 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 const
> >>>>      qualified, failing which, the attribute is ignored."
> >>>>
> >>>> doesn't match the POC patch because it silently accepts the following
> >>>> 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 should
> >>>> 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 = vld1_s32 (a);
> >    int b = 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 ‘vshl_n_s32’,
> >      inlined from ‘f’ 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 consteval
> >>       return 0;
> >>     }
> >>
> >>     constexpr int x = f (1);   // valid with consteval (not without)
> >>     int y = f (y);             // not valid
> > I guess that would be a more useful extension, but I was merely suggesting,
> > to add constexpr to C for computing more complex ICE's.
> > For eg:
> > int a = 1;
> > int b = a + 1;
> >
> > switch() { case b: ... } // error
> >
> > With constexpr:
> > constexpr int a  = 1;
> > constexpr int b = 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 = 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 whole
> >>>>>>>> function, not just a subset of its arguments.  That way arguments
> >>>>>>>> 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 argument
> >>>>>>>>> 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, but
> >>>>>>>>> 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) but 
> >>>>>>>>> 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 ‘g’:
> >>>>>>>>> test.c:7:9: error: argument 2 is not a constant.
> >>>>>>>>>        7 |   f (0, p);
> >>>>>>>>>          |         ^
> >>>>>>>>> test.c:2:6: note: Function ‘f’ 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<type> intrinsics require
> >>>>>>>>>>>>> that the
> >>>>>>>>>>>>> second arg (__b),
> >>>>>>>>>>>>> should be an immediate value.
> >>>>>>>>>>>>> Currently, this check is performed by arm_expand_builtin_args,
> >>>>>>>>>>>>> and if
> >>>>>>>>>>>>> a non-constant
> >>>>>>>>>>>>> value gets passed, it emits the following diagnostic:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> ../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);
> >>>>>>>>>>>>>          |          
> >>>>>>>>>>>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> 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
> >>>>>>>>
> >>>>>>
> >>>>
> >>
>

Reply via email to