[PATCH] RISC-V/testsuite: Fix ILP32 RVV failures from missing <gnu/stubs-ilp32d.h>

[PATCH] RISC-V/testsuite: Fix ILP32 RVV failures from missing <gnu/stubs-ilp32d.h>

[Maciej W. Rozycki]

In non-multilib installations system headers may not be available for 
compilation options using a non-default model, causing build errors such 
as:

In file included from .../include/features.h:527,
                 from .../include/assert.h:35,
                 from .../gcc/testsuite/gcc.target/riscv/rvv/autovec/vmv-imm-template.h:2,
                 from .../gcc/testsuite/gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c:4:
.../include/gnu/stubs.h:11:11: fatal error: gnu/stubs-ilp32d.h: No such file or directory

Therefore we have to be very cautious when trying to use a non-default 
model in the testsuite, preferably avoiding to rely on headers that have 
not been supplied by GCC itself, or otherwise verifying in a preparatory 
step whether the given model is buildable in a given test environment.

In this case however we can easily avoid the issue, because <assert.h> 
facilities are not used at all by "vmv-imm-template.h", which includes 
the header.  Remove the inclusion then, turning these issues:

FAIL: gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c -O3 -ftree-vectorize (test for excess errors)
UNRESOLVED: gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.i 32
UNRESOLVED: gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.x 8
FAIL: gcc.target/riscv/rvv/autovec/vmv-imm-rv32.c -O3 -ftree-vectorize (test for excess errors)
UNRESOLVED: gcc.target/riscv/rvv/autovec/vmv-imm-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.i 32
UNRESOLVED: gcc.target/riscv/rvv/autovec/vmv-imm-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.x 8

into successful results:

PASS: gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c -O3 -ftree-vectorize (test for excess errors)
PASS: gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.i 32
PASS: gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.x 8
PASS: gcc.target/riscv/rvv/autovec/vmv-imm-rv32.c -O3 -ftree-vectorize (test for excess errors)
PASS: gcc.target/riscv/rvv/autovec/vmv-imm-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.i 32
PASS: gcc.target/riscv/rvv/autovec/vmv-imm-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.x 8

in a plain LP64 `riscv64-linux-gnu' configuration.

	gcc/testsuite/
	* gcc.target/riscv/rvv/autovec/vmv-imm-template.h: Remove 
	<assert.h> inclusion.
---
 gcc/testsuite/gcc.target/riscv/rvv/autovec/vmv-imm-template.h |    1 -
 1 file changed, 1 deletion(-)

gcc-test-riscv-rvv-assert.diff
Index: gcc/gcc/testsuite/gcc.target/riscv/rvv/autovec/vmv-imm-template.h
===================================================================
--- gcc.orig/gcc/testsuite/gcc.target/riscv/rvv/autovec/vmv-imm-template.h
+++ gcc/gcc/testsuite/gcc.target/riscv/rvv/autovec/vmv-imm-template.h
@@ -1,5 +1,4 @@
 #include <stdint-gcc.h>
-#include <assert.h>
 
 #define VMV_POS(TYPE,VAL)				\
   __attribute__ ((noipa))                               \

Re: [PATCH] RISC-V/testsuite: Fix ILP32 RVV failures from missing <gnu/stubs-ilp32d.h>

[Jeff Law]

On 9/22/23 17:18, Maciej W. Rozycki wrote:
> In non-multilib installations system headers may not be available for
> compilation options using a non-default model, causing build errors such
> as:
> 
> In file included from .../include/features.h:527,
>                   from .../include/assert.h:35,
>                   from .../gcc/testsuite/gcc.target/riscv/rvv/autovec/vmv-imm-template.h:2,
>                   from .../gcc/testsuite/gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c:4:
> .../include/gnu/stubs.h:11:11: fatal error: gnu/stubs-ilp32d.h: No such file or directory
> 
> Therefore we have to be very cautious when trying to use a non-default
> model in the testsuite, preferably avoiding to rely on headers that have
> not been supplied by GCC itself, or otherwise verifying in a preparatory
> step whether the given model is buildable in a given test environment.
> 
> In this case however we can easily avoid the issue, because <assert.h>
> facilities are not used at all by "vmv-imm-template.h", which includes
> the header.  Remove the inclusion then, turning these issues:
> 
> FAIL: gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c -O3 -ftree-vectorize (test for excess errors)
> UNRESOLVED: gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.i 32
> UNRESOLVED: gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.x 8
> FAIL: gcc.target/riscv/rvv/autovec/vmv-imm-rv32.c -O3 -ftree-vectorize (test for excess errors)
> UNRESOLVED: gcc.target/riscv/rvv/autovec/vmv-imm-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.i 32
> UNRESOLVED: gcc.target/riscv/rvv/autovec/vmv-imm-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.x 8
> 
> into successful results:
> 
> PASS: gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c -O3 -ftree-vectorize (test for excess errors)
> PASS: gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.i 32
> PASS: gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.x 8
> PASS: gcc.target/riscv/rvv/autovec/vmv-imm-rv32.c -O3 -ftree-vectorize (test for excess errors)
> PASS: gcc.target/riscv/rvv/autovec/vmv-imm-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.i 32
> PASS: gcc.target/riscv/rvv/autovec/vmv-imm-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.x 8
> 
> in a plain LP64 `riscv64-linux-gnu' configuration.
> 
> 	gcc/testsuite/
> 	* gcc.target/riscv/rvv/autovec/vmv-imm-template.h: Remove
> 	<assert.h> inclusion.
OK
jeff

