The 12/15/2025 23:53, Andrew Pinski wrote:
> On Mon, Dec 15, 2025 at 11:28 PM Andrew Pinski
> <[email protected]> wrote:
> >
> > On Fri, Nov 7, 2025 at 8:08 AM Alfie Richards <[email protected]>
> > wrote:
> > >
> > > This changes the hook to support checking version mergeability for cases
> > > where the version strings do imply the same version, but are conflicting
> > > in some
> > > other way so cannot be merged.
> > >
> > > This is a change required for adding priority version support in aarch64.
> > >
> > > gcc/ChangeLog:
> > >
> > > * target.def (TARGET_OPTION_SAME_FUNCTION_VERSIONS): Update
> > > documentation.
> > > * tree.cc (disjoint_version_decls): Change for new NULL parameter
> > > to
> > > same_function_versions.
> > > (diagnose_versioned_decls): Update to pass diagnostic location to
> > > same_function_versions.
> > > * doc/tm.texi: Regenerate.
> > > * config/aarch64/aarch64.cc (aarch64_same_function_versions):
> > > Update
> > > hook impl for new argument.
> > > * config/riscv/riscv.cc (riscv_same_function_versions): Update
> > > hook impl
> > > for new argument.
> > > * hooks.cc (hook_stringslice_stringslice_unreachable): Changed
> > > to...
> > > (hook_stringslice_stringslice_locationtptr_unreachable ): ...this
> > > and
> > > add extra argument.
> > > ---
> > > gcc/config/aarch64/aarch64.cc | 19 +++++++++++--------
> > > gcc/config/riscv/riscv.cc | 2 +-
> > > gcc/doc/tm.texi | 8 +++++++-
> > > gcc/hooks.cc | 2 +-
> > > gcc/target.def | 18 +++++++++++++-----
> > > gcc/tree.cc | 34 +++++++++++++++++++---------------
> > > 6 files changed, 52 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> > > index b3e752bbd8c..abc5bafb50d 100644
> > > --- a/gcc/config/aarch64/aarch64.cc
> > > +++ b/gcc/config/aarch64/aarch64.cc
> > > @@ -21278,19 +21278,22 @@ aarch64_get_function_versions_dispatcher (void
> > > *decl)
> > > function. */
> > >
> > > bool
> > > -aarch64_same_function_versions (string_slice str1, string_slice str2)
> > > +aarch64_same_function_versions (string_slice old_str, string_slice
> > > new_str,
> > > + location_t *)
> > > {
> > > enum aarch_parse_opt_result parse_res;
> > > - aarch64_fmv_feature_mask feature_mask1;
> > > - aarch64_fmv_feature_mask feature_mask2;
> > > - parse_res = aarch64_parse_fmv_features (str1, NULL,
> > > - &feature_mask1, NULL);
> > > + aarch64_fmv_feature_mask old_feature_mask;
> > > + aarch64_fmv_feature_mask new_feature_mask;
> > > +
> > > + parse_res = aarch64_parse_fmv_features (old_str, NULL,
> > > &old_feature_mask,
> > > + NULL);
> > > gcc_assert (parse_res == AARCH_PARSE_OK);
> > > - parse_res = aarch64_parse_fmv_features (str2, NULL,
> > > - &feature_mask2, NULL);
> > > +
> > > + parse_res = aarch64_parse_fmv_features (new_str, NULL,
> > > &new_feature_mask,
> > > + NULL);
> > > gcc_assert (parse_res == AARCH_PARSE_OK);
> > >
> > > - return feature_mask1 == feature_mask2;
> > > + return old_feature_mask == new_feature_mask;
> > > }
> > >
> > > /* Implement TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P. Use an opt-out
> > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > > index 7d723fc4d69..a8962860a6b 100644
> > > --- a/gcc/config/riscv/riscv.cc
> > > +++ b/gcc/config/riscv/riscv.cc
> > > @@ -14744,7 +14744,7 @@ compare_fmv_features (const struct
> > > riscv_feature_bits &mask1,
> > > version. */
> > >
> > > bool
> > > -riscv_same_function_versions (string_slice v1, string_slice v2)
> > > +riscv_same_function_versions (string_slice v1, string_slice v2,
> > > location_t *)
> > > {
> > > struct riscv_feature_bits mask1, mask2;
> > > int prio1, prio2;
> > > diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> > > index fd208f53844..17d8c3ac40e 100644
> > > --- a/gcc/doc/tm.texi
> > > +++ b/gcc/doc/tm.texi
> > > @@ -10997,9 +10997,15 @@ changed via the optimize attribute or pragma, see
> > > @code{TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE}
> > > @end deftypefn
> > >
> > > -@deftypefn {Target Hook} bool TARGET_OPTION_SAME_FUNCTION_VERSIONS
> > > (string_slice @var{fn1}, string_slice @var{fn2})
> > > +@deftypefn {Target Hook} bool TARGET_OPTION_SAME_FUNCTION_VERSIONS
> > > (string_slice @var{fn1}, string_slice @var{fn2}, location_t *@var{loc})
> > > This target hook returns @code{true} if the target/target-version strings
> > > @var{fn1} and @var{fn2} imply the same function version.
> > > +
> > > +If @var{loc} is non @code{NULL} then @var{fn1} and @var{fn2} are from an
> > > +earlier and a later declaration respectively, and the hook diagnoses any
> > > +version string incompatibility for these decls.
> > > +If the version strings are incompatible and the declarations can not be
> > > +merged, then hook emits an error at @var{loc}.
> >
> > Constantify loc since this target hook should NOT be changing what loc
> > points to.
Will do.
> > I am not sure I like the idea of passing a pointer here for making
> > this specify if we are merging from the same decl or from different
> > declarations.
> > Also it would be useful to print out where the old declaration was.
> > So I think the interface should be:
> > (string_slice fn1, location_t loc1, string_slice fn2, location_t loc2)
> >
> > And then have UNKNOWN_LOC for both locations for the case of not from
> > an earlier/later declaration.
> > And then in aarch64, do an error for the new decl (loc2) and a note
> > where the old one was.
>
> The other option is to pass both decls. That actually might be the
> better option here.
> That is:
> (string_slice fn1, tree old_decl, string_slice fn2, tree new_decl)
>
> And then the error message in aarch64 would be something like:
> error ("%q+D has an inconsistent function multi-version priority
> value", new_decl);
> note("%q+D was previously declared here with priority value of %d",
> old_decl, old_priority);
I went for this pattern to match what Richard S used in
r16-3212-g84628fdbe51da2 for similar code and a similar use case.
I am slightly hesitant to pass the decl as it feels unatural to pass the
decl and the string_slice from the decl. But it would no doubt make the
diagnostics better.
Happy to do either. My preference is to take your initial suggestion of
passing two locations, though I also don't love using UNKNOWN_LOC to
mean "dont diagnose mergeability" as sometimes UNKNOWN_LOC can be assigned
to nodes for other reasons right?
Kind regards,
Alfie
>
> Thanks,
> Andrew Pinski
>
> >
> > Thanks,
> > Andrew
> >
> > > @end deftypefn
> > >
> > > @deftypefn {Target Hook} bool
> > > TARGET_OPTION_FUNCTIONS_B_RESOLVABLE_FROM_A (tree @var{decl_a}, tree
> > > @var{decl_v}, tree @var{base})
> > > diff --git a/gcc/hooks.cc b/gcc/hooks.cc
> > > index 5321142495b..ff2bd80ae77 100644
> > > --- a/gcc/hooks.cc
> > > +++ b/gcc/hooks.cc
> > > @@ -595,7 +595,7 @@ hook_stringslice_locationtptr_true (string_slice,
> > > location_t *)
> > > }
> > >
> > > bool
> > > -hook_stringslice_stringslice_unreachable (string_slice, string_slice)
> > > +hook_stringslice_stringslice_locationtptr_unreachable (string_slice,
> > > string_slice, location_t *)
> > > {
> > > gcc_unreachable ();
> > > }
> > > diff --git a/gcc/target.def b/gcc/target.def
> > > index f288329ffca..dae7da331c2 100644
> > > --- a/gcc/target.def
> > > +++ b/gcc/target.def
> > > @@ -6954,14 +6954,22 @@ changed via the optimize attribute or pragma,
> > > see\n\
> > > void, (void),
> > > hook_void_void)
> > >
> > > -/* This function returns true if FN1 and FN2 define the same version of a
> > > - function. */
> > > +/* This function returns true if FN1 and FN2 define the same version
> > > + of a function.
> > > + If LOC is not-null then emit a diagnostic if the versions do imply
> > > the same
> > > + version, but are not mergeable. */
> > > DEFHOOK
> > > (same_function_versions,
> > > "This target hook returns @code{true} if the target/target-version
> > > strings\n\
> > > -@var{fn1} and @var{fn2} imply the same function version.",
> > > - bool, (string_slice fn1, string_slice fn2),
> > > - hook_stringslice_stringslice_unreachable)
> > > +@var{fn1} and @var{fn2} imply the same function version.\n\
> > > +\n\
> > > +If @var{loc} is non @code{NULL} then @var{fn1} and @var{fn2} are from
> > > an\n\
> > > +earlier and a later declaration respectively, and the hook diagnoses
> > > any\n\
> > > +version string incompatibility for these decls.\n\
> > > +If the version strings are incompatible and the declarations can not
> > > be\n\
> > > +merged, then hook emits an error at @var{loc}.",
> > > + bool, (string_slice fn1, string_slice fn2, location_t *loc),
> > > + hook_stringslice_stringslice_locationtptr_unreachable)
> > >
> > > /* Checks if we can be certain that function DECL_A could resolve
> > > DECL_B. */
> > > DEFHOOK
> > > diff --git a/gcc/tree.cc b/gcc/tree.cc
> > > index 298784e6960..c4cb65e3220 100644
> > > --- a/gcc/tree.cc
> > > +++ b/gcc/tree.cc
> > > @@ -15605,7 +15605,8 @@ disjoint_version_decls (tree fn1, tree fn2)
> > > for (string_slice v1 : fn1_versions)
> > > {
> > > for (string_slice v2 : fn2_versions)
> > > - if (targetm.target_option.same_function_versions (v1,
> > > v2))
> > > + if (targetm.target_option.same_function_versions (v1,
> > > v2,
> > > +
> > > NULL))
> > > return false;
> > > }
> > > return true;
> > > @@ -15623,7 +15624,7 @@ disjoint_version_decls (tree fn1, tree fn2)
> > > if (!v2.is_valid ())
> > > v2 = "default";
> > > for (string_slice v1 : fn1_versions)
> > > - if (targetm.target_option.same_function_versions (v1, v2))
> > > + if (targetm.target_option.same_function_versions (v1, v2,
> > > NULL))
> > > return false;
> > > return true;
> > > }
> > > @@ -15642,7 +15643,7 @@ disjoint_version_decls (tree fn1, tree fn2)
> > > if (!v2.is_valid ())
> > > v2 = "default";
> > >
> > > - if (targetm.target_option.same_function_versions (v1, v2))
> > > + if (targetm.target_option.same_function_versions (v1, v2, NULL))
> > > return false;
> > >
> > > return true;
> > > @@ -15690,30 +15691,32 @@ diagnose_versioned_decls (tree old_decl, tree
> > > new_decl)
> > > the two sets of target_clones imply the same set of versions. */
> > > if (old_target_clones_attr && new_target_clones_attr)
> > > {
> > > - auto_vec<string_slice> fn1_versions = get_clone_versions
> > > (old_decl);
> > > - auto_vec<string_slice> fn2_versions = get_clone_versions
> > > (new_decl);
> > > + auto_vec<string_slice> old_versions = get_clone_versions
> > > (old_decl);
> > > + auto_vec<string_slice> new_versions = get_clone_versions
> > > (new_decl);
> > >
> > > bool mergeable = true;
> > >
> > > - if (fn1_versions.length () != fn2_versions.length ())
> > > + if (old_versions.length () != new_versions.length ())
> > > mergeable = false;
> > >
> > > /* Check both inclusion directions. */
> > > - for (auto fn1v : fn1_versions)
> > > + for (auto oldv: old_versions)
> > > {
> > > bool matched = false;
> > > - for (auto fn2v : fn2_versions)
> > > - if (targetm.target_option.same_function_versions (fn1v, fn2v))
> > > + for (auto newv: new_versions)
> > > + if (targetm.target_option.same_function_versions (
> > > + oldv, newv, &DECL_SOURCE_LOCATION (new_decl)))
> > > matched = true;
> > > if (!matched)
> > > mergeable = false;
> > > }
> > >
> > > - for (auto fn2v : fn2_versions)
> > > + for (auto newv: new_versions)
> > > {
> > > bool matched = false;
> > > - for (auto fn1v : fn1_versions)
> > > - if (targetm.target_option.same_function_versions (fn1v, fn2v))
> > > + for (auto oldv: old_versions)
> > > + if (targetm.target_option.same_function_versions (
> > > + oldv, newv, &DECL_SOURCE_LOCATION (new_decl)))
> > > matched = true;
> > > if (!matched)
> > > mergeable = false;
> > > @@ -15762,9 +15765,10 @@ diagnose_versioned_decls (tree old_decl, tree
> > > new_decl)
> > > return true;
> > > }
> > >
> > > - /* The only remaining case is two target_version annotated decls. Must
> > > - be mergeable as otherwise are distinct. */
> > > - return false;
> > > + /* The only remaining case is two target_version annotated decls. */
> > > + return (!targetm.target_option.same_function_versions
> > > + (old_target_attr, new_target_attr,
> > > + &DECL_SOURCE_LOCATION (new_decl)));
> > > }
> > >
> > > void
> > > --
> > > 2.34.1
> > >
--
Alfie Richards
