On 09/08/2016 06:30 PM, David Malcolm wrote:
This patch uses rtl_dump_test to start building out a test suite
for cse.
I attempted to create a reproducer for PR 71779; however I'm not yet
able to replicate the bogus cse reported there via the test case.
gcc/ChangeLog:
* cse.c: Include selftest.h and selftest-rtl.h.
(selftest::test_simple_cse): New function.
(selftest::test_pr71779): New function.
(selftest::cse_c_tests): New function.
* selftest-run-tests.c (selftest::run_tests): Call
selftest::cse_c_tests.
* selftest.h (selftest::cse_c_tests): New decl.
So as I look at this, the first thing that comes to mind is whether or
not to refine the dump output.
There's a lot of useless crap in there that really only helps folks that
sitting inside a debugger dumping hunks of RTL (I'm thinking
specifically about the pointers back to tree nodes for DECLs). Those
addresses are useless in other contexts.
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.
---
gcc/cse.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++
gcc/selftest-run-tests.c | 1 +
gcc/selftest.h | 1 +
3 files changed, 111 insertions(+)
diff --git a/gcc/cse.c b/gcc/cse.c
index 0bfd7ff..f4f06fe 100644
--- a/gcc/cse.c
+++ b/gcc/cse.c
@@ -41,6 +41,8 @@ along with GCC; see the file COPYING3. If not see
#include "tree-pass.h"
#include "dbgcnt.h"
#include "rtl-iter.h"
+#include "selftest.h"
+#include "selftest-rtl.h"
#ifndef LOAD_EXTEND_OP
#define LOAD_EXTEND_OP(M) UNKNOWN
@@ -7773,3 +7775,110 @@ make_pass_cse_after_global_opts (gcc::context *ctxt)
{
return new pass_cse_after_global_opts (ctxt);
}
+
+#if CHECKING_P
+
+namespace selftest {
+
+/* Selftests for CSE. */
+
+/* Simple test of eliminating a redundant (reg + 1) computation
+ i.e. that:
+ r101 = r100 + 1;
+ r102 = r100 + 1; <<< common subexpression
+ *r103 = r101 * r102;
+ can be CSE-ed to:
+ r101 = r100 + 1;
+ r102 = r101; <<< replaced
+ *r103 = r101 * r102;
+ by cse_main. */
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.
+
+static void
+test_simple_cse ()
+{
+ /* Only run this tests for i386. */
+#ifndef I386_OPTS_H
+ return;
+#endif
+
+ const char *input_dump
+ = (/* "r101 = r100 + 1;" */
+ "(insn 1 0 2 2 (set (reg:SI 101)\n"
+ " (plus:SI (reg:SI 100)\n"
+ " (const_int 1 [0x1]))) -1 (nil))\n"
+ /* "r102 = r100 + 1;" */
+ "(insn 2 1 3 2 (set (reg:SI 102)\n"
+ " (plus:SI (reg:SI 100)\n"
+ " (const_int 1 [0x1]))) -1 (nil))\n"
+ /* "*r103 = r101 * r102;" */
+ "(insn 3 2 0 2 (set (mem:SI (reg:SI 103) [1 i+0 S4 A32])\n"
+ " (mult:SI (reg:SI 101) (reg:SI 102))) -1 (nil))\n"
+ );
I don't think we need to comment each RTL insn for those which are so
obvious. It just adds to the visual clutter.
+
+ /* 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?
