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.
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
