Segher Boessenkool <seg...@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.
So personally I think we should stick with the current choice
of a default-on warning.
Thanks,
Richard
