[v1,06/16] Change function versions to be implicitly ordered.

Message ID 20250203130421.2192732-8-alfie.richards@arm.com
State New
Headers
Series FMV refactor and ACLE compliance. |

Checks

Context Check Description
rivoscibot/toolchain-ci-rivos-apply-patch success Patch applied
rivoscibot/toolchain-ci-rivos-lint warning Lint failed
rivoscibot/toolchain-ci-rivos-build--newlib-rv64gcv-lp64d-multilib success Build passed
rivoscibot/toolchain-ci-rivos-build--linux-rv64gc_zba_zbb_zbc_zbs-lp64d-multilib success Build passed
rivoscibot/toolchain-ci-rivos-build--linux-rv64gcv-lp64d-multilib success Build passed
rivoscibot/toolchain-ci-rivos-test success Testing passed

Commit Message

Alfie Richards Feb. 3, 2025, 1:04 p.m. UTC
  This changes function version structures to maintain the default version
as the first declaration in the linked data structures by giving priority
to the set containing the default when constructing the structure.

This allows for removing logic for moving the default to the first
position which was duplicated across target specific code and enables
easier reasoning about function sets when checking for a default.

gcc/ChangeLog:

	* cgraph.cc (cgraph_node::record_function_versions): Update to
	implicitly keep default first.
	* config/aarch64/aarch64.cc (aarch64_get_function_versions_dispatcher):
	Remove reordering.
	* config/i386/i386-features.cc (ix86_get_function_versions_dispatcher):
	Remove reordering.
	* config/riscv/riscv.cc (riscv_get_function_versions_dispatcher):
	Remove reordering.
	* config/rs6000/rs6000.cc (rs6000_get_function_versions_dispatcher):
	Remove reordering.
---
 gcc/cgraph.cc                    | 39 +++++++++++++++++++++++-------
 gcc/config/aarch64/aarch64.cc    | 37 +++++++---------------------
 gcc/config/i386/i386-features.cc | 33 ++++---------------------
 gcc/config/riscv/riscv.cc        | 41 +++++++-------------------------
 gcc/config/rs6000/rs6000.cc      | 35 +++++----------------------
 5 files changed, 58 insertions(+), 127 deletions(-)
  

Comments

Richard Sandiford Feb. 3, 2025, 5:16 p.m. UTC | #1
Alfie Richards <alfie.richards@arm.com> writes:
> This changes function version structures to maintain the default version
> as the first declaration in the linked data structures by giving priority
> to the set containing the default when constructing the structure.
>
> This allows for removing logic for moving the default to the first
> position which was duplicated across target specific code and enables
> easier reasoning about function sets when checking for a default.
>
> gcc/ChangeLog:
>
> 	* cgraph.cc (cgraph_node::record_function_versions): Update to
> 	implicitly keep default first.
> 	* config/aarch64/aarch64.cc (aarch64_get_function_versions_dispatcher):
> 	Remove reordering.
> 	* config/i386/i386-features.cc (ix86_get_function_versions_dispatcher):
> 	Remove reordering.
> 	* config/riscv/riscv.cc (riscv_get_function_versions_dispatcher):
> 	Remove reordering.
> 	* config/rs6000/rs6000.cc (rs6000_get_function_versions_dispatcher):
> 	Remove reordering.

Thanks, this is a really nice clean-up.  I see that it's already the
documented expectation:

  /* Chains all the semantically identical function versions.  The
     first function in this chain is the version_info node of the
     default function.  */
  cgraph_function_version_info *prev;

So in a sense the patch isn't changing the structures.  It's simply making
the current expectation always hold, rather than hold after a certain point.

Some comments below.

