Message ID | 1474505970-4753-1-git-send-email-dmalcolm@redhat.com |
---|---|
State | New |
Headers | show |
On 09/21/2016 06:59 PM, David Malcolm wrote: > On Mon, 2016-09-19 at 11:31 -0600, Jeff Law wrote: >> On 09/16/2016 03:19 PM, David Malcolm wrote: >>> >>>> When possible I don't think we want the tests to be target >>>> specific. >>>> Hmm, I'm probably about to argue for Bernd's work... The 71779 >>>> testcase >>>> is a great example -- it uses high/lo_sum. Not all targets >>>> support >>>> that >>>> -- as long as we don't try to recognize those insns we're likely >>>> OK, >>>> but >>>> that seems fragile long term. Having an idealized target means >>>> we >>>> can >>>> ignore much of these issues. >>> >>> An alternative would be to pick a specific target for each test. >> It's an alternative, but not one I particularly like since those >> tests >> won't be consistently run. With an abstracted target like Bernd >> suggests we ought to be able to make most tests work with the >> abstracted >> target and minimize the number of truely target specific tests. >> >> >>>> So I'm real curious, what happens if you run this RTL selftest >>>> under >>>> valgrind? I have the sneaking suspicion that we'll start doing >>>> some >>>> uninitialized memory reads. >>> >>> I'm seeing various leaks (an htab within linemap_init for all of >>> the >>> line_table fixtures), but no uninitialized reads. >> Wow. I must say I'm surprised. Good news though. >> >> >>>> + >>>>> + /* Dump taken from comment 2 of PR 71779, of >>>>> + "...the relevant memory access coming out of expand" >>>>> + with basic block IDs added, and prev/next insns set to >>>>> + 0 at ends. */ >>>>> + const char *input_dump >>>>> + = (";; MEM[(struct isl_obj *)&obj1] = >>>>> &isl_obj_map_vtable;\n" >>>>> + "(insn 1045 0 1046 2 (set (reg:SI 480)\n" >>>>> + " (high:SI (symbol_ref:SI >>>>> (\"isl_obj_map_vtable\") >>>>> [flags 0xc0] <var_decl 0x7fa0363ea240 isl_obj_map_vtable>))) >>>>> y.c:12702 -1\n" >>>>> + " (nil))\n" >>>>> + "(insn 1046 1045 1047 2 (set (reg/f:SI 479)\n" >>>>> + " (lo_sum:SI (reg:SI 480)\n" >>>>> + " (symbol_ref:SI (\"isl_obj_map_vtable\") >>>>> [flags >>>>> 0xc0] <var_decl 0x7fa0363ea240 isl_obj_map_vtable>))) y.c:12702 >>>>> -1\n" >>>>> + " (expr_list:REG_EQUAL (symbol_ref:SI >>>>> (\"isl_obj_map_vtable\") [flags 0xc0] <var_decl 0x7fa0363ea240 >>>>> isl_obj_map_vtable>)\n" >>>>> + " (nil)))\n" >>>>> + "(insn 1047 1046 1048 2 (set (reg:DI 481)\n" >>>>> + " (subreg:DI (reg/f:SI 479) 0)) y.c:12702 -1\n" >>>>> + " (nil))\n" >>>>> + "(insn 1048 1047 1049 2 (set (zero_extract:DI (reg/v:DI >>>>> 191 >>>>> [ obj1D.17368 ])\n" >>>>> + " (const_int 32 [0x20])\n" >>>>> + " (const_int 0 [0]))\n" >>>>> + " (reg:DI 481)) y.c:12702 -1\n" >>>>> + " (nil))\n" >>>> So looking at this just makes my head hurt. Escaping, lots of >>>> quotes, >>>> unnecessary things in the dump, etc. The question I would have >>>> is >>>> why >>>> not have a file with the dump and read the file? >>> >>> (nods) >>> >>> Seems like I need to add a mechanism for telling the selftests >>> which >>> directory to load the tests relative to. >> What about putting them inside the appropriate gcc.target >> directories? >> We could have one for the "generic" target assuming we build >> something >> like that, the others could live in their target specific directory. >> >> >> jeff > > Having selftests that read RTL dumps load them from files rather than > embedding them as strings in the source requires a way to locate the > relevant files. > > This patch adds a selftest::locate_file function for locating such > files, relative to "$(SRCDIR)/gcc/testsuite/selftests". This is > done via a new argument to -fself-test, which supplies the current > value of "$(SRCDIR)/gcc" to cc1. > > I chose "$(SRCDIR)/gcc/testsuite/selftests", so as to be below > gcc/testsuite, but not below any of the existing DejaGnu subdirectories, > to avoid selftest-specific files from being picked up by .exp globbing > patterns. We could add target-specific directories below that dir if > necessary. > > I've rewritten the rest of the patch kit to use this to load from .rtl > dump files within that directory, rather than embedding the dumps as > string literals in the C source. > > The patch also exposes a selftests::path_to_src_gcc, which could be > used by a selftest to e.g. load a DejaGnu file, so that if need be > we could share .rtl input files between both -fself-test tests and > DejaGnu-based tests for the .rtl frontend. > > (Depends on the approved-when-needed > "[PATCH 2/9] Add selftest::read_file" > https://gcc.gnu.org/ml/gcc-patches/2016-09/msg00476.html ). > > Successfully bootstrapped®rtested on x86_64-pc-linux-gnu. > > OK for trunk once the dependencies are in? > > gcc/ChangeLog: > * Makefile.in (s-selftest) Add $(srcdir) as an argument of > -fself-test. > (selftest-gdb): Likewise. > (selftest-valgrind): Likewise. > * common.opt (fself-test): Rename to... > (fself-test=): ...this, documenting the meaning of the argument. > * selftest-run-tests.c: Include "options.h". > (selftest::run_tests): Initialize selftest::path_to_src_gcc from > flag_self_test. > * selftest.c (selftest::path_to_src_gcc): New global. > (selftest::locate_file): New function. > (selftest::test_locate_file): New function. > (selftest::selftest_c_tests): Call test_locate_file. > * selftest.h (selftest::locate_file): New decl. > (selftest::path_to_src_gcc): New decl. > > gcc/testsuite/ChangeLog: > * gcc.dg/cpp/pr71591.c: Add a fake value for the argument of > -fself-test. > * selftests/example.txt: New file. I do think we should go ahead and plan for a target subdirectory. With that added, this should be OK once dependencies are in. jeff
diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 332c85e..94146ae 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -1876,18 +1876,19 @@ rest.cross: specs .PHONY: selftest selftest: s-selftest s-selftest: $(GCC_PASSES) cc1$(exeext) stmp-int-hdrs - $(GCC_FOR_TARGET) -xc -S -c /dev/null -fself-test + $(GCC_FOR_TARGET) -xc -S -c /dev/null -fself-test=$(srcdir) $(STAMP) $@ # Convenience method for running selftests under gdb: .PHONY: selftest-gdb selftest-gdb: $(GCC_PASSES) cc1$(exeext) stmp-int-hdrs - $(GCC_FOR_TARGET) -xc -S -c /dev/null -fself-test -wrapper gdb,--args + $(GCC_FOR_TARGET) -xc -S -c /dev/null -fself-test=$(srcdir) \ + -wrapper gdb,--args # Convenience method for running selftests under valgrind: .PHONY: selftest-valgrind selftest-valgrind: $(GCC_PASSES) cc1$(exeext) stmp-int-hdrs - $(GCC_FOR_TARGET) -xc -S -c /dev/null -fself-test \ + $(GCC_FOR_TARGET) -xc -S -c /dev/null -fself-test=$(srcdir) \ -wrapper valgrind,--leak-check=full # Recompile all the language-independent object files. diff --git a/gcc/common.opt b/gcc/common.opt index fa1c036..603fade 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -2098,9 +2098,9 @@ fselective-scheduling2 Common Report Var(flag_selective_scheduling2) Optimization Run selective scheduling after reload. -fself-test -Common Undocumented Var(flag_self_test) -Run self-tests. +fself-test= +Common Undocumented Joined Var(flag_self_test) +Run self-tests, using the given path to locate test files. fsel-sched-pipelining Common Report Var(flag_sel_sched_pipelining) Init(0) Optimization diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c index 54a9b0f..ecc3d71 100644 --- a/gcc/selftest-run-tests.c +++ b/gcc/selftest-run-tests.c @@ -23,6 +23,7 @@ along with GCC; see the file COPYING3. If not see #include "selftest.h" #include "tree.h" #include "langhooks.h" +#include "options.h" /* This function needed to be split out from selftest.c as it references tests from the whole source tree, and so is within @@ -37,6 +38,12 @@ along with GCC; see the file COPYING3. If not see void selftest::run_tests () { + /* Makefile.in has -fself-test=$(srcdir), so that flag_self_test + contains the path to the "gcc" subdirectory of the source tree + (without a trailing slash). Copy it up to path_to_src_gcc, to + avoid selftest.c depending on option-handling. */ + path_to_src_gcc = flag_self_test; + long start_time = get_run_time (); /* Run all the tests, in hand-coded order of (approximate) dependencies: diff --git a/gcc/selftest.c b/gcc/selftest.c index cf7031f..2a481ad 100644 --- a/gcc/selftest.c +++ b/gcc/selftest.c @@ -198,6 +198,21 @@ read_file (const location &loc, const char *path) return result; } +/* The path of SRCDIR/gcc. */ + +const char *path_to_src_gcc = NULL; + +/* Convert a path relative to SRCDIR/gcc/testsuite/selftests + to a real path (either absolute, or relative to pwd). + The result should be freed by the caller. */ + +char * +locate_file (const char *name) +{ + ASSERT_NE (NULL, path_to_src_gcc); + return concat (path_to_src_gcc, "/testsuite/selftests/", name, NULL); +} + /* Selftests for the selftest system itself. */ /* Sanity-check the ASSERT_ macros with various passing cases. */ @@ -240,6 +255,18 @@ test_read_file () free (buf); } +/* Verify locate_file (and read_file). */ + +static void +test_locate_file () +{ + char *path = locate_file ("example.txt"); + char *buf = read_file (SELFTEST_LOCATION, path); + ASSERT_STREQ ("example of a selftest file\n", buf); + free (buf); + free (path); +} + /* Run all of the selftests within this file. */ void @@ -248,6 +275,7 @@ selftest_c_tests () test_assertions (); test_named_temp_file (); test_read_file (); + test_locate_file (); } } // namespace selftest diff --git a/gcc/selftest.h b/gcc/selftest.h index 86ad14c..8b9c007 100644 --- a/gcc/selftest.h +++ b/gcc/selftest.h @@ -187,6 +187,16 @@ class temp_override extern void forcibly_ggc_collect (); +/* Convert a path relative to SRCDIR/gcc/testsuite/selftests + to a real path (either absolute, or relative to pwd). + The result should be freed by the caller. */ + +extern char *locate_file (const char *path); + +/* The path of SRCDIR/gcc. */ + +extern const char *path_to_src_gcc; + /* Declarations for specific families of tests (by source file), in alphabetical order. */ extern void bitmap_c_tests (); diff --git a/gcc/testsuite/gcc.dg/cpp/pr71591.c b/gcc/testsuite/gcc.dg/cpp/pr71591.c index e92cb52..0e3d7b1 100644 --- a/gcc/testsuite/gcc.dg/cpp/pr71591.c +++ b/gcc/testsuite/gcc.dg/cpp/pr71591.c @@ -1,5 +1,5 @@ /* PR rtl-optimization/71591 */ /* { dg-do preprocess } */ -/* { dg-options "-fself-test" } */ +/* { dg-options "-fself-test=fake-value" } */ /* { dg-message "self-tests incompatible with -E" "" { target *-*-* } 0 } */ diff --git a/gcc/testsuite/selftests/example.txt b/gcc/testsuite/selftests/example.txt new file mode 100644 index 0000000..fbfaa33 --- /dev/null +++ b/gcc/testsuite/selftests/example.txt @@ -0,0 +1 @@ +example of a selftest file