RISC-V: Support extension version number 0.0

Message ID 1f10468a4c8e5c5ba8984d2afa92bdca7be7f871.1691385124.git.research_trasio@irq.a4lg.com
State Accepted
Headers
Series RISC-V: Support extension version number 0.0 |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Tsukasa OI Aug. 7, 2023, 5:13 a.m. UTC
  From: Tsukasa OI <research_trasio@irq.a4lg.com>

Before this commit, the ISA string with "xextname0p0" is handled as the
'Xextname' without version number information (not version 0.0 as specified
in the string).

This commit makes able to handle version 0.0 and enable handling placeholder
extensions (with *actual* version number) easily (without being handled as
an unknown version).

This behavior is consistent with other locations that use the
"find_any_version" variable.

bfd/ChangeLog:

	* elfxx-riscv.c (riscv_parsing_subset_version): Check whether we
	have actually found a version number.
---
 bfd/elfxx-riscv.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)


base-commit: ad233d0d74661e5206d093e826db1eb4120c5fef
  

Comments

Nelson Chu Aug. 7, 2023, 7:05 a.m. UTC | #1
On Mon, Aug 7, 2023 at 1:13 PM Tsukasa OI <research_trasio@irq.a4lg.com>
wrote:

> From: Tsukasa OI <research_trasio@irq.a4lg.com>
>
> Before this commit, the ISA string with "xextname0p0" is handled as the
> 'Xextname' without version number information (not version 0.0 as specified
> in the string).
>

This is an expected behavior.  The 0p0 means 0.0, since we won't have any
version 0.0, so it means users don't give any specific version for
extension, and let the compiler/assembler decide (usually use the default
setting).  The xextname0p0 is illegal, since toolchain won't have any
default versions for those x extensions, so users must give a meaningful
one.

Nelson


