Message ID | 20150204.084128.280634107.kkojima@rr.iij4u.or.jp |
---|---|
State | New |
Headers | show |
Hi Kaz! 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! > 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
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
Thomas Schwinge <thomas@codesourcery.com> wrote: > To resolve the immediate problem: is my approval "enough" for Kaz to > commit the patch, or does that need a "more authoritative approval"? I'd like to commit my patch as a "quick fix" in a few days if no one objects. > 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.) Another example with the asm output of m68k compiler: lea acc_on_device,%a2 jsr (%a2) ... jsr (%a2) ... jsr (%a2) ... jsr (%a2) for -fopenacc -O -fno-openacc acc_on_device-1.c. It seems that getting the number of calls for the specific function isn't easy with the asm output on some targets. Regards, kaz
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" } }