RISC-V: Fix testcase failed when default -mcmodel=medany

Hi,

This patch fix testcase failed when I build RISC-V GCC with -mcmodel=medany
as default. If set to medany, stack_save_restore.c testcase will fail because of
the reduced use of s3 registers in assembly (thus calling __riscv_save/store_3
instead of __riscv_save/store_4). Explicitly add -mcmodel=medlow to solve this
problem.

Best,
Lehua

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/stack_save_restore.c: Add -mcmodel=medlow

---
 gcc/testsuite/gcc.target/riscv/stack_save_restore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Not familiar with this stuff. 
I leave it other RISC-V folks to review.



juzhe.zhong@rivai.ai
 
From: Lehua Ding
Date: 2023-07-18 15:42
To: gcc-patches
CC: juzhe.zhong; rdapp.gcc; kito.cheng; palmer; jeffreyalaw
Subject: [PATCH] RISC-V: Fix testcase failed when default -mcmodel=medany
Hi,
 
This patch fix testcase failed when I build RISC-V GCC with -mcmodel=medany
as default. If set to medany, stack_save_restore.c testcase will fail because of
the reduced use of s3 registers in assembly (thus calling __riscv_save/store_3
instead of __riscv_save/store_4). Explicitly add -mcmodel=medlow to solve this
problem.
 
Best,
Lehua
 
gcc/testsuite/ChangeLog:
 
* gcc.target/riscv/stack_save_restore.c: Add -mcmodel=medlow
 
---
gcc/testsuite/gcc.target/riscv/stack_save_restore.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
 
diff --git a/gcc/testsuite/gcc.target/riscv/stack_save_restore.c b/gcc/testsuite/gcc.target/riscv/stack_save_restore.c
index 522e706cfbf..a2430783474 100644
--- a/gcc/testsuite/gcc.target/riscv/stack_save_restore.c
+++ b/gcc/testsuite/gcc.target/riscv/stack_save_restore.c
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options "-march=rv32imafc -mabi=ilp32f -msave-restore -O2 -fno-schedule-insns -fno-schedule-insns2 -fno-unroll-loops -fno-peel-loops -fno-lto" } */
+/* { dg-options "-march=rv32imafc -mabi=ilp32f -msave-restore -O2 -fno-schedule-insns -fno-schedule-insns2 -fno-unroll-loops -fno-peel-loops -fno-lto -mcmodel=medlow" } */
/* { dg-final { check-function-bodies "**" "" } } */
char my_getchar();

Hi Lehua,

> This patch fix testcase failed when I build RISC-V GCC with -mcmodel=medany
> as default. If set to medany, stack_save_restore.c testcase will fail because of
> the reduced use of s3 registers in assembly (thus calling __riscv_save/store_3
> instead of __riscv_save/store_4). Explicitly add -mcmodel=medlow to solve this
> problem.

Wouldn't you rather want to adjust the test to not check for one register
number but 3 or 4 instead?  There might be future changes in default behavior
that would invalidate the test as well.

Regards
 Robin

Hi Robin,


> Wouldn't you rather want to adjust the test to not check for one register
> number but 3 or 4 instead?

I think the purpose of this testcase is to check whether the modifications to
the stack frame are as expected, so it is necessary to specify exactly whether
three or four registers are saved. But I think its need to add another testcase
which use another option -mcmodel=medany for coverage.


> There might be future changes in default behavior
> that would invalidate the test as well.

Because -mcmodel is explicitly specified in the testcase, future changes
to the default value of -mcmodel will not cause the test case to fail.


Best,
Lehua

Hi Lehua,

> I think the purpose of this testcase is to check whether the modifications to
> the stack frame are as expected, so it is necessary to specify exactly whether
> three or four registers are saved. But I think its need to add another testcase
> which use another option -mcmodel=medany for coverage.

In general I'm fine with this small change of course, I just wonder if
the test case is not brittle anyway.  From what I can tell the respective
change is independent of the actual number of registers so maybe it's enough to
not compare the fully body but just make sure the addis are not present?
That way, the test could also work for -march=rv64 (which saves one
register less anyway regardless of mcmodel - but the change still helps)
or maybe even with instruction scheduling.  Would you mind checking this still?
Thanks.

If it turns out not to be possible, let's stick with the medany fix for now
and add a TODO.

Regards
 Robin