This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC testsuite] Fix PR64850, tweak acc_on_device* tests


Hi!

On Wed, 4 Feb 2015 10:40:40 +0100, I wrote:
> On Wed, 04 Feb 2015 08:41:28 +0900 (JST), Kaz Kojima <kkojima@rr.iij4u.or.jp> wrote:
> > Several goacc/acc_on_device tests fail for a few targets:
> > 
> > hppa2.0w-hp-hpux11.11 (PR testsuite/64850)
> > https://gcc.gnu.org/ml/gcc-testresults/2015-01/msg02659.html
> > 
> > m68k-unknown-linux-gnu
> > https://gcc.gnu.org/ml/gcc-testresults/2015-01/msg02960.html
> > 
> > sh4-unknown-linux-gnu
> > https://gcc.gnu.org/ml/gcc-testresults/2015-01/msg02930.html
> > 
> > Also they fail with special options
> > x86_64-unknown-linux-gnu -fpic -mcmodel=large
> > https://gcc.gnu.org/ml/gcc-testresults/2015-02/msg00198.html
> 
> Thanks for looking into this -- incidentally, I also started looking into
> it yesterday...  :-)
> 
> > Those tests scan .expand rtl dumps to get the number of calls for
> > acc_on_device function.  For almost targets, the call rtx looks
> > something like
> > 
> >   (call (mem:QI (symbol_ref:SI ("acc_on_device") [flags 0x41]  <function_decl 0xb7614100 acc_on_device>) [0 acc_on_device S1 A8])
> > 
> > and tests use the regular expression "\\\(call \[^\\n\]*\\\"acc_on_device"
> > to detect it.
> > This expression doesn't match with the corresponding call rtx
> > 
> >   (call (mem:SI (symbol_ref/v:SI ("@acc_on_device") [flags 0x41]  <function_decl 0xb764d900 acc_on_device>) [0 acc_on_device S4 A32])
> > 
> > for hppa and something like
> > 
> >   (call (mem:QI (reg/f:SI 33) [0 acc_on_device S1 A8])
> > 
> > for m68k and sh.  All call rtxes have the function name in
> > the alias set of its mem rtx and it seems that the regular
> > expression "\\\(call \[^\\n\]* acc_on_device" works for all
> > cases.  The attached patch is tested on i686-pc-linux-gnu and
> > sh4-unknown-linux-gnu.
> 
> > 	PR testsuite/64850
> > 	* gcc.dg/goacc/acc_on_device-1.c: Use a space instead of \\\" in
> > 	the expression to find calls.
> > 	* c-c++-common/goacc/acc_on_device-2.c: Likewise.
> > 	* c-c++-common/goacc/acc_on_device-2-off.c: Likewise.
> > 	* gfortran.dg/goacc/acc_on_device-1.f95: Likewise.
> > 	* gfortran.dg/goacc/acc_on_device-2.f95: Likewise.
> > 	* gfortran.dg/goacc/acc_on_device-2-off.f95: Likewise.
> 
> The other idea that I had is to separately scan/count the symbol_ref and
> the call (or call_insn?), but I'm not sure if that is "better".  So, your
> patch looks good to me, thanks!

To resolve the immediate problem: is my approval "enough" for Kaz to
commit the patch, or does that need a "more authoritative approval"?

Jakub then suggested on IRC:

    <jakub> tschwinge: I don't understand why do you want to scan rtl dumps at all; if you want to verify the library function is not called, I'd say best just scan-assembler-not acc_on_device

I think I copied this approach from some other test case (but don't
remember which).  In the current set of tests, we need to verify that the
acc_on_device library function is called 0, 1, or 4 times (see below).

For example, for gcc.dg/goacc/acc_on_device-1.c we'Ve got:

    $ build-gcc/gcc/xgcc -Bbuild-gcc/gcc/ source-gcc/gcc/testsuite/gcc.dg/goacc/acc_on_device-1.c -fopenacc -O -std=c89 -Wno-implicit-function-declaration -S -fpic -mcmodel=large -o acc_on_device-1.s
    $ grep acc_on_device < acc_on_device-1.s
            .file   "acc_on_device-1.c"
            movabsq $acc_on_device@PLTOFF, %rdx
            movabsq $acc_on_device@PLTOFF, %rdx
            movabsq $acc_on_device@PLTOFF, %rdx
            movabsq $acc_on_device@PLTOFF, %rdx

Isn't it even more fragile to scan here for acc_on_device being called
four times compared to the -fdump-rtl-expand dump?  Or should I split up
the four tests into four separate files?  (I guess I lack knowledge of
the best approach for doing such a thing in the GCC testsuite.)

For reference:

> > diff --git a/c-c++-common/goacc/acc_on_device-2-off.c b/c-c++-common/goacc/acc_on_device-2-off.c
> > index 25d21ad..ea31047 100644
> > --- a/c-c++-common/goacc/acc_on_device-2-off.c
> > +++ b/c-c++-common/goacc/acc_on_device-2-off.c
> > @@ -20,6 +20,6 @@ f (void)
> >  }
> >  
> >  /* Without -fopenacc, we're expecting one call.
> > -   { dg-final { scan-rtl-dump-times "\\\(call \[^\\n\]*\\\"acc_on_device" 1 "expand" } } */
> > +   { dg-final { scan-rtl-dump-times "\\\(call \[^\\n\]* acc_on_device" 1 "expand" } } */
> >  
> >  /* { dg-final { cleanup-rtl-dump "expand" } } */
> > diff --git a/c-c++-common/goacc/acc_on_device-2.c b/c-c++-common/goacc/acc_on_device-2.c
> > index d5389a9..2f4ee2b 100644
> > --- a/c-c++-common/goacc/acc_on_device-2.c
> > +++ b/c-c++-common/goacc/acc_on_device-2.c
> > @@ -24,6 +24,6 @@ f (void)
> >     perturbs expansion as a builtin, which expects an int parameter.  It's fine
> >     when changing acc_device_t to plain int, but that's not what we're doing in
> >     <openacc.h>.
> > -   { dg-final { scan-rtl-dump-times "\\\(call \[^\\n\]*\\\"acc_on_device" 0 "expand" { xfail c++ } } } */
> > +   { dg-final { scan-rtl-dump-times "\\\(call \[^\\n\]* acc_on_device" 0 "expand" { xfail c++ } } } */
> >  
> >  /* { dg-final { cleanup-rtl-dump "expand" } } */
> > diff --git a/gcc.dg/goacc/acc_on_device-1.c b/gcc.dg/goacc/acc_on_device-1.c
> > index 1a0276e..d0dbc82 100644
> > --- a/gcc.dg/goacc/acc_on_device-1.c
> > +++ b/gcc.dg/goacc/acc_on_device-1.c
> > @@ -15,6 +15,6 @@ f (void)
> >  }
> >  
> >  /* Unsuitable to be handled as a builtin, so we're expecting four calls.
> > -   { dg-final { scan-rtl-dump-times "\\\(call \[^\\n\]*\\\"acc_on_device" 4 "expand" } } */
> > +   { dg-final { scan-rtl-dump-times "\\\(call \[^\\n\]* acc_on_device" 4 "expand" } } */
> >  
> >  /* { dg-final { cleanup-rtl-dump "expand" } } */
> > diff --git a/gfortran.dg/goacc/acc_on_device-1.f95 b/gfortran.dg/goacc/acc_on_device-1.f95
> > index 9dfde26..0126d9c 100644
> > --- a/gfortran.dg/goacc/acc_on_device-1.f95
> > +++ b/gfortran.dg/goacc/acc_on_device-1.f95
> > @@ -17,6 +17,6 @@ logical function f ()
> >  end function f
> >  
> >  ! Unsuitable to be handled as a builtin, so we're expecting four calls.
> > -! { dg-final { scan-rtl-dump-times "\\\(call \[^\\n\]*\\\"acc_on_device" 4 "expand" } }
> > +! { dg-final { scan-rtl-dump-times "\\\(call \[^\\n\]* acc_on_device" 4 "expand" } }
> >  
> >  ! { dg-final { cleanup-rtl-dump "expand" } }
> > diff --git a/gfortran.dg/goacc/acc_on_device-2-off.f95 b/gfortran.dg/goacc/acc_on_device-2-off.f95
> > index cf28264..0a4978e 100644
> > --- a/gfortran.dg/goacc/acc_on_device-2-off.f95
> > +++ b/gfortran.dg/goacc/acc_on_device-2-off.f95
> > @@ -34,6 +34,6 @@ logical (4) function f ()
> >  end function f
> >  
> >  ! Without -fopenacc, we're expecting one call.
> > -! { dg-final { scan-rtl-dump-times "\\\(call \[^\\n\]*\\\"acc_on_device" 1 "expand" } }
> > +! { dg-final { scan-rtl-dump-times "\\\(call \[^\\n\]* acc_on_device" 1 "expand" } }
> >  
> >  ! { dg-final { cleanup-rtl-dump "expand" } }
> > diff --git a/gfortran.dg/goacc/acc_on_device-2.f95 b/gfortran.dg/goacc/acc_on_device-2.f95
> > index 7730a60..43ad022 100644
> > --- a/gfortran.dg/goacc/acc_on_device-2.f95
> > +++ b/gfortran.dg/goacc/acc_on_device-2.f95
> > @@ -35,6 +35,6 @@ end function f
> >  
> >  ! With -fopenacc, we're expecting the builtin to be expanded, so no calls.
> >  ! TODO: not working.
> > -! { dg-final { scan-rtl-dump-times "\\\(call \[^\\n\]*\\\"acc_on_device" 0 "expand" { xfail *-*-* } } }
> > +! { dg-final { scan-rtl-dump-times "\\\(call \[^\\n\]* acc_on_device" 0 "expand" { xfail *-*-* } } }
> >  
> >  ! { dg-final { cleanup-rtl-dump "expand" } }


GrÃÃe,
 Thomas

Attachment: signature.asc
Description: PGP signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]