RISC-V: Correct as --dump-config output for --target=rv64*
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
gas/ChangeLog:
* config/tc-riscv.c (riscv_target_format): Correct as
--dump-config output for --target=rv64*.
* testsuite/gas/riscv/elf32-bigriscv_dump.d: New test.
* testsuite/gas/riscv/elf32-littleriscv_dump.d: New test.
* testsuite/gas/riscv/elf64-bigriscv_dump.d: New test.
* testsuite/gas/riscv/elf64-littleriscv_dump.d: New test.
* testsuite/gas/riscv/linux32-bigriscv_dump.d: New test.
* testsuite/gas/riscv/linux32-littleriscv_dump.d: New test.
* testsuite/gas/riscv/linux64-bigriscv_dump.d: New test.
* testsuite/gas/riscv/linux64-littleriscv_dump.d: New test.
Signed-off-by: Xiao Zeng <zengxiao@eswincomputing.com>
---
gas/config/tc-riscv.c | 12 ++++++++++++
gas/testsuite/gas/riscv/elf32-bigriscv_dump.d | 6 ++++++
gas/testsuite/gas/riscv/elf32-littleriscv_dump.d | 6 ++++++
gas/testsuite/gas/riscv/elf64-bigriscv_dump.d | 6 ++++++
gas/testsuite/gas/riscv/elf64-littleriscv_dump.d | 6 ++++++
gas/testsuite/gas/riscv/linux32-bigriscv_dump.d | 6 ++++++
gas/testsuite/gas/riscv/linux32-littleriscv_dump.d | 6 ++++++
gas/testsuite/gas/riscv/linux64-bigriscv_dump.d | 6 ++++++
gas/testsuite/gas/riscv/linux64-littleriscv_dump.d | 6 ++++++
9 files changed, 60 insertions(+)
create mode 100644 gas/testsuite/gas/riscv/elf32-bigriscv_dump.d
create mode 100644 gas/testsuite/gas/riscv/elf32-littleriscv_dump.d
create mode 100644 gas/testsuite/gas/riscv/elf64-bigriscv_dump.d
create mode 100644 gas/testsuite/gas/riscv/elf64-littleriscv_dump.d
create mode 100644 gas/testsuite/gas/riscv/linux32-bigriscv_dump.d
create mode 100644 gas/testsuite/gas/riscv/linux32-littleriscv_dump.d
create mode 100644 gas/testsuite/gas/riscv/linux64-bigriscv_dump.d
create mode 100644 gas/testsuite/gas/riscv/linux64-littleriscv_dump.d
Comments
On Mon, Jan 6, 2025 at 4:22 PM Xiao Zeng <zengxiao@eswincomputing.com>
wrote:
> gas/ChangeLog:
>
> * config/tc-riscv.c (riscv_target_format): Correct as
> --dump-config output for --target=rv64*.
> * testsuite/gas/riscv/elf32-bigriscv_dump.d: New test.
> * testsuite/gas/riscv/elf32-littleriscv_dump.d: New test.
> * testsuite/gas/riscv/elf64-bigriscv_dump.d: New test.
> * testsuite/gas/riscv/elf64-littleriscv_dump.d: New test.
> * testsuite/gas/riscv/linux32-bigriscv_dump.d: New test.
> * testsuite/gas/riscv/linux32-littleriscv_dump.d: New test.
> * testsuite/gas/riscv/linux64-bigriscv_dump.d: New test.
> * testsuite/gas/riscv/linux64-littleriscv_dump.d: New test.
>
> Signed-off-by: Xiao Zeng <zengxiao@eswincomputing.com>
> ---
> gas/config/tc-riscv.c | 12 ++++++++++++
> gas/testsuite/gas/riscv/elf32-bigriscv_dump.d | 6 ++++++
> gas/testsuite/gas/riscv/elf32-littleriscv_dump.d | 6 ++++++
> gas/testsuite/gas/riscv/elf64-bigriscv_dump.d | 6 ++++++
> gas/testsuite/gas/riscv/elf64-littleriscv_dump.d | 6 ++++++
> gas/testsuite/gas/riscv/linux32-bigriscv_dump.d | 6 ++++++
> gas/testsuite/gas/riscv/linux32-littleriscv_dump.d | 6 ++++++
> gas/testsuite/gas/riscv/linux64-bigriscv_dump.d | 6 ++++++
> gas/testsuite/gas/riscv/linux64-littleriscv_dump.d | 6 ++++++
> 9 files changed, 60 insertions(+)
> create mode 100644 gas/testsuite/gas/riscv/elf32-bigriscv_dump.d
> create mode 100644 gas/testsuite/gas/riscv/elf32-littleriscv_dump.d
> create mode 100644 gas/testsuite/gas/riscv/elf64-bigriscv_dump.d
> create mode 100644 gas/testsuite/gas/riscv/elf64-littleriscv_dump.d
> create mode 100644 gas/testsuite/gas/riscv/linux32-bigriscv_dump.d
> create mode 100644 gas/testsuite/gas/riscv/linux32-littleriscv_dump.d
> create mode 100644 gas/testsuite/gas/riscv/linux64-bigriscv_dump.d
> create mode 100644 gas/testsuite/gas/riscv/linux64-littleriscv_dump.d
>
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index a915c8b4995..35d19ac4e5d 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -734,6 +734,18 @@ riscv_check_mapping_symbols (bfd *abfd
> ATTRIBUTE_UNUSED,
> const char *
> riscv_target_format (void)
> {
> + /* When as --dump-config is used, xlen is not initialized, resulting in
> + bfd-target being set to elf32-*riscv even with --target=rv64*. */
> + if (xlen == 0)
> + {
> + if (strcmp (default_arch, "riscv32") == 0)
> + xlen = 32;
> + else if (strcmp (default_arch, "riscv64") == 0)
> + xlen = 64;
> + else
> + as_bad ("unknown default architecture `%s'", default_arch);
> + }
>
Assuming this is the v2 patch, the as_abort is better here as Jan had
suggested.
As for the test cases ...
> diff --git a/gas/testsuite/gas/riscv/elf32-bigriscv_dump.d
> b/gas/testsuite/gas/riscv/elf32-bigriscv_dump.d
> new file mode 100644
> index 00000000000..28cbef31666
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/elf32-bigriscv_dump.d
> @@ -0,0 +1,6 @@
> +#as: -mbig-endian -dump
> +
> +alias = riscv32-unknown-elf
> +canonical = riscv32-unknown-elf
> +cpu-type = riscv32
> +bfd-target = elf32-bigriscv
> diff --git a/gas/testsuite/gas/riscv/elf64-bigriscv_dump.d
> b/gas/testsuite/gas/riscv/elf64-bigriscv_dump.d
> new file mode 100644
> index 00000000000..9942545af0f
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/elf64-bigriscv_dump.d
> @@ -0,0 +1,6 @@
> +#as: -mbig-endian -dump
> +
> +alias = riscv64-unknown-elf
> +canonical = riscv64-unknown-elf
> +cpu-type = riscv64
> +bfd-target = elf64-bigriscv
>
Take these two as examples. I don't see any change in the
gas/testsuite/gas/riscv/riscv.exp, so I think the elf64-xxx_dump test cases
will fail for riscv32-unknown-[elf|linux]-as, and
the elf32-xxx_dump test cases will fail for
riscv64-unknown-[elf|linux]-as, then the binutils regressions of
riscv-gnu-toolchain should fail. Btw, the config alias also may make test
cases broken, so...
... There are probably two solutions,
1. Add similar istarget checks as i386 and other targets,
if { [istarget riscv32-*-elf] } then {
run_dump_test "elf32-bigriscv_dump"
run_dump_test "elf32-littleriscv_dump"
} elseif { [istarget riscv64-*-elf] } then {
...
} ...
But this makes things complicated, so we can give -march=rv32i or rv64i in
the testcase directly ...
2. Only two test cases needed,
% cat dump-config-riscv64
#as: -march=rv64i -dump-config
#...
bfd-target = (elf|linux)64-(big|little)riscv
% cat dump-config-riscv32
#as: -march=rv32i -dump-config
#...
bfd-target = (elf|linux)32-(big|little)riscv
I will choose the second one if you ask me.
Thanks
Nelson
2025-01-07 15:40 Nelson Chu <nelson@rivosinc.com> wrote:
>
>On Mon, Jan 6, 2025 at 4:22 PM Xiao Zeng <zengxiao@eswincomputing.com>
>wrote:
>
>> gas/ChangeLog:
>>
>> * config/tc-riscv.c (riscv_target_format): Correct as
>> --dump-config output for --target=rv64*.
>> * testsuite/gas/riscv/elf32-bigriscv_dump.d: New test.
>> * testsuite/gas/riscv/elf32-littleriscv_dump.d: New test.
>> * testsuite/gas/riscv/elf64-bigriscv_dump.d: New test.
>> * testsuite/gas/riscv/elf64-littleriscv_dump.d: New test.
>> * testsuite/gas/riscv/linux32-bigriscv_dump.d: New test.
>> * testsuite/gas/riscv/linux32-littleriscv_dump.d: New test.
>> * testsuite/gas/riscv/linux64-bigriscv_dump.d: New test.
>> * testsuite/gas/riscv/linux64-littleriscv_dump.d: New test.
>>
>> Signed-off-by: Xiao Zeng <zengxiao@eswincomputing.com>
>> ---
>> gas/config/tc-riscv.c | 12 ++++++++++++
>> gas/testsuite/gas/riscv/elf32-bigriscv_dump.d | 6 ++++++
>> gas/testsuite/gas/riscv/elf32-littleriscv_dump.d | 6 ++++++
>> gas/testsuite/gas/riscv/elf64-bigriscv_dump.d | 6 ++++++
>> gas/testsuite/gas/riscv/elf64-littleriscv_dump.d | 6 ++++++
>> gas/testsuite/gas/riscv/linux32-bigriscv_dump.d | 6 ++++++
>> gas/testsuite/gas/riscv/linux32-littleriscv_dump.d | 6 ++++++
>> gas/testsuite/gas/riscv/linux64-bigriscv_dump.d | 6 ++++++
>> gas/testsuite/gas/riscv/linux64-littleriscv_dump.d | 6 ++++++
>> 9 files changed, 60 insertions(+)
>> create mode 100644 gas/testsuite/gas/riscv/elf32-bigriscv_dump.d
>> create mode 100644 gas/testsuite/gas/riscv/elf32-littleriscv_dump.d
>> create mode 100644 gas/testsuite/gas/riscv/elf64-bigriscv_dump.d
>> create mode 100644 gas/testsuite/gas/riscv/elf64-littleriscv_dump.d
>> create mode 100644 gas/testsuite/gas/riscv/linux32-bigriscv_dump.d
>> create mode 100644 gas/testsuite/gas/riscv/linux32-littleriscv_dump.d
>> create mode 100644 gas/testsuite/gas/riscv/linux64-bigriscv_dump.d
>> create mode 100644 gas/testsuite/gas/riscv/linux64-littleriscv_dump.d
>>
>> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
>> index a915c8b4995..35d19ac4e5d 100644
>> --- a/gas/config/tc-riscv.c
>> +++ b/gas/config/tc-riscv.c
>> @@ -734,6 +734,18 @@ riscv_check_mapping_symbols (bfd *abfd
>> ATTRIBUTE_UNUSED,
>> const char *
>> riscv_target_format (void)
>> {
>> + /* When as --dump-config is used, xlen is not initialized, resulting in
>> + bfd-target being set to elf32-*riscv even with --target=rv64*. */
>> + if (xlen == 0)
>> + {
>> + if (strcmp (default_arch, "riscv32") == 0)
>> + xlen = 32;
>> + else if (strcmp (default_arch, "riscv64") == 0)
>> + xlen = 64;
>> + else
>> + as_bad ("unknown default architecture `%s'", default_arch);
>> + }
>>
>
>Assuming this is the v2 patch, the as_abort is better here as Jan had
>suggested.
Thank you, Jan, for your code review comments.
>
>As for the test cases ...
>
>
>> diff --git a/gas/testsuite/gas/riscv/elf32-bigriscv_dump.d
>> b/gas/testsuite/gas/riscv/elf32-bigriscv_dump.d
>> new file mode 100644
>> index 00000000000..28cbef31666
>> --- /dev/null
>> +++ b/gas/testsuite/gas/riscv/elf32-bigriscv_dump.d
>> @@ -0,0 +1,6 @@
>> +#as: -mbig-endian -dump
>> +
>> +alias = riscv32-unknown-elf
>> +canonical = riscv32-unknown-elf
>> +cpu-type = riscv32
>> +bfd-target = elf32-bigriscv
>> diff --git a/gas/testsuite/gas/riscv/elf64-bigriscv_dump.d
>> b/gas/testsuite/gas/riscv/elf64-bigriscv_dump.d
>> new file mode 100644
>> index 00000000000..9942545af0f
>> --- /dev/null
>> +++ b/gas/testsuite/gas/riscv/elf64-bigriscv_dump.d
>> @@ -0,0 +1,6 @@
>> +#as: -mbig-endian -dump
>> +
>> +alias = riscv64-unknown-elf
>> +canonical = riscv64-unknown-elf
>> +cpu-type = riscv64
>> +bfd-target = elf64-bigriscv
>>
>
>Take these two as examples. I don't see any change in the
>gas/testsuite/gas/riscv/riscv.exp, so I think the elf64-xxx_dump test cases
>will fail for riscv32-unknown-[elf|linux]-as, and
> the elf32-xxx_dump test cases will fail for
>riscv64-unknown-[elf|linux]-as, then the binutils regressions of
>riscv-gnu-toolchain should fail. Btw, the config alias also may make test
>cases broken, so...
Yes, based on this, I find it quite challenging to design appropriate test cases.
>
>... There are probably two solutions,
>1. Add similar istarget checks as i386 and other targets,
>if { [istarget riscv32-*-elf] } then {
>run_dump_test "elf32-bigriscv_dump"
>run_dump_test "elf32-littleriscv_dump"
>} elseif { [istarget riscv64-*-elf] } then {
>...
>} ...
>
>But this makes things complicated, so we can give -march=rv32i or rv64i in
>the testcase directly ...
>
>2. Only two test cases needed,
>% cat dump-config-riscv64
>#as: -march=rv64i -dump-config
>
>#...
>bfd-target = (elf|linux)64-(big|little)riscv
>
>% cat dump-config-riscv32
>#as: -march=rv32i -dump-config
>
>#...
>bfd-target = (elf|linux)32-(big|little)riscv
>
>I will choose the second one if you ask me.
Yes, I would make the same choice as you.
The code reviews are all very clear.
Could I kindly ask Nelson to make these changes and push them to the trunk?
Thank you!
>
>Thanks
>Nelson
Thanks
Xiao Zeng
@@ -734,6 +734,18 @@ riscv_check_mapping_symbols (bfd *abfd ATTRIBUTE_UNUSED,
const char *
riscv_target_format (void)
{
+ /* When as --dump-config is used, xlen is not initialized, resulting in
+ bfd-target being set to elf32-*riscv even with --target=rv64*. */
+ if (xlen == 0)
+ {
+ if (strcmp (default_arch, "riscv32") == 0)
+ xlen = 32;
+ else if (strcmp (default_arch, "riscv64") == 0)
+ xlen = 64;
+ else
+ as_bad ("unknown default architecture `%s'", default_arch);
+ }
+
if (target_big_endian)
return xlen == 64 ? "elf64-bigriscv" : "elf32-bigriscv";
else
new file mode 100644
@@ -0,0 +1,6 @@
+#as: -mbig-endian -dump
+
+alias = riscv32-unknown-elf
+canonical = riscv32-unknown-elf
+cpu-type = riscv32
+bfd-target = elf32-bigriscv
new file mode 100644
@@ -0,0 +1,6 @@
+#as: -mlittle-endian -dump
+
+alias = riscv32-unknown-elf
+canonical = riscv32-unknown-elf
+cpu-type = riscv32
+bfd-target = elf32-littleriscv
new file mode 100644
@@ -0,0 +1,6 @@
+#as: -mbig-endian -dump
+
+alias = riscv64-unknown-elf
+canonical = riscv64-unknown-elf
+cpu-type = riscv64
+bfd-target = elf64-bigriscv
new file mode 100644
@@ -0,0 +1,6 @@
+#as: -mlittle-endian -dump
+
+alias = riscv64-unknown-elf
+canonical = riscv64-unknown-elf
+cpu-type = riscv64
+bfd-target = elf64-littleriscv
new file mode 100644
@@ -0,0 +1,6 @@
+#as: -mbig-endian -dump
+
+alias = riscv32-unknown-linux-gnu
+canonical = riscv32-unknown-linux-gnu
+cpu-type = riscv32
+bfd-target = elf32-bigriscv
new file mode 100644
@@ -0,0 +1,6 @@
+#as: -mlittle-endian -dump
+
+alias = riscv32-unknown-linux-gnu
+canonical = riscv32-unknown-linux-gnu
+cpu-type = riscv32
+bfd-target = elf32-littleriscv
new file mode 100644
@@ -0,0 +1,6 @@
+#as: -mbig-endian -dump
+
+alias = riscv64-unknown-linux-gnu
+canonical = riscv64-unknown-linux-gnu
+cpu-type = riscv64
+bfd-target = elf64-bigriscv
new file mode 100644
@@ -0,0 +1,6 @@
+#as: -mlittle-endian -dump
+
+alias = riscv64-unknown-linux-gnu
+canonical = riscv64-unknown-linux-gnu
+cpu-type = riscv64
+bfd-target = elf64-littleriscv