Re: [RFC] RISC-V: Implment H modifier for printing the next register name

[Jeff Law]

On 4/26/25 2:46 AM, Jin Ma wrote:

On Sat, 26 Apr 2025 09:59:45 +0300, Dimitar Dimitrov wrote:

On Fri, Apr 25, 2025 at 01:25:50PM +0800, Jin Ma wrote:

For RV32 inline assembly, when handling 64-bit integer data, it is
often necessary to process the lower and upper 32 bits separately.
Unfortunately, we can only output the current register name
(lower 32 bits) but not the next register name (upper 32 bits).

To address this, the modifier 'H' has been added to allow users
to handle the upper 32 bits of the data. While I believe the
modifier 'N' (representing the next register name) might be more
suitable for this functionality, 'N' is already in use.
Therefore, 'H' (representing the high register) was chosen instead.

Does anyone have any comments on this?

gcc/ChangeLog:

        * config/riscv/riscv.cc (riscv_print_operand): Add H.
        * doc/extend.texi: Document for H.

gcc/testsuite/ChangeLog:

        * gcc.target/riscv/modifier-H.c: New test.
---
  gcc/config/riscv/riscv.cc                   | 12 +++++++++++
  gcc/doc/extend.texi                         |  1 +
  gcc/testsuite/gcc.target/riscv/modifier-H.c | 22 +++++++++++++++++++++
  3 files changed, 35 insertions(+)
  create mode 100644 gcc/testsuite/gcc.target/riscv/modifier-H.c

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index bad59e248d0..4ef96532f35 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -6879,6 +6879,7 @@ riscv_asm_output_opcode (FILE *asm_out_file, const char 
*p)
     'T'        Print shift-index of inverted single-bit mask OP.
     '~'        Print w if TARGET_64BIT is true; otherwise not print anything.
     'N'  Print register encoding as integer (0-31).
+   'H'  Print the name of the high register for OP, which is the next register.

Note please keep this list and the list in riscv.md in sync. */ @@ -7174,6 +7175,17 @@ riscv_print_operand (FILE *file, rtx op, int letter)

        asm_fprintf (file, "%u", (regno - offset));
        break;
        }
+    case 'H':
+      {
+       if (!REG_P (op))
+         {
+           output_operand_lossage ("modifier 'H' require register operand");
+           break;
+         }

Given the intended usage, does it make sense to limit this modifier only
to regular integer registers? Example:

   if (REGNO (op) > 31 )
     {
       output_operand_lossage ("modifier 'H' is for integer registers only");
       break;
     }
   if (REGNO (op) == 31 )
     {
       output_operand_lossage ("modifier 'H' cannot be applied to R31");
       break;
     }

Is it an error to apply H modifier to R0?

If not, would you consider rejecting the last HW register:
   if (REGNO (op) >= FIRST_PSEUDO_REGISTER - 1 )
     {
       output_operand_lossage ("modifier 'H' cannot be applied to last HW 
register");
       break;
     }

Thanks. These are excellent review comments, and I will incorporate them into 
the next version.

Does anyone else have additional suggestions?

I think with Dimitar's suggestions this patch is OK for the trunk.

Thanks!
jeff