Re: [PATCH] RISC-V/testsuite: Fix ILP32 RVV failures from missing <gnu/stubs-ilp32d.h>

[Maciej W. Rozycki]

On Fri, 22 Sep 2023, Jeff Law wrote:

> > 	gcc/testsuite/
> > 	* gcc.target/riscv/rvv/autovec/vmv-imm-template.h: Remove
> > 	<assert.h> inclusion.
> OK

 Now committed, thanks.

  Maciej

Re: [PATCH] RISC-V/testsuite: Fix ILP32 RVV failures from missing <gnu/stubs-ilp32d.h>

[Vineet Gupta]

+CC Patrick who's been chasing similar issues.

On 9/23/23 00:18, Maciej W. Rozycki wrote:
> In non-multilib installations system headers may not be available for
> compilation options using a non-default model, causing build errors such
> as:
>
> In file included from .../include/features.h:527,
>                   from .../include/assert.h:35,
>                   from .../gcc/testsuite/gcc.target/riscv/rvv/autovec/vmv-imm-template.h:2,
>                   from .../gcc/testsuite/gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c:4:
> .../include/gnu/stubs.h:11:11: fatal error: gnu/stubs-ilp32d.h: No such file or directory
>
> Therefore we have to be very cautious when trying to use a non-default
> model in the testsuite, preferably avoiding to rely on headers that have
> not been supplied by GCC itself, or otherwise verifying in a preparatory
> step whether the given model is buildable in a given test environment.
>
> In this case however we can easily avoid the issue, because <assert.h>
> facilities are not used at all by "vmv-imm-template.h", which includes
> the header.  Remove the inclusion then, turning these issues:
>
> FAIL: gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c -O3 -ftree-vectorize (test for excess errors)
> UNRESOLVED: gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.i 32
> UNRESOLVED: gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.x 8
> FAIL: gcc.target/riscv/rvv/autovec/vmv-imm-rv32.c -O3 -ftree-vectorize (test for excess errors)
> UNRESOLVED: gcc.target/riscv/rvv/autovec/vmv-imm-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.i 32
> UNRESOLVED: gcc.target/riscv/rvv/autovec/vmv-imm-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.x 8
>
> into successful results:
>
> PASS: gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c -O3 -ftree-vectorize (test for excess errors)
> PASS: gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.i 32
> PASS: gcc.target/riscv/rvv/autovec/vmv-imm-fixed-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.x 8
> PASS: gcc.target/riscv/rvv/autovec/vmv-imm-rv32.c -O3 -ftree-vectorize (test for excess errors)
> PASS: gcc.target/riscv/rvv/autovec/vmv-imm-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.i 32
> PASS: gcc.target/riscv/rvv/autovec/vmv-imm-rv32.c -O3 -ftree-vectorize  scan-assembler-times vmv.v.x 8
>
> in a plain LP64 `riscv64-linux-gnu' configuration.
>
> 	gcc/testsuite/
> 	* gcc.target/riscv/rvv/autovec/vmv-imm-template.h: Remove
> 	<assert.h> inclusion.
> ---
>   gcc/testsuite/gcc.target/riscv/rvv/autovec/vmv-imm-template.h |    1 -
>   1 file changed, 1 deletion(-)
>
> gcc-test-riscv-rvv-assert.diff
> Index: gcc/gcc/testsuite/gcc.target/riscv/rvv/autovec/vmv-imm-template.h
> ===================================================================
> --- gcc.orig/gcc/testsuite/gcc.target/riscv/rvv/autovec/vmv-imm-template.h
> +++ gcc/gcc/testsuite/gcc.target/riscv/rvv/autovec/vmv-imm-template.h
> @@ -1,5 +1,4 @@
>   #include <stdint-gcc.h>
> -#include <assert.h>
>   
>   #define VMV_POS(TYPE,VAL)				\
>     __attribute__ ((noipa))

This fix is great but is there a more general solution to the problem 
when we toolchain is built for say just rv64 (and thus only those 
headers) vs. test building for say rv32 (and failing to build due to 
lack of headers) or vice-versa.

Thx,
-Vineet

Re: [PATCH] RISC-V/testsuite: Fix ILP32 RVV failures from missing <gnu/stubs-ilp32d.h>

[Maciej W. Rozycki]

On Sun, 24 Sep 2023, Vineet Gupta wrote:

> This fix is great but is there a more general solution to the problem when we
> toolchain is built for say just rv64 (and thus only those headers) vs. test
> building for say rv32 (and failing to build due to lack of headers) or
> vice-versa.

 The MIPS port has logic in its target test script for combining test 
options and excluding ones that are mutually incompatible due to ABI or 
ISA restrictions.  It wasn't written by me and I have only minimally 
tweaked it (and then many years ago), so I can't remember all the details 
offhand.  See the top comment in gcc/testsuite/gcc.target/mips/mips.exp 
for further information including usage.

 I guess it would make sense to pinch that logic for our port, especially 
given our growing number of machine options.  I think it was mentioned at 
one of the patch review calls (Jeff?).

 NB the use of this specific <assert.h> header, still in place elsewhere, 
seems gratuitous to me.  We don't need or indeed want to print anything in 
the test cases (unless verifying something specific to the print facility) 
and if we want to avoid minor code duplication (i.e. not to have explicit:

  if (...)
    __builtin_abort ();

replicated across test cases), we can easily implement this via a local 
header, there's no need to pull in a complex system facility.

 Also I find the use of this facility questionable in the first place: do 
we want these test cases to pass even in the case of an issue if run with 
-DNDEBUG as a target board option (which would cause some tests to be 
optimised away in their entriety)?

  Maciej

Re: [PATCH] RISC-V/testsuite: Fix ILP32 RVV failures from missing <gnu/stubs-ilp32d.h>

[Maciej W. Rozycki]

On Mon, 25 Sep 2023, Maciej W. Rozycki wrote:

>  NB the use of this specific <assert.h> header, still in place elsewhere, 
> seems gratuitous to me.  We don't need or indeed want to print anything in 
> the test cases (unless verifying something specific to the print facility) 
> and if we want to avoid minor code duplication (i.e. not to have explicit:
> 
>   if (...)
>     __builtin_abort ();
> 
> replicated across test cases), we can easily implement this via a local 
> header, there's no need to pull in a complex system facility.

 Overall we ought not to require any system headers in compile tests and 
then link and run tests need a functional target environment anyway.  So 
maybe the use of <assert.h> in run tests isn't as bad after all if not for 
the -DNDEBUG peculiarity.  However I still think the less we depend in 
verification on external components the better, that's one variable to 
exclude.

  Maciej

Re: [PATCH] RISC-V/testsuite: Fix ILP32 RVV failures from missing <gnu/stubs-ilp32d.h>

[Jeff Law]

On 9/25/23 15:17, Maciej W. Rozycki wrote:
> On Mon, 25 Sep 2023, Maciej W. Rozycki wrote:
> 
>>   NB the use of this specific <assert.h> header, still in place elsewhere,
>> seems gratuitous to me.  We don't need or indeed want to print anything in
>> the test cases (unless verifying something specific to the print facility)
>> and if we want to avoid minor code duplication (i.e. not to have explicit:
>>
>>    if (...)
>>      __builtin_abort ();
>>
>> replicated across test cases), we can easily implement this via a local
>> header, there's no need to pull in a complex system facility.
> 
>   Overall we ought not to require any system headers in compile tests and
> then link and run tests need a functional target environment anyway.  So
> maybe the use of <assert.h> in run tests isn't as bad after all if not for
> the -DNDEBUG peculiarity.  However I still think the less we depend in
> verification on external components the better, that's one variable to
> exclude.
Certainly we don't want extraneous #includes.   We can often avoid them 
with a few judicious prototypes, like for abort ().

But we also need to get to the point where we can run tests which have 
#include directives that reference system headers.  Many tests in the 
various GCC testsuites have those directives and we don't want to be 
continually trying to eradicate #includes from those tests.

The standard way to deal with this is single tree builds which are 
deprecated or to have an install tree with the suitable multilib headers 
and libraries.  The latter seems like the only viable solution to me.

jeff

Re: [PATCH] RISC-V/testsuite: Fix ILP32 RVV failures from missing <gnu/stubs-ilp32d.h>

[Palmer Dabbelt]

On Wed, 27 Sep 2023 10:28:55 PDT (-0700), jeffreyalaw@gmail.com wrote:
>
>
> On 9/25/23 15:17, Maciej W. Rozycki wrote:
>> On Mon, 25 Sep 2023, Maciej W. Rozycki wrote:
>>
>>>   NB the use of this specific <assert.h> header, still in place elsewhere,
>>> seems gratuitous to me.  We don't need or indeed want to print anything in
>>> the test cases (unless verifying something specific to the print facility)
>>> and if we want to avoid minor code duplication (i.e. not to have explicit:
>>>
>>>    if (...)
>>>      __builtin_abort ();
>>>
>>> replicated across test cases), we can easily implement this via a local
>>> header, there's no need to pull in a complex system facility.
>>
>>   Overall we ought not to require any system headers in compile tests and
>> then link and run tests need a functional target environment anyway.  So
>> maybe the use of <assert.h> in run tests isn't as bad after all if not for
>> the -DNDEBUG peculiarity.  However I still think the less we depend in
>> verification on external components the better, that's one variable to
>> exclude.
> Certainly we don't want extraneous #includes.   We can often avoid them
> with a few judicious prototypes, like for abort ().
>
> But we also need to get to the point where we can run tests which have
> #include directives that reference system headers.  Many tests in the
> various GCC testsuites have those directives and we don't want to be
> continually trying to eradicate #includes from those tests.
>
> The standard way to deal with this is single tree builds which are
> deprecated or to have an install tree with the suitable multilib headers
> and libraries.  The latter seems like the only viable solution to me.

IMO this is one of those places where we should just be as normal as 
possible.  So if the other big ports allow system headers then we 
should, otherwise we should move everyone over to testing in some way 
we'll catch these before commit.

>
> jeff

Re: [PATCH] RISC-V/testsuite: Fix ILP32 RVV failures from missing <gnu/stubs-ilp32d.h>

[Jeff Law]

On 9/27/23 11:32, Palmer Dabbelt wrote:

> 
> IMO this is one of those places where we should just be as normal as 
> possible.  So if the other big ports allow system headers then we 
> should, otherwise we should move everyone over to testing in some way 
> we'll catch these before commit.
Exactly.  I think the dance we've been doing with stdint-gcc.h is a bit 
silly, but I haven't pushed on it at all.

No other port does anything similar.  When they need stdint.h, they 
include it.  It does mean you have to have the appropriate headers 
installed for each multilib configuration, but that's the way every 
other port handles this problem.  There's no good reason I'm aware of 
for RISC-V to be different.

jeff

Re: [PATCH] RISC-V/testsuite: Fix ILP32 RVV failures from missing <gnu/stubs-ilp32d.h>

[Maciej W. Rozycki]

On Wed, 27 Sep 2023, Jeff Law wrote:

> > IMO this is one of those places where we should just be as normal as
> > possible.  So if the other big ports allow system headers then we should,
> > otherwise we should move everyone over to testing in some way we'll catch
> > these before commit.
> Exactly.  I think the dance we've been doing with stdint-gcc.h is a bit silly,
> but I haven't pushed on it at all.
> 
> No other port does anything similar.  When they need stdint.h, they include
> it.  It does mean you have to have the appropriate headers installed for each
> multilib configuration, but that's the way every other port handles this
> problem.  There's no good reason I'm aware of for RISC-V to be different.

 I agree that using standard system headers where required is a reasonable 
expectation, but I maintain that when using a non-default ABI/multilib by 
an explicit request of a test case, it is the responsibility of our test 
framework to verify that the chosen ABI/multilib is compatible with the 
environment.  I think it should apply equally to all the tests whether 
they are run, link, or compilation tests.

 I think a requirement for a verifier to have headers for all the possible 
ABIs/multilibs installed is unreasonable.  For a hosted target such as 
Linux/*BSD/whatever it may yet be feasible.  For a bare-metal target it 
may not be possible, and in particular such a target may possibly support 
one specific ABI only and #error if an incompatible configuration is 
detected.  This must not cause a test to FAIL, because GIGO.

 Overall I think an expectation ought to be for a given ABI/multilib to be 
verified by running the whole testsuite for the desired configuration, 
either by having it as the default for the toolchain under test or via an 
explicit target board option.  I accept the desire to have alternative 
ABIs/multilibs smoke-tested by a couple of tests explicitly requesting 
them via a compilation option, to verify that the option works if nothing 
else.

 I think the burden of verifying the compatibility of the environment must 
not be on the individual tests, but the framework itself, and I think the 
MIPS framework fulfils the requirement, as it verifies the options given 
in the individual tests without the tests themselves having to request 
anything, they just list the compilation options they require the usual 
way via `dg-options'.

 I think that to verify the compatibility of ABI/multilib options for 