> ---
>  gcc/cgraph.cc                    | 39 +++++++++++++++++++++++-------
>  gcc/config/aarch64/aarch64.cc    | 37 +++++++---------------------
>  gcc/config/i386/i386-features.cc | 33 ++++---------------------
>  gcc/config/riscv/riscv.cc        | 41 +++++++-------------------------
>  gcc/config/rs6000/rs6000.cc      | 35 +++++----------------------
>  5 files changed, 58 insertions(+), 127 deletions(-)
>
> diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
> index d0b19ad850e..1ea38d16e56 100644
> --- a/gcc/cgraph.cc
> +++ b/gcc/cgraph.cc
> @@ -236,37 +236,58 @@ cgraph_node::delete_function_version_by_decl (tree decl)
>  void
>  cgraph_node::record_function_versions (tree decl1, tree decl2)
>  {
> -  cgraph_node *decl1_node = cgraph_node::get_create (decl1);
> -  cgraph_node *decl2_node = cgraph_node::get_create (decl2);
> +  cgraph_node *decl1_node;
> +  cgraph_node *decl2_node;
>    cgraph_function_version_info *decl1_v = NULL;
>    cgraph_function_version_info *decl2_v = NULL;
>    cgraph_function_version_info *before;
>    cgraph_function_version_info *after;
> +  cgraph_function_version_info *temp_node;
> +
> +  decl1_node = cgraph_node::get_create (decl1);
> +  decl2_node = cgraph_node::get_create (decl2);
>  
>    gcc_assert (decl1_node != NULL && decl2_node != NULL);
>    decl1_v = decl1_node->function_version ();
>    decl2_v = decl2_node->function_version ();
>  
> -  if (decl1_v != NULL && decl2_v != NULL)
> -    return;
> -

Could you go into more detail about why this return needs to be removed?
It seems like the assumption was that, if the two decls were already
versioned, they were already versions of the same thing.  For example,
we wouldn't create a set of 4 versions and a set of 2 versions and
only then merge them into a single set of 6 versions.  Is that not
the case with the new scheme?

If we could keep the return, then we could add:

  if (is_function_default_version (decl2)
      || (!decl1_v && !is_function_default_version (decl1)))
    {
      std::swap (decl1, decl2);
      std::swap (decl1_v, decl2_v);
    }

after it and then proceed as before, on the basis that (a) decl1_v and
decl2_v are individually canonical and (b) after the swap, any default
must be decl1 or earlier in decl1_v.  That would avoid a bit of extra
pointer chasing.

>    if (decl1_v == NULL)
>      decl1_v = decl1_node->insert_new_function_version ();
>  
>    if (decl2_v == NULL)
>      decl2_v = decl2_node->insert_new_function_version ();
>  
> -  /* Chain decl2_v and decl1_v.  All semantically identical versions
> -     will be chained together.  */
> +  gcc_assert (decl1_v);
> +  gcc_assert (decl2_v);
>  
>    before = decl1_v;
>    after = decl2_v;
>  
> +  /* Go to first after node.  */
> +  while (after->prev != NULL)
> +    after = after->prev;
> +
> +  /* Go to first before node.  */
> +  while (before->prev != NULL)
> +    before = before->prev;
> +
> +  /* These are already recorded as versions.  */
> +  if (before == after)
> +    return;
> +
> +  /* Possibly swap to make sure the default node stays at the front.  */
> +  if (is_function_default_version (after->this_node->decl))
> +    {
> +      temp_node = after;
> +      after = before;
> +      before = temp_node;
> +    }
> +
> +  /* Go to last node of before.  */
>    while (before->next != NULL)
>      before = before->next;
>  
> -  while (after->prev != NULL)
> -    after= after->prev;
> +  /* Chain decl2_v and decl1_v.  */
>  
>    before->next = after;
>    after->prev = before;
> [...]
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 439cc12f93d..69e47ec7e79 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -13719,7 +13719,6 @@ riscv_get_function_versions_dispatcher (void *decl)
>    struct cgraph_node *node = NULL;
>    struct cgraph_node *default_node = NULL;
>    struct cgraph_function_version_info *node_v = NULL;
> -  struct cgraph_function_version_info *first_v = NULL;
>  
>    tree dispatch_decl = NULL;
>  
> @@ -13736,41 +13735,19 @@ riscv_get_function_versions_dispatcher (void *decl)
>    if (node_v->dispatcher_resolver != NULL)
>      return node_v->dispatcher_resolver;
>  
> -  /* Find the default version and make it the first node.  */
> -  first_v = node_v;
> -  /* Go to the beginning of the chain.  */
> -  while (first_v->prev != NULL)
> -    first_v = first_v->prev;
> -  default_version_info = first_v;
> -
> -  while (default_version_info != NULL)
> -    {
> -      struct riscv_feature_bits res;
> -      int priority; /* Unused.  */
> -      parse_features_for_version (default_version_info->this_node->decl,
> -				  res, priority);
> -      if (res.length == 0)
> -	break;
> -      default_version_info = default_version_info->next;
> -    }
> +  /* The default node is always the beginning of the chain.  */
> +  default_version_info = node_v;
> +  while (default_version_info->prev)
> +    default_version_info = default_version_info->prev;
> +  default_node = default_version_info->this_node;
>  
>    /* If there is no default node, just return NULL.  */
> -  if (default_version_info == NULL)
> +  struct riscv_feature_bits res;
> +  int priority; /* Unused.  */
> +  parse_features_for_version (default_node->decl, res, priority);
> +  if (res.length != 0)
>      return NULL;

I expect you were trying to avoid editorialising, but: how about
converting this to is_function_default_version at the same time?
That seems more logically consistent with the target-independent code
and wasn't an option open to the RISC-V code until patch 5.

To put it another way, not being able to use is_function_default_version
here would suggest that the "default-first" assumption doesn't in fact
hold for RISC-V.

Richard

>  
> -  /* Make default info the first node.  */
> -  if (first_v != default_version_info)
> -    {
> -      default_version_info->prev->next = default_version_info->next;
> -      if (default_version_info->next)
> -	default_version_info->next->prev = default_version_info->prev;
> -      first_v->prev = default_version_info;
> -      default_version_info->next = first_v;
> -      default_version_info->prev = NULL;
> -    }
> -
> -  default_node = default_version_info->this_node;
> -
>    if (targetm.has_ifunc_p ())
>      {
>        struct cgraph_function_version_info *it_v = NULL;
  
Richard Sandiford Feb. 4, 2025, 3:11 p.m. UTC | #2
Richard Sandiford <richard.sandiford@arm.com> writes:
> Alfie Richards <alfie.richards@arm.com> writes:
>> ---
>>  gcc/cgraph.cc                    | 39 +++++++++++++++++++++++-------
>>  gcc/config/aarch64/aarch64.cc    | 37 +++++++---------------------
>>  gcc/config/i386/i386-features.cc | 33 ++++---------------------
>>  gcc/config/riscv/riscv.cc        | 41 +++++++-------------------------
>>  gcc/config/rs6000/rs6000.cc      | 35 +++++----------------------
>>  5 files changed, 58 insertions(+), 127 deletions(-)
>>
>> diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
>> index d0b19ad850e..1ea38d16e56 100644
>> --- a/gcc/cgraph.cc
>> +++ b/gcc/cgraph.cc
>> @@ -236,37 +236,58 @@ cgraph_node::delete_function_version_by_decl (tree decl)
>>  void
>>  cgraph_node::record_function_versions (tree decl1, tree decl2)
>>  {
>> -  cgraph_node *decl1_node = cgraph_node::get_create (decl1);
>> -  cgraph_node *decl2_node = cgraph_node::get_create (decl2);
>> +  cgraph_node *decl1_node;
>> +  cgraph_node *decl2_node;
>>    cgraph_function_version_info *decl1_v = NULL;
>>    cgraph_function_version_info *decl2_v = NULL;
>>    cgraph_function_version_info *before;
>>    cgraph_function_version_info *after;
>> +  cgraph_function_version_info *temp_node;
>> +
>> +  decl1_node = cgraph_node::get_create (decl1);
>> +  decl2_node = cgraph_node::get_create (decl2);
>>  
>>    gcc_assert (decl1_node != NULL && decl2_node != NULL);
>>    decl1_v = decl1_node->function_version ();
>>    decl2_v = decl2_node->function_version ();
>>  
>> -  if (decl1_v != NULL && decl2_v != NULL)
>> -    return;
>> -
>
> Could you go into more detail about why this return needs to be removed?
> It seems like the assumption was that, if the two decls were already
> versioned, they were already versions of the same thing.  For example,
> we wouldn't create a set of 4 versions and a set of 2 versions and
> only then merge them into a single set of 6 versions.  Is that not
> the case with the new scheme?
>
> If we could keep the return, then we could add:
>
>   if (is_function_default_version (decl2)
>       || (!decl1_v && !is_function_default_version (decl1)))

On second thoughts, a slightly cleaner alternative might be:

  if (decl2_v
      ? !is_function_default_version (decl1)
      : is_function_default_version (decl2))

>     {
>       std::swap (decl1, decl2);
>       std::swap (decl1_v, decl2_v);
>     }
>
> after it and then proceed as before, on the basis that (a) decl1_v and
> decl2_v are individually canonical and (b) after the swap, any default
> must be decl1 or earlier in decl1_v.  That would avoid a bit of extra
> pointer chasing.
  
Alfie Richards Feb. 4, 2025, 3:21 p.m. UTC | #3
On 04/02/2025 15:11, Richard Sandiford wrote:
> Richard Sandiford <richard.sandiford@arm.com> writes:
>> Alfie Richards <alfie.richards@arm.com> writes:
>>> ---
>>>   gcc/cgraph.cc                    | 39 +++++++++++++++++++++++-------
>>>   gcc/config/aarch64/aarch64.cc    | 37 +++++++---------------------
>>>   gcc/config/i386/i386-features.cc | 33 ++++---------------------
>>>   gcc/config/riscv/riscv.cc        | 41 +++++++-------------------------
>>>   gcc/config/rs6000/rs6000.cc      | 35 +++++----------------------
>>>   5 files changed, 58 insertions(+), 127 deletions(-)
>>>
>>> diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
>>> index d0b19ad850e..1ea38d16e56 100644
>>> --- a/gcc/cgraph.cc
>>> +++ b/gcc/cgraph.cc
>>> @@ -236,37 +236,58 @@ cgraph_node::delete_function_version_by_decl (tree decl)
>>>   void
>>>   cgraph_node::record_function_versions (tree decl1, tree decl2)
>>>   {
>>> -  cgraph_node *decl1_node = cgraph_node::get_create (decl1);
>>> -  cgraph_node *decl2_node = cgraph_node::get_create (decl2);
>>> +  cgraph_node *decl1_node;
>>> +  cgraph_node *decl2_node;
>>>     cgraph_function_version_info *decl1_v = NULL;
>>>     cgraph_function_version_info *decl2_v = NULL;
>>>     cgraph_function_version_info *before;
>>>     cgraph_function_version_info *after;
>>> +  cgraph_function_version_info *temp_node;
>>> +
>>> +  decl1_node = cgraph_node::get_create (decl1);
>>> +  decl2_node = cgraph_node::get_create (decl2);
>>>   
>>>     gcc_assert (decl1_node != NULL && decl2_node != NULL);
>>>     decl1_v = decl1_node->function_version ();
>>>     decl2_v = decl2_node->function_version ();
>>>   
>>> -  if (decl1_v != NULL && decl2_v != NULL)
>>> -    return;
>>> -
>> Could you go into more detail about why this return needs to be removed?
>> It seems like the assumption was that, if the two decls were already
>> versioned, they were already versions of the same thing.  For example,
>> we wouldn't create a set of 4 versions and a set of 2 versions and
>> only then merge them into a single set of 6 versions.  Is that not
>> the case with the new scheme?
This was removed because function versions are populated when the assembler
name is set (in the commit #9 of this series). So I removed this check and
added the later check which exits if the first node of each series is equal.
I've come up with a better way of handling assembler names though that
doesn't require initializing the function version members so early and
can add this simpler check back in.

You're right that we never join two concurrently built version sets, 
though this function is constructed in a way to make that mostly possible.
>> If we could keep the return, then we could add:
>>
>>    if (is_function_default_version (decl2)
>>        || (!decl1_v && !is_function_default_version (decl1)))
> On second thoughts, a slightly cleaner alternative might be:
>
>    if (decl2_v
>        ? !is_function_default_version (decl1)
>        : is_function_default_version (decl2))
Yeah this is nice, I will switch to this for the next version.
>>      {
>>        std::swap (decl1, decl2);
>>        std::swap (decl1_v, decl2_v);
>>      }
>>
>> after it and then proceed as before, on the basis that (a) decl1_v and
>> decl2_v are individually canonical and (b) after the swap, any default
>> must be decl1 or earlier in decl1_v.  That would avoid a bit of extra
>> pointer chasing.
Thank you for the feedback!
  

Patch

diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
index d0b19ad850e..1ea38d16e56 100644
--- a/gcc/cgraph.cc
+++ b/gcc/cgraph.cc
@@ -236,37 +236,58 @@  cgraph_node::delete_function_version_by_decl (tree decl)
 void
 cgraph_node::record_function_versions (tree decl1, tree decl2)
 {
-  cgraph_node *decl1_node = cgraph_node::get_create (decl1);
-  cgraph_node *decl2_node = cgraph_node::get_create (decl2);
+  cgraph_node *decl1_node;
+  cgraph_node *decl2_node;
   cgraph_function_version_info *decl1_v = NULL;
   cgraph_function_version_info *decl2_v = NULL;
   cgraph_function_version_info *before;
   cgraph_function_version_info *after;
+  cgraph_function_version_info *temp_node;
+
+  decl1_node = cgraph_node::get_create (decl1);
+  decl2_node = cgraph_node::get_create (decl2);
 
   gcc_assert (decl1_node != NULL && decl2_node != NULL);
   decl1_v = decl1_node->function_version ();
   decl2_v = decl2_node->function_version ();
 
-  if (decl1_v != NULL && decl2_v != NULL)
-    return;
-
   if (decl1_v == NULL)
     decl1_v = decl1_node->insert_new_function_version ();
 
   if (decl2_v == NULL)
     decl2_v = decl2_node->insert_new_function_version ();
 
-  /* Chain decl2_v and decl1_v.  All semantically identical versions
-     will be chained together.  */
+  gcc_assert (decl1_v);
+  gcc_assert (decl2_v);
 
   before = decl1_v;
   after = decl2_v;
 
+  /* Go to first after node.  */
+  while (after->prev != NULL)
+    after = after->prev;
+
+  /* Go to first before node.  */
+  while (before->prev != NULL)
+    before = before->prev;
+
+  /* These are already recorded as versions.  */
+  if (before == after)
+    return;
+
+  /* Possibly swap to make sure the default node stays at the front.  */
+  if (is_function_default_version (after->this_node->decl))
+    {
+      temp_node = after;
+      after = before;
+      before = temp_node;
+    }
+
+  /* Go to last node of before.  */
   while (before->next != NULL)
     before = before->next;
 
-  while (after->prev != NULL)
-    after= after->prev;
+  /* Chain decl2_v and decl1_v.  */
 
   before->next = after;
   after->prev = before;
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index be99137b052..15dd7dda48a 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -20630,7 +20630,6 @@  aarch64_get_function_versions_dispatcher (void *decl)
   struct cgraph_node *node = NULL;
   struct cgraph_node *default_node = NULL;
   struct cgraph_function_version_info *node_v = NULL;
-  struct cgraph_function_version_info *first_v = NULL;
 
   tree dispatch_decl = NULL;
 
@@ -20647,37 +20646,17 @@  aarch64_get_function_versions_dispatcher (void *decl)
   if (node_v->dispatcher_resolver != NULL)
     return node_v->dispatcher_resolver;
 
-  /* Find the default version and make it the first node.  */
-  first_v = node_v;
-  /* Go to the beginning of the chain.  */
-  while (first_v->prev != NULL)
-    first_v = first_v->prev;
-  default_version_info = first_v;
-  while (default_version_info != NULL)
-    {
-      if (get_feature_mask_for_version
-	    (default_version_info->this_node->decl) == 0ULL)
-	break;
-      default_version_info = default_version_info->next;
-    }
-
-  /* If there is no default node, just return NULL.  */
-  if (default_version_info == NULL)
-    return NULL;
-
-  /* Make default info the first node.  */
-  if (first_v != default_version_info)
-    {
-      default_version_info->prev->next = default_version_info->next;
-      if (default_version_info->next)
-	default_version_info->next->prev = default_version_info->prev;
-      first_v->prev = default_version_info;
-      default_version_info->next = first_v;
-      default_version_info->prev = NULL;
-    }
+  /* The default node is always the beginning of the chain.  */
+  default_version_info = node_v;
+  while (default_version_info->prev != NULL)
+    default_version_info = default_version_info->prev;
 
   default_node = default_version_info->this_node;
 
+  /* If there is no default version, just return NULL.  */
+  if (!is_function_default_version (default_node->decl))
+    return NULL;
+
   if (targetm.has_ifunc_p ())
     {
       struct cgraph_function_version_info *it_v = NULL;
diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc
index c35ac24fd8a..2eb3a21bb5d 100644
--- a/gcc/config/i386/i386-features.cc
+++ b/gcc/config/i386/i386-features.cc
@@ -3962,7 +3962,6 @@  ix86_get_function_versions_dispatcher (void *decl)
   struct cgraph_node *node = NULL;
   struct cgraph_node *default_node = NULL;
   struct cgraph_function_version_info *node_v = NULL;
-  struct cgraph_function_version_info *first_v = NULL;
 
   tree dispatch_decl = NULL;
 
@@ -3979,37 +3978,15 @@  ix86_get_function_versions_dispatcher (void *decl)
   if (node_v->dispatcher_resolver != NULL)
     return node_v->dispatcher_resolver;
 
-  /* Find the default version and make it the first node.  */
-  first_v = node_v;
-  /* Go to the beginning of the chain.  */
-  while (first_v->prev != NULL)
-    first_v = first_v->prev;
-  default_version_info = first_v;
-  while (default_version_info != NULL)
-    {
-      if (is_function_default_version
-	    (default_version_info->this_node->decl))
-	break;
-      default_version_info = default_version_info->next;
-    }
+  /* The default node is always the beginning of the chain.  */
+  default_version_info = node_v;
+  while (default_version_info->prev != NULL)
+    default_version_info = default_version_info->prev;
 
   /* If there is no default node, just return NULL.  */
-  if (default_version_info == NULL)
+  if (!is_function_default_version (default_node->decl))
     return NULL;
 
-  /* Make default info the first node.  */
-  if (first_v != default_version_info)
-    {
-      default_version_info->prev->next = default_version_info->next;
-      if (default_version_info->next)
-	default_version_info->next->prev = default_version_info->prev;
-      first_v->prev = default_version_info;
-      default_version_info->next = first_v;
-      default_version_info->prev = NULL;
-    }
-
-  default_node = default_version_info->this_node;
-
 #if defined (ASM_OUTPUT_TYPE_DIRECTIVE)
   if (targetm.has_ifunc_p ())
     {
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 439cc12f93d..69e47ec7e79 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -13719,7 +13719,6 @@  riscv_get_function_versions_dispatcher (void *decl)
   struct cgraph_node *node = NULL;
   struct cgraph_node *default_node = NULL;
   struct cgraph_function_version_info *node_v = NULL;
-  struct cgraph_function_version_info *first_v = NULL;
 
   tree dispatch_decl = NULL;
 
@@ -13736,41 +13735,19 @@  riscv_get_function_versions_dispatcher (void *decl)
   if (node_v->dispatcher_resolver != NULL)
     return node_v->dispatcher_resolver;
 
-  /* Find the default version and make it the first node.  */
-  first_v = node_v;
-  /* Go to the beginning of the chain.  */
-  while (first_v->prev != NULL)
-    first_v = first_v->prev;
-  default_version_info = first_v;
-
-  while (default_version_info != NULL)
-    {
-      struct riscv_feature_bits res;
-      int priority; /* Unused.  */
-      parse_features_for_version (default_version_info->this_node->decl,
-				  res, priority);
-      if (res.length == 0)
-	break;
-      default_version_info = default_version_info->next;
-    }
+  /* The default node is always the beginning of the chain.  */
+  default_version_info = node_v;
+  while (default_version_info->prev)
+    default_version_info = default_version_info->prev;
+  default_node = default_version_info->this_node;
 
   /* If there is no default node, just return NULL.  */
-  if (default_version_info == NULL)
+  struct riscv_feature_bits res;
+  int priority; /* Unused.  */
+  parse_features_for_version (default_node->decl, res, priority);
+  if (res.length != 0)
     return NULL;
 
-  /* Make default info the first node.  */
-  if (first_v != default_version_info)
-    {
-      default_version_info->prev->next = default_version_info->next;
-      if (default_version_info->next)
-	default_version_info->next->prev = default_version_info->prev;
-      first_v->prev = default_version_info;
-      default_version_info->next = first_v;
-      default_version_info->prev = NULL;
-    }
-
-  default_node = default_version_info->this_node;
-
   if (targetm.has_ifunc_p ())
     {
       struct cgraph_function_version_info *it_v = NULL;
diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index 675b039c2b6..c59bd43c76d 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -25295,7 +25295,6 @@  rs6000_get_function_versions_dispatcher (void *decl)
   struct cgraph_node *node = NULL;
   struct cgraph_node *default_node = NULL;
   struct cgraph_function_version_info *node_v = NULL;
-  struct cgraph_function_version_info *first_v = NULL;
 
   tree dispatch_decl = NULL;
 
@@ -25315,38 +25314,16 @@  rs6000_get_function_versions_dispatcher (void *decl)
   if (node_v->dispatcher_resolver != NULL)
     return node_v->dispatcher_resolver;
 
-  /* Find the default version and make it the first node.  */
-  first_v = node_v;
-  /* Go to the beginning of the chain.  */
-  while (first_v->prev != NULL)
-    first_v = first_v->prev;
-
-  default_version_info = first_v;
-  while (default_version_info != NULL)
-    {
-      const tree decl2 = default_version_info->this_node->decl;
-      if (is_function_default_version (decl2))
-        break;
-      default_version_info = default_version_info->next;
-    }
+  /* The default node is always the beginning of the chain.  */
+  default_version_info = node_v;
+  while (default_version_info->prev)
+    default_version_info = default_version_info->prev;
+  default_node = default_version_info->this_node;
 
   /* If there is no default node, just return NULL.  */
-  if (default_version_info == NULL)
+  if (!is_function_default_version (default_node->decl))
     return NULL;
 
-  /* Make default info the first node.  */
-  if (first_v != default_version_info)
-    {
-      default_version_info->prev->next = default_version_info->next;
-      if (default_version_info->next)
-        default_version_info->next->prev = default_version_info->prev;
-      first_v->prev = default_version_info;
-      default_version_info->next = first_v;
-      default_version_info->prev = NULL;
-    }
-
-  default_node = default_version_info->this_node;
-
 #ifndef TARGET_LIBC_PROVIDES_HWCAP_IN_TCB
   error_at (DECL_SOURCE_LOCATION (default_node->decl),
 	    "%<target_clones%> attribute needs GLIBC (2.23 and newer) that "