>
> This commit makes able to handle version 0.0 and enable handling
> placeholder
> extensions (with *actual* version number) easily (without being handled as
> an unknown version).
>
> This behavior is consistent with other locations that use the
> "find_any_version" variable.
>
> bfd/ChangeLog:
>
>         * elfxx-riscv.c (riscv_parsing_subset_version): Check whether we
>         have actually found a version number.
> ---
>  bfd/elfxx-riscv.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index ee4598729480..d1db2df2f271 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -1725,6 +1725,7 @@ riscv_parsing_subset_version (const char *p,
>                               int *minor_version)
>  {
>    bool major_p = true;
> +  bool find_any_version = false;
>    int version = 0;
>    char np;
>
> @@ -1745,7 +1746,10 @@ riscv_parsing_subset_version (const char *p,
>           version = 0;
>         }
>        else if (ISDIGIT (*p))
> -       version = (version * 10) + (*p - '0');
> +       {
> +         find_any_version = true;
> +         version = (version * 10) + (*p - '0');
> +       }
>        else
>         break;
>      }
> @@ -1756,7 +1760,7 @@ riscv_parsing_subset_version (const char *p,
>      *minor_version = version;
>
>    /* We can not find any version in string.  */
> -  if (*major_version == 0 && *minor_version == 0)
> +  if (!find_any_version)
>      {
>        *major_version = RISCV_UNKNOWN_VERSION;
>        *minor_version = RISCV_UNKNOWN_VERSION;
>
> base-commit: ad233d0d74661e5206d093e826db1eb4120c5fef
> --
> 2.41.0
>
>
  
Tsukasa OI Aug. 7, 2023, 7:13 a.m. UTC | #2
On 2023/08/07 16:05, Nelson Chu wrote:
> 
> 
> On Mon, Aug 7, 2023 at 1:13 PM Tsukasa OI <research_trasio@irq.a4lg.com
> <mailto:research_trasio@irq.a4lg.com>> wrote:
> 
>     From: Tsukasa OI <research_trasio@irq.a4lg.com
>     <mailto:research_trasio@irq.a4lg.com>>
> 
>     Before this commit, the ISA string with "xextname0p0" is handled as the
>     'Xextname' without version number information (not version 0.0 as
>     specified
>     in the string).
> 
> 
> This is an expected behavior.  The 0p0 means 0.0, since we won't have
> any version 0.0, so it means users don't give any specific version for
> extension, and let the compiler/assembler decide (usually use the
> default setting).  The xextname0p0 is illegal, since toolchain won't
> have any default versions for those x extensions, so users must give a
> meaningful one.
> 
> Nelson

Understood.

I was making multiple branches corresponding some draft extensions and
some of them don't have version numbers (e.g. if the draft is managed in
Google Docs, not on GitHub).

I used 0.0 as the placeholder and since it's invalid, I will replace
them to 0.1 (I know they are going to be 1.0 once ratified but I don't
want to use it for drafts).

Thanks,
Tsukasa

>  
> 
> 
>     This commit makes able to handle version 0.0 and enable handling
>     placeholder
>     extensions (with *actual* version number) easily (without being
>     handled as
>     an unknown version).
> 
>     This behavior is consistent with other locations that use the
>     "find_any_version" variable.
> 
>     bfd/ChangeLog:
> 
>             * elfxx-riscv.c (riscv_parsing_subset_version): Check whether we
>             have actually found a version number.
>     ---
>      bfd/elfxx-riscv.c | 8 ++++++--
>      1 file changed, 6 insertions(+), 2 deletions(-)
> 
>     diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
>     index ee4598729480..d1db2df2f271 100644
>     --- a/bfd/elfxx-riscv.c
>     +++ b/bfd/elfxx-riscv.c
>     @@ -1725,6 +1725,7 @@ riscv_parsing_subset_version (const char *p,
>                                   int *minor_version)
>      {
>        bool major_p = true;
>     +  bool find_any_version = false;
>        int version = 0;
>        char np;
> 
>     @@ -1745,7 +1746,10 @@ riscv_parsing_subset_version (const char *p,
>               version = 0;
>             }
>            else if (ISDIGIT (*p))
>     -       version = (version * 10) + (*p - '0');
>     +       {
>     +         find_any_version = true;
>     +         version = (version * 10) + (*p - '0');
>     +       }
>            else
>             break;
>          }
>     @@ -1756,7 +1760,7 @@ riscv_parsing_subset_version (const char *p,
>          *minor_version = version;
> 
>        /* We can not find any version in string.  */
>     -  if (*major_version == 0 && *minor_version == 0)
>     +  if (!find_any_version)
>          {
>            *major_version = RISCV_UNKNOWN_VERSION;
>            *minor_version = RISCV_UNKNOWN_VERSION;
> 
>     base-commit: ad233d0d74661e5206d093e826db1eb4120c5fef
>     -- 
>     2.41.0
>
  

Patch

diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
index ee4598729480..d1db2df2f271 100644
--- a/bfd/elfxx-riscv.c
+++ b/bfd/elfxx-riscv.c
@@ -1725,6 +1725,7 @@  riscv_parsing_subset_version (const char *p,
 			      int *minor_version)
 {
   bool major_p = true;
+  bool find_any_version = false;
   int version = 0;
   char np;
 
@@ -1745,7 +1746,10 @@  riscv_parsing_subset_version (const char *p,
 	  version = 0;
 	}
       else if (ISDIGIT (*p))
-	version = (version * 10) + (*p - '0');
+	{
+	  find_any_version = true;
+	  version = (version * 10) + (*p - '0');
+	}
       else
 	break;
     }
@@ -1756,7 +1760,7 @@  riscv_parsing_subset_version (const char *p,
     *minor_version = version;
 
   /* We can not find any version in string.  */
-  if (*major_version == 0 && *minor_version == 0)
+  if (!find_any_version)
     {
       *major_version = RISCV_UNKNOWN_VERSION;
       *minor_version = RISCV_UNKNOWN_VERSION;