On Mon, May 26, 2025 at 4:57 AM Christophe Lyon
<christophe.l...@linaro.org> wrote:
>
> ,,
>
> On Mon, 26 May 2025 at 12:54, Andrew Pinski (QUIC)
> <quic_apin...@quicinc.com> wrote:
> >
> > > -----Original Message-----
> > > From: Christophe Lyon <christophe.l...@linaro.org>
> > > Sent: Monday, May 26, 2025 3:09 AM
> > > To: Andrew Pinski (QUIC) <quic_apin...@quicinc.com>
> > > Cc: gcc-patches@gcc.gnu.org
> > > Subject: Re: [PATCH] testsuite: Fix pr101145inf*.c testcases
> > > [PR117494]
> > >
> > > Hi Andrew,
> > >
> > > On Sun, 17 Nov 2024 at 22:49, Andrew Pinski
> > > <quic_apin...@quicinc.com> wrote:
> > > >
> > > > Instead of doing a dg-run with a specific target check for
> > > linux.
> > > > Use signal as the effective-target since this requires the use
> > > of
> > > > ALARM signal to do the testing.
> > > > Also use check_vect in the main and renames main to main1
> > > to make sure
> > > > we don't use the registers.
> > > >
> > > > Tested on x86_64-linux-gnu.
> > >
> > > Can you explain the context of this change? Are you testing on
> > > a target which matches *-*-linux* *-*-gnu* *-*-uclinux* but
> > > does not support 'alarm'?
> > >
> > > I was looking at backporting this and a later patch from
> > > Torbjorn to gcc-14, but I noticed that the default dg-do-what
> > > in this directory is 'compile' and the previous dg-do run {
> > > target *-*-linux* *-*-gnu* *-*-uclinux* } changed it to 'run' on
> > > linux targets (thus on linux we had 2 PASS, one for
> > > compilation, one for execution).
> > > And the test was skipped on non-linux targets.
> > >
> > > After your patch, the test became compile-only on linux, was
> > > this intentional?
> >
> > So vect.exp has some extra code in there where the default to dg-run unless
> > the target you are running with does not support executing with the vector
> > options selected.
> > So for an example on x86_64-linux-gnu with we get the test being executed:
> > ```
> > Setting LD_LIBRARY_PATH to
> > :/bajas/pinskia/src/upstream-gcc-isel/gcc/objdir/gcc:/bajas/pinskia/src/upstream-gcc-isel/gcc/objdir/gcc/32::/bajas/pinskia/src/upstream-gcc-isel/gcc/objdir/gcc:/bajas/pinskia/src/upstream-gcc-isel/gcc/objdir/gcc/32
> > Execution timeout is: 300
> > spawn [open ...]^M
> > PASS: gcc.dg/vect/pr101145inf.c execution test
> > ```
> >
> > If you are not getting the executed test any more then the target which you
> > are running on does not support using the options that are added by
> > vect.exp.
> >
> > This is the exact behavior we need in this case; otherwise you get
> > executable failures in some cases. With dg-run, you will get executable
> > failures on RISC-V because the check_vect functionality is not currently
> > implemented.
> >
> > I don't know which exact linux target you are testing on since you didn't
> > mention either. I can only assume it is arm-linux-gnueabi (as
> > aarch64-linux-gnu just does ` set dg-do-what-default run` so it runs
> > always) which does the following for check_vect_support_and_set_flags:
> > ```
> > } elseif [is-effective-target arm_neon_ok] {
> > eval lappend DEFAULT_VECTCFLAGS [add_options_for_arm_neon ""]
> > # NEON does not support denormals, so is not used for vectorization
> > by
> > # default to avoid loss of precision. We must pass -ffast-math to
> > test
> > # vectorization of float operations.
> > lappend DEFAULT_VECTCFLAGS "-ffast-math"
> > if [is-effective-target arm_neon_hw] {
> > set dg-do-what-default run
> > } else {
> > set dg-do-what-default compile
> > }
> > }
> > ```
> >
> > So ` is-effective-target arm_neon_hw` must be returning false on the
> > hardware you are running with. Which also means check_vect would have just
> > done an `exit (0)` and not do the full test for the hardware you are
> > running on anyways.
> >
>
> I was looking at aarch64-linux-gnu results, but initially for
> gcc.dg/pr114052-1.c where there's a discussion in the PR about its
> backport to gcc-14.
> Looking at where the dg-require-effective-target alarm was coming
> from, I found r15-7152-g57b706d141b87c which also modified
> gcc.dg/vect/pr101145inf.c.
>
> I forgot that vect.exp does set dg-do run, so you are right of course.
> However, I think for the other testcases updated by r15-7152-g57b706d141b87c,
> we probably want to keep the dg-do run line, as currently
> gcc.dg/pr78185.c, gcc.dg/pr116906-1.c and gcc.dg/pr116906-2.c are
> compiile-only tests on aarch64-linux-gnu.
It looks like you got two commits/patches mixed up here. The commit
associated with my patch is r15-5377-g0dc389f21bfd4e .
I have no comment on r15-7152-g57b706d141b87c since it is unrelated to
this patch (except it touched pr101145inf.c adding the alarm check).
You should move the discussion over to
https://inbox.sourceware.org/gcc-patches/20250119201401.4082622-1-torbjorn.svens...@foss.st.com/
patch instead.
Thanks,
Andrew
>
> Thanks,
>
> Christophe
>
> >
> > Thanks,
> > Andrew Pinski
> >
> >
> > >
> > > Maybe we should use instead:
> > > /* { dg-do run } */ (without target selector)
> > > /* { dg-require-effective-target signal } */ not tested, but I
> > > think this would restore the previous behaviour on
> > > *linux* targets, but skip the test on targets which do not
> > > support alarm.
> > >
> > > Am I missing something?
> > > I can send a patch to do this.
> > >
> > > Thanks,
> > >
> > > Christophe
> > >
> > >
> > > >
> > > > PR testsuite/117494
> > > > gcc/testsuite/ChangeLog:
> > > >
> > > > * gcc.dg/vect/pr101145inf.c: Remove dg-do and replace
> > > > with dg-require-effective-target of signal.
> > > > * gcc.dg/vect/pr101145inf_1.c: Likewise.
> > > > * gcc.dg/vect/pr101145inf.inc: Rename main to main1
> > > > and mark as noinline.
> > > > Include tree-vect.h. Have main call check_vect and
> > > main1.
> > > >
> > > > Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> > > > ---
> > > > gcc/testsuite/gcc.dg/vect/pr101145inf.c | 2 +-
> > > > gcc/testsuite/gcc.dg/vect/pr101145inf.inc | 9 ++++++++-
> > > > gcc/testsuite/gcc.dg/vect/pr101145inf_1.c | 2 +-
> > > > 3 files changed, 10 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/gcc/testsuite/gcc.dg/vect/pr101145inf.c
> > > > b/gcc/testsuite/gcc.dg/vect/pr101145inf.c
> > > > index 3ad8c1a2dd7..aa598875aa5 100644
> > > > --- a/gcc/testsuite/gcc.dg/vect/pr101145inf.c
> > > > +++ b/gcc/testsuite/gcc.dg/vect/pr101145inf.c
> > > > @@ -1,4 +1,4 @@
> > > > -/* { dg-do run { target *-*-linux* *-*-gnu* *-*-uclinux* } } */
> > > > +/* { dg-require-effective-target signal } */
> > > > /* { dg-additional-options "-O3" } */ #include <limits.h>
> > > #include
> > > > "pr101145inf.inc"
> > > > diff --git a/gcc/testsuite/gcc.dg/vect/pr101145inf.inc
> > > > b/gcc/testsuite/gcc.dg/vect/pr101145inf.inc
> > > > index 4aa3d049187..eb855b9881a 100644
> > > > --- a/gcc/testsuite/gcc.dg/vect/pr101145inf.inc
> > > > +++ b/gcc/testsuite/gcc.dg/vect/pr101145inf.inc
> > > > @@ -1,6 +1,7 @@
> > > > #include <unistd.h>
> > > > #include <signal.h>
> > > > #include <stdlib.h>
> > > > +#include "tree-vect.h"
> > > >
> > > > void test_finite ();
> > > > void test_infinite ();
> > > > @@ -10,7 +11,8 @@ void do_exit (int i)
> > > > exit (0);
> > > > }
> > > >
> > > > -int main(void)
> > > > +__attribute__((noinline))
> > > > +int main1(void)
> > > > {
> > > > test_finite ();
> > > > struct sigaction s;
> > > > @@ -26,3 +28,8 @@ int main(void)
> > > > return 1;
> > > > }
> > > >
> > > > +int main(void)
> > > > +{
> > > > + check_vect ();
> > > > + return main1();
> > > > +}
> > > > diff --git a/gcc/testsuite/gcc.dg/vect/pr101145inf_1.c
> > > > b/gcc/testsuite/gcc.dg/vect/pr101145inf_1.c
> > > > index e3e9dd46d10..0465788c3cc 100644
> > > > --- a/gcc/testsuite/gcc.dg/vect/pr101145inf_1.c
> > > > +++ b/gcc/testsuite/gcc.dg/vect/pr101145inf_1.c
> > > > @@ -1,4 +1,4 @@
> > > > -/* { dg-do run { target *-*-linux* *-*-gnu* *-*-uclinux* } } */
> > > > +/* { dg-require-effective-target signal } */
> > > > /* { dg-additional-options "-O3" } */ #include <limits.h>
> > > #include
> > > > "pr101145inf.inc"
> > > > --
> > > > 2.43.0
> > > >
