[PATCH v2] RISC-V: Deprecate ".option arch, -ext" for users due to its controversial use

Jiawei jiawei@iscas.ac.cn
Wed Jul 9 09:34:34 GMT 2025


LGTM, thanks.

Jiawei

在 2025/7/9 10:47, Nelson Chu 写道:
> Before we figure out the whole remove situations for ".option arch, -ext", and
> have any RISC-V public spec defines it, we should just deprecate it.
>
> v2 changes,
> * Thanks for Jan's suggestion, update the gas/doc/c-riscv-texi to mention
>    alternatives after deprecating ".option arch, -ext" format.
>
> Thanks for Tsukasa's review.  Cc more people who might be interested or need to
> know this change.  Planned to be submitted before 2.45 branch is created if no
> one objects, thanks.
> ---
>   bfd/elfxx-riscv.c                           | 44 +++++++++++++++------
>   bfd/elfxx-riscv.h                           |  3 ++
>   gas/NEWS                                    |  3 ++
>   gas/config/tc-riscv.c                       |  3 +-
>   gas/doc/c-riscv.texi                        | 28 ++++++-------
>   gas/testsuite/gas/riscv/align-1.s           |  2 +-
>   gas/testsuite/gas/riscv/dis-addr-addiw.s    |  2 +-
>   gas/testsuite/gas/riscv/dis-addr-overflow.s |  4 +-
>   gas/testsuite/gas/riscv/ext.s               |  2 +-
>   gas/testsuite/gas/riscv/mapping-x.s         |  2 +-
>   gas/testsuite/gas/riscv/mapping.s           | 10 ++---
>   gas/testsuite/gas/riscv/option-arch-fail.l  |  7 ++--
>   gas/testsuite/gas/riscv/option-arch-fail.s  |  1 +
>   gas/testsuite/gas/riscv/option-arch.s       |  3 +-
>   gas/testsuite/gas/riscv/shamt-32.s          |  2 +-
>   gas/testsuite/gas/riscv/shamt-64.s          |  2 +-
>   16 files changed, 71 insertions(+), 47 deletions(-)
>
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index 43d3c0d0884..776bfd5cea0 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -2602,7 +2602,7 @@ riscv_remove_subset (riscv_subset_list_t *subset_list,
>      called from riscv_update_subset./
>   
>      The IMPLICIT_EXTS, +extension[version] [,...,+extension_n[version_n]]
> -		      -extension [,...,-extension_n],
> +		      (Deprecated) -extension [,...,-extension_n],
>   		      full ISA.  */
>   
>   static bool
> @@ -2694,16 +2694,26 @@ riscv_update_subset1 (riscv_parse_subset_t *rps,
>   	  return false;
>   	}
>   
> -      if (explicit_subset == NULL
> -	  && (strcmp (subset, "i") == 0
> -	      || strcmp (subset, "e") == 0
> -	      || strcmp (subset, "g") == 0))
> +      if (explicit_subset == NULL)
>   	{
> -	  rps->error_handler
> -	    (_("%scannot + or - base extension `%s' in %s `%s'"),
> -	       errmsg_internal, subset, errmsg_caller, implicit_exts);
> -	  free (subset);
> -	  return false;
> +	  if (removed)
> +	    {
> +	      rps->error_handler
> +		(_("%sdeprecated - extension `%s' in %s `%s'"),
> +		   errmsg_internal, subset, errmsg_caller, implicit_exts);
> +	      free (subset);
> +	      return false;
> +	    }
> +	  else if (strcmp (subset, "i") == 0
> +		   || strcmp (subset, "e") == 0
> +		   || strcmp (subset, "g") == 0)
> +	    {
> +	      rps->error_handler
> +		(_("%scannot + base extension `%s' in %s `%s'"),
> +		   errmsg_internal, subset, errmsg_caller, implicit_exts);
> +	      free (subset);
> +	      return false;
> +	    }
>   	}
>   
>         if (removed)
> @@ -2729,8 +2739,8 @@ riscv_update_subset1 (riscv_parse_subset_t *rps,
>     return no_conflict;
>   }
>   
> -/* Add/Remove an extension to/from the subset list.  This is used for
> -   the .option rvc or norvc, and .option arch directives.  */
> +/* Add an extension to/from the subset list.  This is used for the .option rvc
> +   and .option arch directives.  */
>   
>   bool
>   riscv_update_subset (riscv_parse_subset_t *rps,
> @@ -2739,6 +2749,16 @@ riscv_update_subset (riscv_parse_subset_t *rps,
>     return riscv_update_subset1 (rps, NULL, str);
>   }
>   
> +/* Called from .option norvc directives.  */
> +
> +bool
> +riscv_update_subset_norvc (riscv_parse_subset_t *rps)
> +{
> +  return riscv_update_subset1 (rps, rps->subset_list->head,
> +			       "-c,-zca,-zcd,-zcf,-zcb,-zce,-zcmp,-zcmt,"
> +			       "-zcmop,-zclsd");
> +}
> +
>   /* Check if the FEATURE subset is supported or not in the subset list.
>      Return true if it is supported; Otherwise, return false.  */
>   
> diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
> index db494d08a0c..fbbefb5ac18 100644
> --- a/bfd/elfxx-riscv.h
> +++ b/bfd/elfxx-riscv.h
> @@ -118,6 +118,9 @@ riscv_copy_subset_list (riscv_subset_list_t *);
>   extern bool
>   riscv_update_subset (riscv_parse_subset_t *, const char *);
>   
> +extern bool
> +riscv_update_subset_norvc (riscv_parse_subset_t *);
> +
>   extern bool
>   riscv_subset_supports (riscv_parse_subset_t *, const char *);
>   
> diff --git a/gas/NEWS b/gas/NEWS
> index 66a9a7a30df..cbca8fe8656 100644
> --- a/gas/NEWS
> +++ b/gas/NEWS
> @@ -14,6 +14,9 @@
>   * Support for x86 AVX10.2 256 bit rounding has been dropped, as all the
>     hardware would directly support 512 bit vecotr width.
>   
> +* For RISC-V, the ".option arch, -ext" format is deprecated due to its
> +  controversial use.
> +
>   * For RISC-V, stop generating mapping symbols $x and replace with $x<isa>.  The
>     $x was defined to have the same ISA as previous $x<isa>, but now is defined
>     to have the same ISA as elf architecture attribute.  Once used .option arch
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index cb21620b5ff..1e21a1a58d2 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -5076,8 +5076,7 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
>       }
>     else if (strcmp (name, "norvc") == 0)
>       {
> -      riscv_update_subset (&riscv_rps_as, "-c,-zca,-zcd,-zcf,-zcb,-zce,-zcmp,"
> -					  "-zcmt,-zcmop,-zclsd");
> +      riscv_update_subset_norvc (&riscv_rps_as);
>         riscv_arch_str (xlen, riscv_rps_as.subset_list, true/* update */);
>         riscv_set_rvc (false);
>       }
> diff --git a/gas/doc/c-riscv.texi b/gas/doc/c-riscv.texi
> index a4c819e693a..d2ed964229f 100644
> --- a/gas/doc/c-riscv.texi
> +++ b/gas/doc/c-riscv.texi
> @@ -181,12 +181,14 @@ instead of just
>   It's not expected that options are changed in this manner during regular use,
>   but there are a handful of esoteric cases like the one above where users need
>   to disable particular features of the assembler for particular code sequences.
> -However, it's also useful to enable/disable the extensions for some specific
> -code regions by @samp{.option arch, +-}.  This is very common in the ifunc
> -libraries.  We can support functions which are implemented by different
> -extensions in the same library, but these should not affect any file-level
> -settings, like the elf architecture attribute.  The complete list of option
> -arguments is shown below:
> +However, it's also useful to enable and reset the extensions for some specific
> +code regions by @samp{.option arch, +ext} and @samp{.option arch, ISA}.  Or
> +use @samp{.option push} and @samp{.option pop} at the beginning and end of the
> +code, so that we can indirectly turn on and off extensions in this range.  This
> +is very common in the ifunc libraries.  We can support functions which are
> +implemented by different extensions in the same library, but these should not
> +affect any file-level settings, like the elf architecture attribute.  The
> +complete list of option arguments is shown below:
>   
>   @table @code
>   @item push
> @@ -217,17 +219,11 @@ desirable.
>   Enables or disables the CSR checking.
>   
>   @item arch, @var{+extension[version]} [,...,@var{+extension_n[version_n]}]
> -@itemx arch, @var{-extension} [,...,@var{-extension_n}]
>   @itemx arch, @var{ISA}
> -Enables or disables the extensions for specific code region.  For example,
> -@samp{.option arch, +m2p0} means add m extension with version 2.0, and
> -@samp{.option arch, -f, -d} means remove extensions, f and d, from the
> -architecture string.  Note that, @samp{.option arch, +c, -c} have the same
> -behavior as @samp{.option rvc, norvc}.  However, they are also undesirable
> -sometimes.  Besides, @samp{.option arch, -i} is illegal, since we cannot
> -remove the base i extension anytime.  If you want to reset the whole ISA
> -string, you can also use @samp{.option arch, rv32imac} to overwrite the
> -previous settings.
> +Enables or reset the extensions for specific code region.  For example,
> +@samp{.option arch, +m2p0} means add m extension with version 2.0.
> +@samp{.option arch, rv32imac} means reset and overwrite the previous settings
> +by rv32imac.
>   @end table
>   
>   @cindex INSN directives
> diff --git a/gas/testsuite/gas/riscv/align-1.s b/gas/testsuite/gas/riscv/align-1.s
> index ec28556e7be..ee6a94dea78 100644
> --- a/gas/testsuite/gas/riscv/align-1.s
> +++ b/gas/testsuite/gas/riscv/align-1.s
> @@ -1,3 +1,3 @@
> -	.option arch, -c
> +	.option norvc
>   	.align 2
>   	.align 1
> diff --git a/gas/testsuite/gas/riscv/dis-addr-addiw.s b/gas/testsuite/gas/riscv/dis-addr-addiw.s
> index 7c878f86dd6..38dc1c9fc6b 100644
> --- a/gas/testsuite/gas/riscv/dis-addr-addiw.s
> +++ b/gas/testsuite/gas/riscv/dis-addr-addiw.s
> @@ -11,7 +11,7 @@
>   	.global	_start
>   _start:
>   	.option	push
> -	.option	arch, -c
> +	.option norvc
>   	# _start + 0x00
>   	auipc	t0, 0
>   	addiw	t1, t0, 0x18
> diff --git a/gas/testsuite/gas/riscv/dis-addr-overflow.s b/gas/testsuite/gas/riscv/dis-addr-overflow.s
> index 77ca39c07b6..db48ae3f24b 100644
> --- a/gas/testsuite/gas/riscv/dis-addr-overflow.s
> +++ b/gas/testsuite/gas/riscv/dis-addr-overflow.s
> @@ -24,7 +24,7 @@ topbase = 0
>   
>   target:
>   	.option	push
> -	.option	arch, -c
> +	.option norvc
>   	## Use hi_addr
>   	# Load
>   	lui	t0, 0xfffff
> @@ -50,7 +50,7 @@ target:
>   	c.addi	t6, -20
>   .ifdef rv64
>   	.option	push
> -	.option	arch, -c
> +	.option norvc
>   	# ADDIW (not compressed)
>   	lui	s6, 0xffff8
>   	addiw	s7, s6, -24
> diff --git a/gas/testsuite/gas/riscv/ext.s b/gas/testsuite/gas/riscv/ext.s
> index 0268dcafc1d..b6ccce0304c 100644
> --- a/gas/testsuite/gas/riscv/ext.s
> +++ b/gas/testsuite/gas/riscv/ext.s
> @@ -1,5 +1,5 @@
>   target:
> -	.option arch, -c
> +	.option norvc
>   	zext.b	a0, a0
>   	zext.h	a0, a0
>   	sext.b	a0, a0
> diff --git a/gas/testsuite/gas/riscv/mapping-x.s b/gas/testsuite/gas/riscv/mapping-x.s
> index f17cf2295ca..21312539f66 100644
> --- a/gas/testsuite/gas/riscv/mapping-x.s
> +++ b/gas/testsuite/gas/riscv/mapping-x.s
> @@ -1,5 +1,5 @@
>   .attribute arch, "rv32ic"
> -.option arch, -c
> +.option norvc
>   .insn 4, 0x00000013
>   $x:
>   .insn 2, 0x0001
> diff --git a/gas/testsuite/gas/riscv/mapping.s b/gas/testsuite/gas/riscv/mapping.s
> index 0382cfc3c65..c8ed24cc00d 100644
> --- a/gas/testsuite/gas/riscv/mapping.s
> +++ b/gas/testsuite/gas/riscv/mapping.s
> @@ -11,7 +11,7 @@ j	funcA			# rv32ic
>   .globl funcB
>   funcB:
>   addi	a0, zero, 2		# rv32ic, need to be added since start of section
> -.option arch, -c
> +.option norvc
>   j	funcB			# rv32i
>   .option pop
>   
> @@ -35,7 +35,7 @@ addi	a0, zero, 2		# $x, but same as previous addi, so removed
>   .option arch, +c
>   addi	a0, zero, 1		# $xrv32ic
>   .byte	1			# $d
> -.option arch, -c
> +.option norvc
>   .align	3			# odd alignment, $x replaced by $d + $x
>   addi	a0, zero, 2		# $xrv32i
>   .option pop
> @@ -81,7 +81,7 @@ addi	a0, zero, 2
>   .align	2			# $xrv32ic, .align and .fill are in the different frag, so neither be removed,
>   				# but will be removed in riscv_check_mapping_symbols
>   .fill	1, 0, 0			# $d with zero size, removed in make_mapping_symbol when adding $xrv32ic
> -.option arch, -c
> +.option norvc
>   addi	a0, zero, 1		# $xrv32i
>   addi	a0, zero, 2
>   .option pop
> @@ -89,7 +89,7 @@ addi	a0, zero, 2
>   .section .text.last.section, "ax"
>   .option push
>   .option norelax
> -.option arch, -c
> +.option norvc
>   addi	a0, zero, 1		# $xrv32i
>   .word	1			# $d
>   .align	2			# zero section padding, $x at the end of section, removed in riscv_check_mapping_symbols
> @@ -112,7 +112,7 @@ addi	a0, zero, 2		# $xrv32iac
>   .option arch, rv32ic
>   .balign	4			# $xrv32ic, add at the start of section
>   addi	a0, zero, 1		# $x, won't added
> -.option arch, -c
> +.option norvc
>   .align	3			# $x, won't added
>   addi	a0, zero, 2		# $xrv32i
>   .option pop
> diff --git a/gas/testsuite/gas/riscv/option-arch-fail.l b/gas/testsuite/gas/riscv/option-arch-fail.l
> index d83f01d8ad5..191f088c21b 100644
> --- a/gas/testsuite/gas/riscv/option-arch-fail.l
> +++ b/gas/testsuite/gas/riscv/option-arch-fail.l
> @@ -1,8 +1,9 @@
>   .*Assembler messages:
>   .*Error: m2p0: ISA string must begin with rv32, rv64 or Profiles
> -.*Error: cannot \+ or \- base extension `i' in .option arch `\-i'
> -.*Error: cannot \+ or \- base extension `e' in .option arch `\+e'
> -.*Error: cannot \+ or \- base extension `g' in .option arch `\-g'
> +.*Error: deprecated \- extension `i' in .option arch `\-i'
> +.*Error: cannot \+ base extension `e' in .option arch `\+e'
> +.*Error: deprecated \- extension `g' in .option arch `\-g'
> +.*Error: deprecated \- extension `c' in .option arch `\-c'
>   .*Error: unknown ISA extension `zsubset' in .option arch `\+zsubset2p0'
>   .*Error: unknown ISA extension `f2p0_d' in .option arch `\+f2p0_d2p0'
>   .*Error: unknown ISA extension `' in .option arch `\+'
> diff --git a/gas/testsuite/gas/riscv/option-arch-fail.s b/gas/testsuite/gas/riscv/option-arch-fail.s
> index 101587aee16..2ba7a16a3af 100644
> --- a/gas/testsuite/gas/riscv/option-arch-fail.s
> +++ b/gas/testsuite/gas/riscv/option-arch-fail.s
> @@ -4,6 +4,7 @@
>   .option arch, -i
>   .option arch, +e
>   .option arch, -g
> +.option arch, -c
>   .option arch, +zsubset2p0
>   .option arch, +f2p0_d2p0
>   .option arch, +
> diff --git a/gas/testsuite/gas/riscv/option-arch.s b/gas/testsuite/gas/riscv/option-arch.s
> index a37659f2c6c..8836bc153b1 100644
> --- a/gas/testsuite/gas/riscv/option-arch.s
> +++ b/gas/testsuite/gas/riscv/option-arch.s
> @@ -1,7 +1,8 @@
>   .attribute arch, "rv64ic"	# file-level, rv64ic
>   add	a0, a0, a1
>   .option push
> -.option arch, +d2p0, -c, +xvendor1p0
> +.option arch, +d2p0, +xvendor1p0
> +.option norvc
>   add	a0, a0, a1		# func-level, rv64i_d2p0_xvendor1p0
>   frcsr	a0
>   .option pop
> diff --git a/gas/testsuite/gas/riscv/shamt-32.s b/gas/testsuite/gas/riscv/shamt-32.s
> index a673f8810dc..9fa18fb38a9 100644
> --- a/gas/testsuite/gas/riscv/shamt-32.s
> +++ b/gas/testsuite/gas/riscv/shamt-32.s
> @@ -1,4 +1,4 @@
> -	.option arch, -c
> +	.option norvc
>   	slli	a0, a0, 0
>   	slli	a0, a0, 31
>   	slli	a0, a0, 32
> diff --git a/gas/testsuite/gas/riscv/shamt-64.s b/gas/testsuite/gas/riscv/shamt-64.s
> index 8878318f4a7..5ca5e6daa3d 100644
> --- a/gas/testsuite/gas/riscv/shamt-64.s
> +++ b/gas/testsuite/gas/riscv/shamt-64.s
> @@ -1,4 +1,4 @@
> -	.option arch, -c
> +	.option norvc
>   	slli	a0, a0, 0
>   	slli	a0, a0, 31
>   	slli	a0, a0, 32



More information about the Binutils mailing list