.option norvc disables all 16-bit instructions in binutils

yclwlcy commented 2 weeks ago

According to the RISC-V Assembly Programmer's Manual, .option norvc is equivalent to .option arch, -c. However, .option norvc disables all 16-bit instructions in binutils. Should the binutils implementation be modified to align with the spec?

Thanks!

pz9115 commented 2 weeks ago

I think Nelson had a solution in Binutils part, please check the follow issue and PR, thanks.

https://github.com/riscv-collab/riscv-gnu-toolchain/issues/445 https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/393

TommyMurphyTM1234 commented 2 weeks ago

Shouldn't there be an issue logged upstream against Binutils (ld?) for this? And maybe against the LLVM linker too? And other linkers such as Mold and Gold? Or does it need to be addressed in the ABI spec first?

cmuellner commented 2 weeks ago

The ASM manual states:

NOTE: There is a difference between .option rvc/.option norvc and .option arch, +c/.option arch, -c. The latter won't set EF_RISCV_RVC in the ELF flags.

So, the ASM manual does not state that .option arch, -c and .option norvc are equivalent. And the documented difference is the reported difference.

TommyMurphyTM1234 commented 2 weeks ago

Isn't this issue a duplicate of this one:

https://github.com/riscv-collab/riscv-gnu-toolchain/issues/445

Do we actually need both issues open or can they be "merged"?

yclwlcy commented 2 weeks ago

Thanks for the responses. I’d like to clarify my question. My primary concern is about the assembler (not the linker). Consider the following assembly code examples.

Example 1:

.attribute arch, "rv64ic_zcb"
.option arch, -c
c.lbu x8, (x15) # Zcb instruction

Example 2:

.attribute arch, "rv64ic_zcb"
.option norvc
c.lbu x8, (x15) # Zcb instruction

Example 1 can be assembled, but Example 2 cannot be assembled.

TommyMurphyTM1234 commented 2 weeks ago

Hi @yclwlcy - so the issue is that .option norvc might have been intended to disable C extension 16-bit compressed instructions but, in practice, actually disables ALL 16-bit instructions including those that are part of other non-C RISC-V extensions (e.g. Zcb)?

I presume that .option norvc is already so widely employed that just saying "don't use it" isn't a practical solution?

yclwlcy commented 2 weeks ago

Hi @yclwlcy - so the issue is that .option norvc might have been intended to disable C extension 16-bit compressed instructions but, in practice, actually disables ALL 16-bit instructions including those that are part of other non-C RISC-V extensions (e.g. Zcb)?

Yes.

I presume that .option norvc is already so widely employed that just saying "don't use it" isn't a practical solution?

I suggest modifying the behavior of .option norvc as follows.

   else if (strcmp (name, "norvc") == 0)
     {
       riscv_update_subset (&riscv_rps_as, "-c");
       riscv_set_arch_str (&riscv_rps_as.subset_list->arch_str);
       riscv_set_rvc (false);
+      if (riscv_subset_supports (&riscv_rps_as, "zca"))
+        riscv_set_rvc (true);
     }

TommyMurphyTM1234 commented 2 weeks ago

Hi @yclwlcy - so the issue is that .option norvc might have been intended to disable C extension 16-bit compressed instructions but, in practice, actually disables ALL 16-bit instructions including those that are part of other non-C RISC-V extensions (e.g. Zcb)?

Yes.

I presume that .option norvc is already so widely employed that just saying "don't use it" isn't a practical solution?

I suggest modifying the behavior of .option norvc as follows.
   else if (strcmp (name, "norvc") == 0)
     {
       riscv_update_subset (&riscv_rps_as, "-c");
       riscv_set_arch_str (&riscv_rps_as.subset_list->arch_str);
       riscv_set_rvc (false);
+      if (riscv_subset_supports (&riscv_rps_as, "zca"))
+        riscv_set_rvc (true);
     }

Apologies if any of these are dumb questions/comments but...

I guess that the code snippet above comes from here?
- https://github.com/bminor/binutils-gdb/blob/5cca20f614694ba6af3c10ca5bb93c32cdb9522b/gas/config/tc-riscv.c#L4862-L4867
Is this a good solution given that it means that .option norvc silently becomes a nop if the target (?) happens to support Zca and this may come as an unpleasant surprise to the user?
Are there, or might there be in the future, other non C extensions that use 16-bit instruction encodings meaning that the check above would have to be updated to check for them too?

yclwlcy commented 2 weeks ago

I guess that the code snippet above comes from here?

https://github.com/bminor/binutils-gdb/blob/5cca20f614694ba6af3c10ca5bb93c32cdb9522b/gas/config/tc-riscv.c#L4862-L4867

Yes.

Is this a good solution given that it means that .option norvc silently becomes a nop if the target (?) happens to support Zca and this may come as an unpleasant surprise to the user?

Yes, just like .option arch, -c.

Are there, or might there be in the future, other non C extensions that use 16-bit instruction encodings meaning that the check above would have to be updated to check for them too?

Just let them imply zca, like existing extensions including 16-bit instructions.

TommyMurphyTM1234 commented 2 weeks ago

I guess that the code snippet above comes from here?

https://github.com/bminor/binutils-gdb/blob/5cca20f614694ba6af3c10ca5bb93c32cdb9522b/gas/config/tc-riscv.c#L4862-L4867

Yes.

Thanks.

Is this a good solution given that it means that .option norvc silently becomes a nop if the target (?) happens to support Zca and this may come as an unpleasant surprise to the user?

Yes, just like .option arch, -c.

Sorry - where is it specified and/or implemented that .option arch, -c becomes a nop if the target supports Zca?

Are there, or might there be in the future, other non C extensions that use 16-bit instruction encodings meaning that the check above would have to be updated to check for them too?

Just let them imply zca, like existing extensions including 16-bit instructions.

Sorry - I don't understand what you mean. Maybe you can clarify?

BTW - the link in the original post is broken:

According to the RISC-V Assembly Programmer's Manual,

yclwlcy commented 2 weeks ago

Is this a good solution given that it means that .option norvc silently becomes a nop if the target (?) happens to support Zca and this may come as an unpleasant surprise to the user?

Yes, just like .option arch, -c.

Sorry - where is it specified and/or implemented that .option arch, -c becomes a nop if the target supports Zca?

In binutils, C and Zca can be set at the same time. If we remove C from rv32ic_zca using .option arch, -c, we will get rv32i_zca, which means .option arch, -c actually has no effect.

Are there, or might there be in the future, other non C extensions that use 16-bit instruction encodings meaning that the check above would have to be updated to check for them too?

Just let them imply zca, like existing extensions including 16-bit instructions.

Sorry - I don't understand what you mean. Maybe you can clarify?

In binutils, all the extensions which contain 16-bit instructions imply zca. So if we add a new extension which contains 16-bit instructions, we can simply let it imply zca instead of updating the checking mechanism.

BTW - the link in the original post is broken:

According to the RISC-V Assembly Programmer's Manual,

It seems to be moved to RISC-V Assembly Programmer's Manual. Thanks for reminding!

riscv-collab / riscv-gnu-toolchain

.option norvc disables all 16-bit instructions in binutils #1542