Re: [PATCH 1/2] Add a parameter for the builtin function of prefetch to align with LLVM

[Segher Boessenkool <segher@kernel.crashing.org>]

On Mon, Oct 24, 2022 at 11:00:26AM +0100, Richard Sandiford wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
> > On Thu, Oct 20, 2022 at 07:34:13AM +0000, Jiang, Haochen wrote:
> >> > > +  /* Argument 3 must be either zero or one.  */
> >> > > +  if (INTVAL (op3) != 0 && INTVAL (op3) != 1)
> >> > > +    {
> >> > > +      warning (0, "invalid fourth argument to %<__builtin_prefetch%>;"
> >> > > +	" using one");
> >> > 
> >> > "using 1" makes sense maybe, but "using one" reads as "using an
> >> > argument", not very sane.
> >> > 
> >> > An error would be better here anyway?
> >> 
> >> Will change to 1 to avoid confusion in that. The reason why this is a warning
> >> is because previous ones related to constant arguments out of range in prefetch
> >> are also using warning.
> >
> > Please don't repeat historical mistakes.  You might not want to fix the
> > existing code (since that can in theory break existing user code), but
> > that is not a reason to punish users of a new feature as well ;-)
> 
> I agree an error would be appropriate for something like
> __builtin_clear_cache.  But __builtin_prefetch is a hint only.
> Nothing should break if the compiler simply evaluates the arguments
> and does nothing else.
> 
> Using a warning in that situation means that, if the ranges of
> parameters are increased in future, older compilers won't needlessly
> reject new code.

It means that if we want "2" to have a new meaning in the future, we can
not do that, since it will use the semantics of "1" on older compilers
(and that might well not be compatible).

And for what?  Is it ever so convenient for people to write random
numbers here?


Segher