[PATCH] RISC-V: Check the Zca extension when disabling the C extension using .option norvc.
Palmer Dabbelt
palmer@rivosinc.com
Wed Oct 9 00:15:54 GMT 2024
On Tue, 08 Oct 2024 00:34:43 PDT (-0700), Nelson Chu wrote:
> Yeah thanks for the clarification. The .option norvc intents to make all
> instructions are 32 bits, although removing extensions is not the original
> intention, but for the correctness probably also needs to remove all 16-bit
> extensions, including the new zcmop/zcmt/zcmp?
>
> However, I prefer to keep the original intention of ".option rvc/norvc",
> which means allow/dis-allow 16-bit instructions, so that probably not same
> as "allow/dis-allow the standard C extension" since we will/already ratify
> more 16-bit ZC extensions, which aren't included in standard C. That
> should also works for RVC flags since it means no 16 bits instructions if
> the flag isn't raised. Therefore,
>
> .option rvc = +c,+zcmop,+zcmt,+zcmp, ...
> .option norvc = -c,-zcmop,-zcmt,-zcmt, ...
>
> cc more people, Jim, Palmer and Kito ;)
I don't really know if we can say much about the original intent, this
all comes back from the time before we had all these sub-extensions so
we really just hadn't planned for any of this complexity (hence it
breaking a lot of the early design).
Having `.option norvc` disable all 16-bit instruction does seem like
what users would want, though -- otherwise we end up in an odd spot
where there's no straight-forward way to disable opportunistic
compression, which will be a headache. Unless I'm missing something we
already disable all 16-bit instructions under `.option norvc`, so we're
safe on the implementation side here.
Unfortunately the RISC-V ASM manual docs are pretty clear that `.option
norvc` disables the C extension
== `.option`
=== `rvc`/`norvc`
This option will be deprecated soon after `.option arch` has been widely
implemented on main stream open source toolchains.
Enable/disable the C-extension for the following code region. This option is
equivalent to `.option arch, +c`/`.option arch, -c`, but widely supported by
older toolchain versions.
which doesn't match that behavior. I'm generally fine just ignoring
these RISC-V software specs where they don't make sense, though, as they
tend to be full of wacky stuff that we already ignore all over the
place.
The binutils docs are a bit vague
@item rvc
@itemx norvc
Enables or disables the generation of compressed instructions. Instructions
are opportunistically compressed by the RISC-V assembler when possible, but
sometimes this behavior is not desirable, especially when handling alignments.
but I think we can just call something like
diff --git a/gas/doc/c-riscv.texi b/gas/doc/c-riscv.texi
index 7484a71798a..45b9617b6b9 100644
--- a/gas/doc/c-riscv.texi
+++ b/gas/doc/c-riscv.texi
@@ -189,7 +189,7 @@ command-line options are respected for the bulk of the file being assembled.
@item rvc
@itemx norvc
-Enables or disables the generation of compressed instructions. Instructions
+Enables or disables the generation of 16-bit instructions. Instructions
are opportunistically compressed by the RISC-V assembler when possible, but
sometimes this behavior is not desirable, especially when handling alignments.
a fix to make the docs match the implementation. I guess the name is a
bit clunky now, but I don't think that's such a big deal.
Looks like we also have an odditiy here where `.option arch, -c` doesn't
disable Zca if it's been explicitly turned on. I'm not sure what we
want to do there.
We probably also want something along the lines of
diff --git a/gas/doc/c-riscv.texi b/gas/doc/c-riscv.texi
index 7484a71798a..870ade20582 100644
--- a/gas/doc/c-riscv.texi
+++ b/gas/doc/c-riscv.texi
@@ -214,7 +214,7 @@ Enables or disables the CSR checking.
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
+architecture string. Note that, @samp{.option arch, +c, -c} doesn't 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
but a bit friendlier. I think that whole block probably needs a
rewrite, though...
> Nelson
>
>
> On Tue, Oct 8, 2024 at 2:41 PM Andrew Waterman <andrew@sifive.com> wrote:
>
>> On Mon, Oct 7, 2024 at 11:32 PM Nelson Chu <nelson@rivosinc.com> wrote:
>> >
>> > Hi,
>> >
>> > According to the newest ISA spec,
>> https://github.com/riscv/riscv-isa-manual/blob/main/src/zc.adoc, the C is
>> actually same as Zca, Zcf and Zcd, so this change looks conflict. The
>> original code should be correct, the RVC flag means enable the C extension,
>> so NORVC means disable C, and it makes sense to disable the RVC flag. But
>> this patch also reminds us that the current toolchain needs to be updated
>> to the newest spec.
>> >
>> > 1. C needs imply Zca, Zcf and Zcd, so the implicit table needs to be
>> updated.
>> >
>> > 2. Removing extensions using ".option arch" is more complicated than the
>> first implemented. For this case, C is the set of Zca, Zcf and Zcd, so we
>> need to imply those extensions when adding C, and should also need to
>> remove those extensions when removing C. That is - riscv_update_subset
>> (&riscv_rps_as, "-c,-zca,-zcf,-zcd") for .option norvc. But unfortunately
>> it's not always work to directly remove the extension from the implicit
>> table in reverse. For example, zcd implies d and zca, but remove zcd
>> doesn't mean remove d since zcd isn't a super set of d and zca.
>> >
>> > cc Andrew in case I am wrong ;)
>>
>> Nelson's right about the ISA implications.
>>
>> ".option rvc" is of course not defined by the ISA, and the ASM manual
>> doesn't do a great job of defining it, either. But the original
>> intent was to suppress automatic instruction compression (regardless
>> of exactly which extension it was). IOW, the intent was to make it so
>> that all instructions that are notionally 32 bits long remain 32 bits
>> long. Enabling or disabling extensions was not the intent.
>>
>> >
>> > Thanks
>> > Nelson
>> >
>> >
>> > On Fri, Oct 4, 2024 at 3:12 PM Ethan Yu-Cheng Liang(梁淯程) <
>> ycl669@andestech.com> wrote:
>> >>
>> >> .option norvc disables only the C extension, not all 16-bit
>> instructions.
>> >>
>> >> Signed-off-by: Ethan Yu-Cheng Liang <ycl669@andestech.com>
>> >> ---
>> >> gas/config/tc-riscv.c | 2 +-
>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
>> >> index ef455e449b9..4ef7b8e689e 100644
>> >> --- a/gas/config/tc-riscv.c
>> >> +++ b/gas/config/tc-riscv.c
>> >> @@ -4917,7 +4917,7 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
>> >> {
>> >> riscv_update_subset (&riscv_rps_as, "-c");
>> >> riscv_set_arch_str (&riscv_rps_as.subset_list->arch_str);
>> >> - riscv_set_rvc (false);
>> >> + riscv_set_rvc (riscv_subset_supports (&riscv_rps_as, "zca"));
>> >> }
>> >> else if (strcmp (name, "pic") == 0)
>> >> riscv_opts.pic = true;
>> >> --
>> >> 2.45.2
>> >>
>> >> CONFIDENTIALITY NOTICE:
>> >>
>> >> This e-mail (and its attachments) may contain confidential and legally
>> privileged information or information protected from disclosure. If you are
>> not the intended recipient, you are hereby notified that any disclosure,
>> copying, distribution, or use of the information contained herein is
>> strictly prohibited. In this case, please immediately notify the sender by
>> return e-mail, delete the message (and any accompanying documents) and
>> destroy all printed hard copies. Thank you for your cooperation.
>> >>
>> >> Copyright ANDES TECHNOLOGY CORPORATION - All Rights Reserved.
>>
More information about the Binutils
mailing list