| Message ID | 5537DC12.1050201@arm.com |
|---|---|
| State | New |
| Headers | show |
On 22 April 2015 at 19:36, Alan Lawrence <alan.lawrence@arm.com> wrote: > In the first revision of Christophe Lyon's advsimd-intrinsics tests, > https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00532.html , both > gcc-dg-runtest (to assemble only) and c-torture-execute were used. In review > the gcc-dg-runtest part was then dropped, and execution tests continued > using c-torture-execute. However, c-torture-execute ignores e.g. dg-options > directives in the individual test files, whereas gcc-dg-runtest does not. > > This patch switches to gcc-dg-runtest (with dg-do-what-default = "run") for > all tests, allowing use of e.g. dg-options (in testsuite patch 3/3). This Sandra has recently committed a slightly different patch. If you want to update your, here are few comments/questions: - why do you add "-w" to additional_flags? - you changed the way we iterate over the tests, but this removes the possiblity to actually execute only a subset of the available tests, such as RUNTESTFLAGS=advsimd-intrinsics.exp=vadd.c Christophe. > generally seems to work OK - indeed I also dropped the > parallelization-disabling code - and the new advsimd-intrinsics.exp now > follows gcc.c-torture/compile/compile.exp and > gcc.c-torture/execute/execute.exp very closely. However, there are side > effects, of which we should be aware, but with which I think we can live, > specifically: > > (1) the lines in the test log change from... > > PASS: gcc.target/aarch64/advsimd-intrinsics/vcombine.c compilation, -O1 > PASS: gcc.target/aarch64/advsimd-intrinsics/vcombine.c execution, -O1 > > ...to... > > PASS: gcc.target/aarch64/advsimd-intrinsics/vcombine.c -O1 execution test > > (that is, the compilation line disappears, but the (test for excess errors) > remains unchanged) > > (2) The "-Og -g" variant is no longer tested; all of -O0, -O1, -O2, -O2 > -flto -fno-use-linker-plugin -flto-partition=none, -O2 -flto > -fuse-linker-plugin -fno-fat-lto-objects, -O3 -fomit-frame-pointer, -O3 -g, > -Os still are. My feeling is that this set of options is exhaustive enough. > > Cross-tested arm-none-eabi, aarch64-none-elf, aarch64_be-none-elf; natively > tested arm-none-linux-gnueabihf and aarch64-none-linux-gnu. > > gcc/testsuite/ChangeLog: > > * gcc.target/aarch64/advsimd-intrinsics/advsimd-intrinsics.exp: > Use gcc-dg-runtest.
Christophe Lyon wrote: > On 22 April 2015 at 19:36, Alan Lawrence <alan.lawrence@arm.com> wrote: >> In the first revision of Christophe Lyon's advsimd-intrinsics tests, >> https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00532.html , both >> gcc-dg-runtest (to assemble only) and c-torture-execute were used. In review >> the gcc-dg-runtest part was then dropped, and execution tests continued >> using c-torture-execute. However, c-torture-execute ignores e.g. dg-options >> directives in the individual test files, whereas gcc-dg-runtest does not. >> >> This patch switches to gcc-dg-runtest (with dg-do-what-default = "run") for >> all tests, allowing use of e.g. dg-options (in testsuite patch 3/3). This > > Sandra has recently committed a slightly different patch. > > If you want to update your, here are few comments/questions: > - why do you add "-w" to additional_flags? Hmmm. Not sure now. I agree, it appears to work without, so will drop that. > - you changed the way we iterate over the tests, but this removes the > possiblity to actually execute only a subset of the available tests, > such as RUNTESTFLAGS=advsimd-intrinsics.exp=vadd.c I don't see this symptom - I am able to execute such subsets with either my, or Sandra's, advsimd-intrinsics.exp. Is it that you have to check runtest_file_p because you are setting gcc_parallel_test_enable to 0? I'm doing more testing now, but I think I can drop my advsimd-intrinsics.exp changes altogether; I'll post an updated patch series shortly. In the meantime I'm curious as to why you found the gcc_parallel_test_enable necessary? (And is it safe to reset it to 1 afterwards, rather than to a saved value?) TYVM for your other comments and review - I will incorporate all into my next revision. Thanks, Alan
On 26 May 2015 at 18:25, Alan Lawrence <alan.lawrence@arm.com> wrote: > Christophe Lyon wrote: >> >> On 22 April 2015 at 19:36, Alan Lawrence <alan.lawrence@arm.com> wrote: >>> >>> In the first revision of Christophe Lyon's advsimd-intrinsics tests, >>> https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00532.html , both >>> gcc-dg-runtest (to assemble only) and c-torture-execute were used. In >>> review >>> the gcc-dg-runtest part was then dropped, and execution tests continued >>> using c-torture-execute. However, c-torture-execute ignores e.g. >>> dg-options >>> directives in the individual test files, whereas gcc-dg-runtest does not. >>> >>> This patch switches to gcc-dg-runtest (with dg-do-what-default = "run") >>> for >>> all tests, allowing use of e.g. dg-options (in testsuite patch 3/3). This >> >> >> Sandra has recently committed a slightly different patch. >> >> If you want to update your, here are few comments/questions: >> - why do you add "-w" to additional_flags? > > > Hmmm. Not sure now. I agree, it appears to work without, so will drop that. > >> - you changed the way we iterate over the tests, but this removes the >> possiblity to actually execute only a subset of the available tests, >> such as RUNTESTFLAGS=advsimd-intrinsics.exp=vadd.c > > > I don't see this symptom - I am able to execute such subsets with either my, > or Sandra's, advsimd-intrinsics.exp. I didn't try to run with your patch, I thought it was an oversight of yours. Sorry, indeed I've just checked that gcc-dg-runtest includes the filter. > > Is it that you have to check runtest_file_p because you are setting > gcc_parallel_test_enable to 0? > > I'm doing more testing now, but I think I can drop my advsimd-intrinsics.exp > changes altogether; I'll post an updated patch series shortly. > > In the meantime I'm curious as to why you found the gcc_parallel_test_enable > necessary? (And is it safe to reset it to 1 afterwards, rather than to a > saved value?) See https://gcc.gnu.org/ml/gcc/2014-10/msg00081.html > > TYVM for your other comments and review - I will incorporate all into my > next revision. Thanks. > > Thanks, Alan >
diff --git a/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/advsimd-intrinsics.exp b/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/advsimd-intrinsics.exp index 551299ef5634b7fea68d2e2f813ab61270b59e35..aa5d4d5ec2f45fa745fac152125e1badc2f2df43 100644 --- a/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/advsimd-intrinsics.exp +++ b/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/advsimd-intrinsics.exp @@ -26,10 +26,6 @@ if {![istarget arm*-*-*] load_lib gcc-dg.exp # Initialize `dg'. -load_lib c-torture.exp -load_lib target-supports.exp -load_lib torture-options.exp - dg-init if {[istarget arm*-*-*] @@ -37,29 +33,14 @@ if {[istarget arm*-*-*] return } -torture-init -set-torture-options $C_TORTURE_OPTIONS {{}} $LTO_TORTURE_OPTIONS - # Make sure Neon flags are provided, if necessary. -set additional_flags [add_options_for_arm_neon ""] +set additional_flags [add_options_for_arm_neon "-w"] # Main loop. -foreach src [lsort [glob -nocomplain $srcdir/$subdir/*.c]] { - # If we're only testing specific files and this isn't one of them, skip it. - if ![runtest_file_p $runtests $src] then { - continue - } - - # runtest_file_p is already run above, and the code below can run - # runtest_file_p again, make sure everything for this test is - # performed if the above runtest_file_p decided this runtest - # instance should execute the test - gcc_parallel_test_enable 0 - c-torture-execute $src $additional_flags - gcc-dg-runtest $src "" $additional_flags - gcc_parallel_test_enable 1 -} +set saved-dg-do-what-default ${dg-do-what-default} +set dg-do-what-default "run" +gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.\[cS\]]] "" ${additional_flags} +set dg-do-what-default ${saved-dg-do-what-default} # All done. -torture-finish dg-finish