On 31/07/2025 16:37, Richard Sandiford wrote:
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.
NP, thanks for the review!
I was never very happy with how this hook came out so happy to change it.
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.
Yup agreed, in hindsight that's a better option. I will send an updated
version.
Thanks,
Alfie
It might be better to use a positive or neutral name, to avoid double
negatives. Maybe something like check_* or verify_*?
Thanks,
Richard
