,,

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.

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
> > >

Reply via email to