diff mbox series

RISC-V: Set require-effective-target rv64 for PR113742

Message ID 20240214194631.1877280-1-ewlu@rivosinc.com
State Unresolved
Headers
Series RISC-V: Set require-effective-target rv64 for PR113742 |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Edwin Lu Feb. 14, 2024, 7:46 p.m. UTC 
  The testcase pr113742.c is failing for 32 bit targets due to the following cc1
error:
cc1: error: ABI requries '-march=rv64'

Disable testing on rv32 targets

	PR target/113742

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/pr113742.c: add require-effective-target

Signed-off-by: Edwin Lu <ewlu@rivosinc.com>
---
 gcc/testsuite/gcc.target/riscv/pr113742.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Robin Dapp Feb. 14, 2024, 8:09 p.m. UTC | #1 
On 2/14/24 20:46, Edwin Lu wrote:
> The testcase pr113742.c is failing for 32 bit targets due to the following cc1
> error:
> cc1: error: ABI requries '-march=rv64'

I think we usually just add exactly this to the test options (so
it is always run rather than just on a 64-bit target.

Regards
 Robin
Edwin Lu Feb. 14, 2024, 9:34 p.m. UTC | #2 
On 2/14/2024 12:09 PM, Robin Dapp wrote:
> On 2/14/24 20:46, Edwin Lu wrote:
>> The testcase pr113742.c is failing for 32 bit targets due to the following cc1
>> error:
>> cc1: error: ABI requries '-march=rv64'
> I think we usually just add exactly this to the test options (so
> it is always run rather than just on a 64-bit target.
>
> Regards
>   Robin

Ah oops I glanced over the /* { dg-do compile } */part. It should be 
fine to add '-march=rv64gc' instead then?

Edwin
Robin Dapp Feb. 15, 2024, 11:43 a.m. UTC | #3 
> Ah oops I glanced over the /* { dg-do compile } */part. It should be
> fine to add '-march=rv64gc' instead then?

Hmm it's a bit tricky.  So generally -mcpu=sifive-p670 includes rv64
but it does not override a previously specified -march=rv32 (that might
have been added by the test harness or the test target).  It looks
like it does override a (build option and thus not directly specified
when compiling) --with-arch=rv32.

For now I'd stick with something like -march=rv64gc -mtune=sifive-p670
(but please check if the original problem does occur with this).
While you're at it you could delete the redundant '/' in the first
line.

In general it's a bit counterintuitive a test specifying a
particular CPU (that supports several extensions) might have
those overridden when e.g. testing on a rv32 target not supporting
those.  We also do not support cpu names in the march string
so there is no nice way of overriding previously specified marchs.

Kito: Any idea regarding this?  I read in your commit message that
mcpu has lower precedence than march.  Right now that allows us to
somewhat silently remove architecture options that are specified
last on the command line.

aarch64 warns in case something is in conflict, maybe we should do
that as well?

At least I find it a bit annoying that we don't have a way of
saying:
"This test always needs to be compiled with all arch features of
cpu = ..." and rather need to specify -march=rv64gcv_z..._z...

Without having this thought through, can't mcpu be of kind of
similar precedence to march and we'd let the one specified last
"win" in case of conflicts?  Possibly with an exception for
the 32/64 bit.  Does LLVM not have this problem?

Regards
 Robin
Monk Chiang Feb. 20, 2024, 9:37 a.m. UTC | #4 
Hi Edwin,
I think just replace to:
/* { dg-options "-O2 -finstrument-functions -mabi=lp64d -march=rv64gc
-mtune=sifive-p600-series" } */

On Thu, Feb 15, 2024 at 7:43 PM Robin Dapp <rdapp.gcc@gmail.com> wrote:

> > Ah oops I glanced over the /* { dg-do compile } */part. It should be
> > fine to add '-march=rv64gc' instead then?
>
> Hmm it's a bit tricky.  So generally -mcpu=sifive-p670 includes rv64
> but it does not override a previously specified -march=rv32 (that might
> have been added by the test harness or the test target).  It looks
> like it does override a (build option and thus not directly specified
> when compiling) --with-arch=rv32.
>
> For now I'd stick with something like -march=rv64gc -mtune=sifive-p670
> (but please check if the original problem does occur with this).
> While you're at it you could delete the redundant '/' in the first
> line.
>
> In general it's a bit counterintuitive a test specifying a
> particular CPU (that supports several extensions) might have
> those overridden when e.g. testing on a rv32 target not supporting
> those.  We also do not support cpu names in the march string
> so there is no nice way of overriding previously specified marchs.
>
> Kito: Any idea regarding this?  I read in your commit message that
> mcpu has lower precedence than march.  Right now that allows us to
> somewhat silently remove architecture options that are specified
> last on the command line.
>
> aarch64 warns in case something is in conflict, maybe we should do
> that as well?
>
> At least I find it a bit annoying that we don't have a way of
> saying:
> "This test always needs to be compiled with all arch features of
> cpu = ..." and rather need to specify -march=rv64gcv_z..._z...
>
> Without having this thought through, can't mcpu be of kind of
> similar precedence to march and we'd let the one specified last
> "win" in case of conflicts?  Possibly with an exception for
> the 32/64 bit.  Does LLVM not have this problem?
>
> Regards
>  Robin
>
>
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.target/riscv/pr113742.c b/gcc/testsuite/gcc.target/riscv/pr113742.c
index ab8934c2a8a..9cea92ed97c 100644
--- a/gcc/testsuite/gcc.target/riscv/pr113742.c
+++ b/gcc/testsuite/gcc.target/riscv/pr113742.c
@@ -1,4 +1,5 @@ 
 //* { dg-do compile } */
 /* { dg-options "-O2 -finstrument-functions -mabi=lp64d -mcpu=sifive-p670" } */
+/* { dg-require-effective-target rv64 } */
 
 void foo(void) {}