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
