On Tue, 27 Jul 2021 at 13:49, Richard Biener <richard.guenther@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 <pinskia@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.

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