Alfie Richards <alfie.richa...@arm.com> writes:
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index 5e305643b3a..253ea6dd77f 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -12268,6 +12268,11 @@ function version at run-time for a given set of
> function versions.
> body must be generated.
> @end deftypefn
>
> +@deftypefn {Target Hook} bool TARGET_REJECT_FUNCTION_CLONE_VERSION
> (string_slice @var{str}, location_t @var{loc})
> +This hook is used to ignore versions specified in a target_clones
Markup nit: @code{target_clones}
> +annotation. @var{str} is the version to be considered.
> +@end deftypefn
> +
Sorry to be awkward, and sorry if this conflicts with another review,
but I think this interface falls between two stools. Passing a location
means that the hook can report an error in its own right, but not passing
a quiet/report flag means that the hook can't tell when it is being called
as a pure query.
It might be more consistent to either:
(1) Make this hook a pure query that doesn't take a location_t and
doesn't report errors directly. Like you say, this would require
more refactoring of the RISC-V code. It would also deny targets the
opportunity to report more bespoke diagnostics.
(2) Delegate all diagnostics to the hook, with a parameter to say when
no diagnostic is needed. The aarch64 version would then do:
warning (OPT_Wattributes,
"invalid %<target_clones%> version %qB ignored",
&v);
itself. It could also report a specific feature that wasn't
recognised, rather than reporting the full string.
(2) seems better to me, but I suppose it means more duplication
between targets.
It might be better to use a positive or neutral name, to avoid double
negatives. Maybe something like check_* or verify_*?
Thanks,
Richard
