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