RISC-V: widen LEB128 support
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_binutils_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_binutils_check--master-arm |
success
|
Test passed
|
Commit Message
Do away with at least one of the limitations - all other targets permit
multiple values to be specified with a single directive. Re-arrange the
logic further to also overcome an internal error in
riscv_insert_uleb128_fixes(), as e.g. observed by the all/sleb128-2
testcase. This way there's also no need to parse expressions twice,
thus also not raising the same diagnostics (if any) twice.
Note how this addresses a pre-existing XFAIL (where the comment wasn't
really applicable either for RISC-V).
Also update documentation, also to mention that differences between
symbols may be used with .uleb128 (albeit I'm uncertain whether there
are limitations).
---
Imo even for XFAILed testcases an internal error would better yield a
real test failure.
Comments
Okay, looks good, thanks for fixing the xfail of the testcase :-)
Nelson
On Wed, Jan 22, 2025 at 4:55 PM Jan Beulich <jbeulich@suse.com> wrote:
> Do away with at least one of the limitations - all other targets permit
> multiple values to be specified with a single directive. Re-arrange the
> logic further to also overcome an internal error in
> riscv_insert_uleb128_fixes(), as e.g. observed by the all/sleb128-2
> testcase. This way there's also no need to parse expressions twice,
> thus also not raising the same diagnostics (if any) twice.
>
> Note how this addresses a pre-existing XFAIL (where the comment wasn't
> really applicable either for RISC-V).
>
> Also update documentation, also to mention that differences between
> symbols may be used with .uleb128 (albeit I'm uncertain whether there
> are limitations).
> ---
> Imo even for XFAILed testcases an internal error would better yield a
> real test failure.
>
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -5441,19 +5441,22 @@ riscv_elf_final_processing (void)
> static void
> s_riscv_leb128 (int sign)
> {
> - expressionS exp;
> - char *save_in = input_line_pointer;
> + do
> + {
> + expressionS exp;
>
> - expression (&exp);
> - if (sign && exp.X_op != O_constant)
> - as_bad (_("non-constant .sleb128 is not supported"));
> - else if (!sign && exp.X_op != O_constant && exp.X_op != O_subtract)
> - as_bad (_(".uleb128 only supports constant or subtract expressions"));
> + expression (&exp);
> + if (sign && exp.X_op != O_constant)
> + as_bad (_("non-constant .sleb128 is not supported"));
> + else if (!sign && exp.X_op != O_constant && exp.X_op != O_subtract)
> + as_bad (_(".uleb128 only supports constant or subtract
> expressions"));
> + else
> + emit_leb128_expr (&exp, sign);
> + }
> + while (*input_line_pointer++ == ',');
>
> + input_line_pointer--;
> demand_empty_rest_of_line ();
> -
> - input_line_pointer = save_in;
> - return s_leb128 (sign);
> }
>
> /* Parse the .insn directive. There are three formats,
> --- a/gas/doc/c-riscv.texi
> +++ b/gas/doc/c-riscv.texi
> @@ -139,12 +139,13 @@ meant to be used by the compiler in shar
> thread local variables.
>
> @cindex LEB128 directives
> -@item .uleb128 @var{value}
> -@itemx .sleb128 @var{value}
> -Emits a signed or unsigned LEB128 value at the current position. This
> only
> +@item .uleb128 @var{values}
> +@itemx .sleb128 @var{values}
> +Emits signed or unsigned LEB128 values at the current position. This only
> accepts constant expressions, because symbol addresses can change with
> relaxation, and we don't support relocations to modify LEB128 values at
> link
> -time.
> +time. An exception are differences between symbols, which may be used
> with
> +@code{.uleb128}.
>
> @cindex Option directive
> @cindex @code{option} directive
> --- a/gas/testsuite/gas/elf/dwarf2-6.d
> +++ b/gas/testsuite/gas/elf/dwarf2-6.d
> @@ -2,7 +2,7 @@
> #readelf: -wlL
> #name: DWARF2 6
> # These targets either do not support or do not evaluate the subtraction
> of symbols at assembly time.
> -#xfail: cr16-* crx-* riscv*-*
> +#xfail: cr16-* crx-*
>
> Raw dump of debug contents of section .debug_line:
>
>
@@ -5441,19 +5441,22 @@ riscv_elf_final_processing (void)
static void
s_riscv_leb128 (int sign)
{
- expressionS exp;
- char *save_in = input_line_pointer;
+ do
+ {
+ expressionS exp;
- expression (&exp);
- if (sign && exp.X_op != O_constant)
- as_bad (_("non-constant .sleb128 is not supported"));
- else if (!sign && exp.X_op != O_constant && exp.X_op != O_subtract)
- as_bad (_(".uleb128 only supports constant or subtract expressions"));
+ expression (&exp);
+ if (sign && exp.X_op != O_constant)
+ as_bad (_("non-constant .sleb128 is not supported"));
+ else if (!sign && exp.X_op != O_constant && exp.X_op != O_subtract)
+ as_bad (_(".uleb128 only supports constant or subtract expressions"));
+ else
+ emit_leb128_expr (&exp, sign);
+ }
+ while (*input_line_pointer++ == ',');
+ input_line_pointer--;
demand_empty_rest_of_line ();
-
- input_line_pointer = save_in;
- return s_leb128 (sign);
}
/* Parse the .insn directive. There are three formats,
@@ -139,12 +139,13 @@ meant to be used by the compiler in shar
thread local variables.
@cindex LEB128 directives
-@item .uleb128 @var{value}
-@itemx .sleb128 @var{value}
-Emits a signed or unsigned LEB128 value at the current position. This only
+@item .uleb128 @var{values}
+@itemx .sleb128 @var{values}
+Emits signed or unsigned LEB128 values at the current position. This only
accepts constant expressions, because symbol addresses can change with
relaxation, and we don't support relocations to modify LEB128 values at link
-time.
+time. An exception are differences between symbols, which may be used with
+@code{.uleb128}.
@cindex Option directive
@cindex @code{option} directive
@@ -2,7 +2,7 @@
#readelf: -wlL
#name: DWARF2 6
# These targets either do not support or do not evaluate the subtraction of symbols at assembly time.
-#xfail: cr16-* crx-* riscv*-*
+#xfail: cr16-* crx-*
Raw dump of debug contents of section .debug_line: