* [PATCH] RISC-V: Fix register class subset checks for CLASS_MAX_NREGS
@ 2021-11-02 15:17 Maciej W. Rozycki
2021-11-02 17:00 ` Kito Cheng
0 siblings, 1 reply; 3+ messages in thread
From: Maciej W. Rozycki @ 2021-11-02 15:17 UTC (permalink / raw)
To: gcc-patches; +Cc: Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson
Fix the register class subset checks in the determination of the maximum
number of consecutive registers needed to hold a value of a given mode.
The number depends on whether a register is a general-purpose or a
floating-point register, so check whether the register class requested
is a subset (argument 1 to `reg_class_subset_p') rather than superset
(argument 2) of GR_REGS or FP_REGS class respectively.
gcc/
* config/riscv/riscv.c (riscv_class_max_nregs): Swap the
arguments to `reg_class_subset_p'.
---
Hi,
This looks like a thinko to me, but please double-check I have not become
confused myself.
My understanding is that current code works, because the SIBCALL_REGS and
JALR_REGS classes are only ever used for addresses, which won't ever span
multiple registers, and then the only class other than ALL_REGS that
inludes floating-point registers is FP_REGS. Therefore the hook will only
ever see either GR_REGS or FP_REGS as the class enquired about and
consequently the result of the `reg_class_subset_p' calls will be the same
regardless of the order of the arguments chosen. We should be getting the
semantics right regardless.
Regression-tested with the `riscv64-linux-gnu' target. OK to apply?
Maciej
---
gcc/config/riscv/riscv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
gcc-riscv-class-max-nregs.diff
Index: gcc/gcc/config/riscv/riscv.c
===================================================================
--- gcc.orig/gcc/config/riscv/riscv.c
+++ gcc/gcc/config/riscv/riscv.c
@@ -4811,10 +4811,10 @@ riscv_modes_tieable_p (machine_mode mode
static unsigned char
riscv_class_max_nregs (reg_class_t rclass, machine_mode mode)
{
- if (reg_class_subset_p (FP_REGS, rclass))
+ if (reg_class_subset_p (rclass, FP_REGS))
return riscv_hard_regno_nregs (FP_REG_FIRST, mode);
- if (reg_class_subset_p (GR_REGS, rclass))
+ if (reg_class_subset_p (rclass, GR_REGS))
return riscv_hard_regno_nregs (GP_REG_FIRST, mode);
return 0;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] RISC-V: Fix register class subset checks for CLASS_MAX_NREGS
2021-11-02 15:17 [PATCH] RISC-V: Fix register class subset checks for CLASS_MAX_NREGS Maciej W. Rozycki
@ 2021-11-02 17:00 ` Kito Cheng
2021-11-03 17:07 ` Maciej W. Rozycki
0 siblings, 1 reply; 3+ messages in thread
From: Kito Cheng @ 2021-11-02 17:00 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: GCC Patches, Andrew Waterman
Hi Maciej:
LGTM, My first impression of this patch is also confusing about the
ordering of arguments for reg_class_subset_p, but after I double
checked that with gdb and read the comment above the
reg_class_subset_p, I think this change is right.
Thanks!
On Tue, Nov 2, 2021 at 11:17 PM Maciej W. Rozycki <macro@embecosm.com> wrote:
>
> Fix the register class subset checks in the determination of the maximum
> number of consecutive registers needed to hold a value of a given mode.
>
> The number depends on whether a register is a general-purpose or a
> floating-point register, so check whether the register class requested
> is a subset (argument 1 to `reg_class_subset_p') rather than superset
> (argument 2) of GR_REGS or FP_REGS class respectively.
>
> gcc/
> * config/riscv/riscv.c (riscv_class_max_nregs): Swap the
> arguments to `reg_class_subset_p'.
> ---
> Hi,
>
> This looks like a thinko to me, but please double-check I have not become
> confused myself.
>
> My understanding is that current code works, because the SIBCALL_REGS and
> JALR_REGS classes are only ever used for addresses, which won't ever span
> multiple registers, and then the only class other than ALL_REGS that
> inludes floating-point registers is FP_REGS. Therefore the hook will only
> ever see either GR_REGS or FP_REGS as the class enquired about and
> consequently the result of the `reg_class_subset_p' calls will be the same
> regardless of the order of the arguments chosen. We should be getting the
> semantics right regardless.
>
> Regression-tested with the `riscv64-linux-gnu' target. OK to apply?
>
> Maciej
> ---
> gcc/config/riscv/riscv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> gcc-riscv-class-max-nregs.diff
> Index: gcc/gcc/config/riscv/riscv.c
> ===================================================================
> --- gcc.orig/gcc/config/riscv/riscv.c
> +++ gcc/gcc/config/riscv/riscv.c
> @@ -4811,10 +4811,10 @@ riscv_modes_tieable_p (machine_mode mode
> static unsigned char
> riscv_class_max_nregs (reg_class_t rclass, machine_mode mode)
> {
> - if (reg_class_subset_p (FP_REGS, rclass))
> + if (reg_class_subset_p (rclass, FP_REGS))
> return riscv_hard_regno_nregs (FP_REG_FIRST, mode);
>
> - if (reg_class_subset_p (GR_REGS, rclass))
> + if (reg_class_subset_p (rclass, GR_REGS))
> return riscv_hard_regno_nregs (GP_REG_FIRST, mode);
>
> return 0;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] RISC-V: Fix register class subset checks for CLASS_MAX_NREGS
2021-11-02 17:00 ` Kito Cheng
@ 2021-11-03 17:07 ` Maciej W. Rozycki
0 siblings, 0 replies; 3+ messages in thread
From: Maciej W. Rozycki @ 2021-11-03 17:07 UTC (permalink / raw)
To: Kito Cheng; +Cc: GCC Patches, Andrew Waterman
On Wed, 3 Nov 2021, Kito Cheng wrote:
> LGTM, My first impression of this patch is also confusing about the
> ordering of arguments for reg_class_subset_p, but after I double
> checked that with gdb and read the comment above the
> reg_class_subset_p, I think this change is right.
Applied now, thanks for your review.
Maciej
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-11-03 17:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-02 15:17 [PATCH] RISC-V: Fix register class subset checks for CLASS_MAX_NREGS Maciej W. Rozycki
2021-11-02 17:00 ` Kito Cheng
2021-11-03 17:07 ` Maciej W. Rozycki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).