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?
Best regards,
Jin
