[RFC] Adding a new attribute to function param to mark it as constant

[Prathamesh Kulkarni]

On Wed, 4 Aug 2021 at 18:30, Richard Earnshaw
<Richard.Earnshaw@foss.arm.com> wrote:
>
> On 04/08/2021 13:46, Segher Boessenkool wrote:
> > On Wed, Aug 04, 2021 at 05:20:58PM +0530, Prathamesh Kulkarni wrote:
> >> On Wed, 4 Aug 2021 at 15:49, Segher Boessenkool
> >> <segher@kernel.crashing.org> wrote:
> >>> Both __builtin_constant_p and __is_constexpr will not work in your use
> >>> case (since a function argument is not a constant, let alone an ICE).
> >>> It only becomes a constant value later on.  The manual (for the former)
> >>> says:
> >>>   You may use this built-in function in either a macro or an inline
> >>>   function. However, if you use it in an inlined function and pass an
> >>>   argument of the function as the argument to the built-in, GCC never
> >>>   returns 1 when you call the inline function with a string constant or
> >>>   compound literal (see Compound Literals) and does not return 1 when you
> >>>   pass a constant numeric value to the inline function unless you specify
> >>>   the -O option.
> >> Indeed, that's why I was thinking if we should use an attribute to mark param as
> >> a constant, so during type-checking the function call, the compiler
> >> can emit a diagnostic if the passed arg
> >> is not a constant.
> >
> > That will depend on the vagaries of what optimisations the compiler
> > managed to do :-(
> >
> >> Alternatively -- as you suggest, we could define a new builtin, say
> >> __builtin_ice(x) that returns true if 'x' is an ICE.
> >
> > (That is a terrible name, it's not clear at all to the reader, just
> > write it out?  It is fun if you know what it means, but infuriating
> > otherwise.)
> >
> >> And wrap the intrinsic inside a macro that would check if the arg is an ICE ?
> >
> > That will work yeah.  Maybe not as elegant as you'd like, but not all
> > that bad, and it *works*.  Well, hopefully it does :-)
> >
> >> For eg:
> >>
> >> __extension__ extern __inline int32x2_t
> >> __attribute__  ((__always_inline__, __gnu_inline__, __artificial__))
> >> vshl_n_s32_1 (int32x2_t __a, const int __b)
> >> {
> >>   return __builtin_neon_vshl_nv2si (__a, __b);
> >> }
> >>
> >> #define vshl_n_s32(__a, __b) \
> >> ({ typeof (__a) a = (__a); \
> >>    _Static_assert (__builtin_constant_p ((__b)), #__b " is not an
> >> integer constant"); \
> >>    vshl_n_s32_1 (a, (__b)); })
> >>
> >> void f(int32x2_t x, const int y)
> >> {
> >>   vshl_n_s32 (x, 2);
> >>   vshl_n_s32 (x, y);
> >>
> >>   int z = 1;
> >>   vshl_n_s32 (x, z);
> >> }
> >>
> >> With this, the compiler rejects vshl_n_s32 (x, y) and vshl_n_s32 (x,
> >> z) at all optimization levels since neither 'y' nor 'z' is an ICE.
> >
> > You used __builtin_constant_p though, which works differently, so the
> > test is not conclusive, might not show what you want to show.
> >
> >> Instead of __builtin_constant_p, we could use __builtin_ice.
> >> Would that be a reasonable approach ?
> >
> > I think it will work, yes.
> >
> >> But this changes the semantics of intrinsic from being an inline
> >> function to a macro, and I am not sure if that's a good idea.
> >
> > Well, what happens if you call the actual builtin directly, with some
> > non-constant parameter?  That just fails with a more cryptic error,
> > right?  So you can view this as some syntactic sugar to make these
> > intrinsics easier to use.
> >
> > Hrm I now remember a place I could have used this:
> >
> > #define mtspr(n, x) do { asm("mtspr %1,%0" : : "r"(x), "n"(n)); } while (0)
> > #define mfspr(n) ({ \
> >       u32 x; asm volatile("mfspr %0,%1" : "=r"(x) : "n"(n)); x; \
> > })
> >
> > It is quite similar to your builtin code really, and I did resort to
> > macros there, for similar reasons :-)
> >
> >
> > Segher
> >
>
> We don't want to have to resort to macros.  Not least because at some
> point we want to replace the content of arm_neon.h with a single #pragma
> directive to remove all the parsing of the header that's needed.  What's
> more, if we had a suitable pragma we'd stand a fighting chance of being
> able to extend support to other languages as well that don't use the
> pre-processor, such as Fortran or Ada (not that that is on the cards
> right now).
Hi,
IIUC, a more general issue here, is that the intrinsics require
special type-checking of arguments, beyond what is dictated by the
Standard.
An argument needing to be an ICE could be seen as one instance.

So perhaps, should there be some mechanism to tell the FE to let the
target do additional checking for a particular function call, say by
explicitly marking
it with "intrinsic" attribute ? So while type checking a call to a
function marked with "intrinsic" attribute, FE can invoke target
handler with name of function and
corresponding arguments passed, and then leave it to the target for
further checking ?
For vshl_n case, the target hook would check that the 2nd arg is an
integer constant within the permissible range.

I propose to do this only for intrinsics that need special checking
and can be entirely implemented with C extensions and won't need
pattern definitions / builtins,
which I assume won't be a lot. I wonder if there'd be any apart from
ones ending with _n ?

Does the approach look reasonable ?

Thanks,
Prathamesh
>
> R.