compilation tests we can pick up a system header we can reasonably expect 
to change depending on the ABI/multilib chosen, and therefore to pull any 
ABI/multilib-specific bits, such as <limits.h>.  If a given environment 
turns out incompatible with a given ABI/multilib option, then all the 
tests requesting said option ought to be automatically demoted to 
UNSUPPORTED.

 Most importantly implementing this approach in our test framework is a 
one-off project, while requiring people to have their environment set up 
for ABI/multilib configurations they have no interest in would cause them 
continuous effort.

  Maciej

Re: [PATCH] RISC-V/testsuite: Fix ILP32 RVV failures from missing <gnu/stubs-ilp32d.h>

[Jeff Law]

On 9/28/23 03:46, Maciej W. Rozycki wrote:
> On Wed, 27 Sep 2023, Jeff Law wrote:
> 
>>> IMO this is one of those places where we should just be as normal as
>>> possible.  So if the other big ports allow system headers then we should,
>>> otherwise we should move everyone over to testing in some way we'll catch
>>> these before commit.
>> Exactly.  I think the dance we've been doing with stdint-gcc.h is a bit silly,
>> but I haven't pushed on it at all.
>>
>> No other port does anything similar.  When they need stdint.h, they include
>> it.  It does mean you have to have the appropriate headers installed for each
>> multilib configuration, but that's the way every other port handles this
>> problem.  There's no good reason I'm aware of for RISC-V to be different.
> 
>   I agree that using standard system headers where required is a reasonable
> expectation, but I maintain that when using a non-default ABI/multilib by
> an explicit request of a test case, it is the responsibility of our test
> framework to verify that the chosen ABI/multilib is compatible with the
> environment.  I think it should apply equally to all the tests whether
> they are run, link, or compilation tests.
No other target enforces this kind of requirement and I would maintain 
that we shouldn't either.


> 
>   I think a requirement for a verifier to have headers for all the possible
> ABIs/multilibs installed is unreasonable.  For a hosted target such as
> Linux/*BSD/whatever it may yet be feasible.  For a bare-metal target it
> may not be possible, and in particular such a target may possibly support
> one specific ABI only and #error if an incompatible configuration is
> detected.  This must not cause a test to FAIL, because GIGO.
If you're testing options that you don't have headers/libraries for, 
then that's a testsuite bug irrespective of bare metal vs linux.  It's 
been like that for about 30 years at this point.  I fail to see why 
RISC-V should be special in this regard.

Jeff