diff mbox

[8/9] final.c selftests

Message ID 1473381053-18817-9-git-send-email-dmalcolm@redhat.com
State New
Headers show

Commit Message

David Malcolm Sept. 9, 2016, 12:30 a.m. UTC
gcc/ChangeLog:
	* final.c: Include selftest.h and selftest-rtl.h.
	(class selftest::temp_asm_out): New subclass of
	selftest::named_temp_file.
	(selftest::temp_asm_out::temp_asm_out): New ctor.
	(selftest::temp_asm_out::~temp_asm_out): New dtor.
	(class selftest::asm_out_test): New subclass of
	selftest::rtl_dump_test.
	(selftest::asm_out_test::asm_out_test): New ctor.
	(selftest::test_jump_insn): New function.
	(selftest::test_empty_function): New function.
	(selftest::test_asm_for_insn): New function.
	(TEST_ASM_FOR_INSN): New macro.
	(selftest::test_x86_64_leal): New function.
	(selftest::test_x86_64_negl): New function.
	(selftest::test_x86_64_cmpl): New function.
	(selftest::test_x86_64_cmovge): New function.
	(selftest::test_x86_64_ret): New function.
	(selftest::final_c_tests): New function.
	* selftest-run-tests.c (selftest::run_tests): Call
	selftest::final_c_tests.
	* selftest.h (selftest::final_c_tests): New decl.
---
 gcc/final.c              | 271 +++++++++++++++++++++++++++++++++++++++++++++++
 gcc/selftest-run-tests.c |   1 +
 gcc/selftest.h           |   1 +
 3 files changed, 273 insertions(+)

Comments

Jeff Law Sept. 16, 2016, 8:45 p.m. UTC | #1
On 09/08/2016 06:30 PM, David Malcolm wrote:
> gcc/ChangeLog:
> 	* final.c: Include selftest.h and selftest-rtl.h.
> 	(class selftest::temp_asm_out): New subclass of
> 	selftest::named_temp_file.
> 	(selftest::temp_asm_out::temp_asm_out): New ctor.
> 	(selftest::temp_asm_out::~temp_asm_out): New dtor.
> 	(class selftest::asm_out_test): New subclass of
> 	selftest::rtl_dump_test.
> 	(selftest::asm_out_test::asm_out_test): New ctor.
> 	(selftest::test_jump_insn): New function.
> 	(selftest::test_empty_function): New function.
> 	(selftest::test_asm_for_insn): New function.
> 	(TEST_ASM_FOR_INSN): New macro.
> 	(selftest::test_x86_64_leal): New function.
> 	(selftest::test_x86_64_negl): New function.
> 	(selftest::test_x86_64_cmpl): New function.
> 	(selftest::test_x86_64_cmovge): New function.
> 	(selftest::test_x86_64_ret): New function.
> 	(selftest::final_c_tests): New function.
> 	* selftest-run-tests.c (selftest::run_tests): Call
> 	selftest::final_c_tests.
> 	* selftest.h (selftest::final_c_tests): New decl.
I'm really not sure how useful these tests are going to be and would 
question the long term maintenance costs of keeping them up-to-date.

I could see perhaps verifying that when there are multiple alternatives 
that the correct one is selected or somesuch, but these tests really 
don't seem to be covering anything particularly useful.

Jeff
David Malcolm Sept. 16, 2016, 9:34 p.m. UTC | #2
On Fri, 2016-09-16 at 14:45 -0600, Jeff Law wrote:
> On 09/08/2016 06:30 PM, David Malcolm wrote:
> > gcc/ChangeLog:
> > 	* final.c: Include selftest.h and selftest-rtl.h.
> > 	(class selftest::temp_asm_out): New subclass of
> > 	selftest::named_temp_file.
> > 	(selftest::temp_asm_out::temp_asm_out): New ctor.
> > 	(selftest::temp_asm_out::~temp_asm_out): New dtor.
> > 	(class selftest::asm_out_test): New subclass of
> > 	selftest::rtl_dump_test.
> > 	(selftest::asm_out_test::asm_out_test): New ctor.
> > 	(selftest::test_jump_insn): New function.
> > 	(selftest::test_empty_function): New function.
> > 	(selftest::test_asm_for_insn): New function.
> > 	(TEST_ASM_FOR_INSN): New macro.
> > 	(selftest::test_x86_64_leal): New function.
> > 	(selftest::test_x86_64_negl): New function.
> > 	(selftest::test_x86_64_cmpl): New function.
> > 	(selftest::test_x86_64_cmovge): New function.
> > 	(selftest::test_x86_64_ret): New function.
> > 	(selftest::final_c_tests): New function.
> > 	* selftest-run-tests.c (selftest::run_tests): Call
> > 	selftest::final_c_tests.
> > 	* selftest.h (selftest::final_c_tests): New decl.
> I'm really not sure how useful these tests are going to be and would 
> question the long term maintenance costs of keeping them up-to-date.
> 
> I could see perhaps verifying that when there are multiple
> alternatives 
> that the correct one is selected or somesuch, but these tests really 
> don't seem to be covering anything particularly useful.

My thinking here was that it might be useful to verify insn recognition
and output when someone is bringing up a new target, or adding new
insns to a .md file; the selftest::test_x86_64_cmpl show the beginnings
of how one might write that in selftest form.

But I'm happy to drop it.
Dave
Jeff Law Sept. 19, 2016, 9:34 p.m. UTC | #3
On 09/16/2016 03:34 PM, David Malcolm wrote:
> On Fri, 2016-09-16 at 14:45 -0600, Jeff Law wrote:
>> On 09/08/2016 06:30 PM, David Malcolm wrote:
>>> gcc/ChangeLog:
>>> 	* final.c: Include selftest.h and selftest-rtl.h.
>>> 	(class selftest::temp_asm_out): New subclass of
>>> 	selftest::named_temp_file.
>>> 	(selftest::temp_asm_out::temp_asm_out): New ctor.
>>> 	(selftest::temp_asm_out::~temp_asm_out): New dtor.
>>> 	(class selftest::asm_out_test): New subclass of
>>> 	selftest::rtl_dump_test.
>>> 	(selftest::asm_out_test::asm_out_test): New ctor.
>>> 	(selftest::test_jump_insn): New function.
>>> 	(selftest::test_empty_function): New function.
>>> 	(selftest::test_asm_for_insn): New function.
>>> 	(TEST_ASM_FOR_INSN): New macro.
>>> 	(selftest::test_x86_64_leal): New function.
>>> 	(selftest::test_x86_64_negl): New function.
>>> 	(selftest::test_x86_64_cmpl): New function.
>>> 	(selftest::test_x86_64_cmovge): New function.
>>> 	(selftest::test_x86_64_ret): New function.
>>> 	(selftest::final_c_tests): New function.
>>> 	* selftest-run-tests.c (selftest::run_tests): Call
>>> 	selftest::final_c_tests.
>>> 	* selftest.h (selftest::final_c_tests): New decl.
>> I'm really not sure how useful these tests are going to be and would
>> question the long term maintenance costs of keeping them up-to-date.
>>
>> I could see perhaps verifying that when there are multiple
>> alternatives
>> that the correct one is selected or somesuch, but these tests really
>> don't seem to be covering anything particularly useful.
>
> My thinking here was that it might be useful to verify insn recognition
> and output when someone is bringing up a new target, or adding new
> insns to a .md file; the selftest::test_x86_64_cmpl show the beginnings
> of how one might write that in selftest form.
Understood.  I'm just not sure how helpful that will be in practice and 
it seems like a maintenance nightmare.

The case where I think something like this is useful is when there's 
multiple ways to express the same insn and there's a preferred form. 
Say because the preferred form is smaller, faster, whatever.

But even that can be incredibly painful.  Consider zero-ing a register 
on the x86.  There's mov 0,reg; xor reg, reg; sub reg,reg, etc.  Which 
is preferred depends on the micro-architecture and possibly surrounding 
context, whether or not the containing block is hot or not, etc.


>
> But I'm happy to drop it.
I don't think it buys us much right now.  It is archived if we end up 
changing our minds.

jeff
diff mbox

Patch

diff --git a/gcc/final.c b/gcc/final.c
index eccc3d8..990f898 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -78,6 +78,8 @@  along with GCC; see the file COPYING3.  If not see
 #include "asan.h"
 #include "rtl-iter.h"
 #include "print-rtl.h"
+#include "selftest.h"
+#include "selftest-rtl.h"
 
 #ifdef XCOFF_DEBUGGING_INFO
 #include "xcoffout.h"		/* Needed for external data declarations.  */
@@ -4894,3 +4896,272 @@  get_call_reg_set_usage (rtx_insn *insn, HARD_REG_SET *reg_set,
   COPY_HARD_REG_SET (*reg_set, default_set);
   return false;
 }
+
+#if CHECKING_P
+
+namespace selftest {
+
+/* Fixture for temporarily setting the global "asm_out_file"
+   to a named temporary file.  */
+
+class temp_asm_out : public named_temp_file
+{
+ public:
+  temp_asm_out (const location &loc);
+  ~temp_asm_out ();
+};
+
+/* Constructor.  Open a tempfile for writing and set "asm_out_file" to it.  */
+
+temp_asm_out::temp_asm_out (const location &loc)
+: named_temp_file (".s")
+{
+  gcc_assert (asm_out_file == NULL);
+  asm_out_file = fopen (get_filename (), "w");
+  if (!asm_out_file)
+    ::selftest::fail_formatted (loc, "unable to open tempfile: %s",
+				get_filename ());
+}
+
+/* Destructor.  Close the tempfile and unset "asm_out_file".
+   The tempfile is unlinked by the named_temp_file dtor.  */
+
+temp_asm_out::~temp_asm_out ()
+{
+  fclose (asm_out_file);
+  asm_out_file = NULL;
+}
+
+/* A subclass of rtl_dump_test for testing asm output.
+   Overrides asm_out_file to a tempfile, and temporarily
+   sets "reload_completed = 1;".  */
+
+class asm_out_test : public rtl_dump_test
+{
+ public:
+  asm_out_test (const location &loc, const char *dump_content);
+
+  /* Get the asm output written so far.  The caller should free
+     the returned ptr.  */
+  char *
+  get_output (const location &loc) const
+  {
+    fflush (asm_out_file);
+    return read_file (loc, m_asm_out.get_filename ());
+  }
+
+ private:
+  temp_asm_out m_asm_out;
+  temp_override <int> m_override_reload_completed;
+  temp_override <section *> m_override_in_section;
+};
+
+/* asm_out_test's constructor.  Write DUMP_CONTENT to a tempfile,
+   overrides "asm_out_file" to a tempfile, and temporarily
+   sets "reload_completed = 1;".
+   Assume that no pseudo regs are present in DUMP_CONTENT (by
+   using the default arg to rtl_dump_test's ctor).  */
+
+asm_out_test::asm_out_test (const location &loc, const char *dump_content)
+: rtl_dump_test (dump_content),
+  m_asm_out (loc),
+  /* Temporarily set reload_completed = true.
+     Needed e.g. by ix86_can_use_return_insn_p.  */
+  m_override_reload_completed (reload_completed, true),
+  /* Temporarily set "in_section = NULL;".  */
+  m_override_in_section (in_section, NULL)
+{
+}
+
+/* Test writing asm for a jump_insn.  */
+
+static void
+test_jump_insn ()
+{
+  const char *input_dump
+    = ("(jump_insn 1 0 2 4 (set (pc)\n"
+       "        (label_ref 3)) ../../src/gcc/testsuite/rtl.dg/test.c:4 -1\n"
+       "     (nil)\n"
+       " -> 3)\n"
+       "(barrier 2 1 3)\n"
+       "(code_label 3 2 0 5 2 (nil) [1 uses])\n");
+  asm_out_test t (SELFTEST_LOCATION, input_dump);
+
+  rtx_insn *jump_insn = get_insn_by_uid (1);
+  ASSERT_EQ (JUMP_INSN, GET_CODE (jump_insn));
+
+  rtx_insn *barrier = get_insn_by_uid (2);
+  ASSERT_EQ (BARRIER, GET_CODE (barrier));
+
+  rtx_insn *code_label = get_insn_by_uid (3);
+  ASSERT_EQ (CODE_LABEL, GET_CODE (code_label));
+
+  int seen;
+  final_scan_insn (jump_insn, stderr, 0, 0, &seen);
+
+  char *c = t.get_output (SELFTEST_LOCATION);
+  ASSERT_STREQ ("\tjmp\t.L2\n", c);
+  free (c);
+}
+
+/* Test writing asm for an empty function.  */
+
+static void
+test_empty_function ()
+{
+  /* Dump of the dump from cc1 in "test.c.289r.dwarf2" given this input:
+       void test_1 (void) {}
+     and compiling with -Os (for x86_64).  */
+  const char *dump
+    = (";; Function test_1 (test_1, funcdef_no=0, decl_uid=1758, cgraph_uid=0, symbol_order=0)\n"
+       "(note 1 0 3 (nil) NOTE_INSN_DELETED)\n"
+       "(note 3 1 8 2 [bb 2] NOTE_INSN_BASIC_BLOCK)\n"
+       "(note 8 3 2 2 NOTE_INSN_PROLOGUE_END)\n"
+       "(note 2 8 9 2 NOTE_INSN_FUNCTION_BEG)\n"
+       "(note 9 2 10 2 NOTE_INSN_EPILOGUE_BEG)\n"
+       "(jump_insn:TI 10 9 11 2 (simple_return) test.c:3 697 {simple_return_internal}\n"
+       "     (nil)\n"
+       " -> simple_return)\n"
+       "(barrier 11 10 7)\n"
+       "(note 7 11 0 (nil) NOTE_INSN_DELETED)\n");
+
+  asm_out_test t (SELFTEST_LOCATION, dump);
+
+  shorten_branches (get_insns ());
+  rest_of_handle_final ();
+
+  /* Verify that some asm was written out.  */
+  char *c = t.get_output (SELFTEST_LOCATION);
+  if (0)
+    fprintf (stdout, "%s", c);
+  ASSERT_STR_CONTAINS (c, "\t.text\n");
+  ASSERT_STR_CONTAINS (c, "\n\t.type\ttest_1, @function\n");
+  ASSERT_STR_CONTAINS (c, "\ntest_1:\n");
+  ASSERT_STR_CONTAINS (c, "\n\t.cfi_startproc\n");
+  ASSERT_STR_CONTAINS (c, "\n\tret\n");
+  ASSERT_STR_CONTAINS (c, "\n\t.cfi_endproc\n");
+  ASSERT_STR_CONTAINS (c, "\n\t.size\ttest_1, .-test_1\n");
+  free (c);
+}
+
+/* Parse DUMP via an asm_out_test and run final_scan_insn on the first
+   insn seen in the dump.  Verify that the generated asm equals
+   EXPECTED_ASM.  Use LOC as the effective location when reporting any
+   errors.  */
+
+static void
+test_asm_for_insn (const location &loc, const char *dump,
+		   const char *expected_asm)
+{
+  asm_out_test t (loc, dump);
+
+  /* Locate the first insn in the dump.  */
+  rtx_insn *insn = get_insns ();
+
+  /* Write out asm for the insn.  */
+  int seen;
+  final_scan_insn (insn, stderr, 0, 0, &seen);
+
+  /* Verify that the expected asm was written out.  */
+  char *written_asm = t.get_output (loc);
+  ASSERT_STREQ_AT (loc, expected_asm, written_asm);
+  free (written_asm);
+}
+
+/* Parse DUMP via an asm_out_test and run final_scan_insn on the first
+   insn seen in the dump.  Verify that the generated asm equals
+   EXPECTED_ASM.  */
+
+#define TEST_ASM_FOR_INSN(DUMP, EXPECTED_ASM)			    \
+  SELFTEST_BEGIN_STMT							\
+    test_asm_for_insn (SELFTEST_LOCATION, (DUMP), (EXPECTED_ASM));	\
+  SELFTEST_END_STMT
+
+/* Various tests of how a particular instruction is recognized and
+   written out.  These all assume x86_64 and perhaps make additional
+   tuning assumptions.  */
+
+static void
+test_x86_64_leal ()
+{
+  TEST_ASM_FOR_INSN (
+    ("(insn:TI 31 0 0 2 (set (reg:SI 0 ax [93])\n"
+     "        (plus:SI (reg/v:SI 1 dx [orig:90 k ] [90])\n"
+     "            (const_int 4 [0x4]))) ../../src/gcc/testsuite/rtl.dg/test.c:4 -1\n"
+     "     (nil))\n"),
+    "\tleal\t4(%rdx), %eax\n");
+}
+
+static void
+test_x86_64_negl ()
+{
+  TEST_ASM_FOR_INSN (
+    ("(insn 27 0 0 2 (parallel [\n"
+     "            (set (reg:SI 1 dx [92])\n"
+     "                (neg:SI (reg/v:SI 1 dx [orig:90 k ] [90])))\n"
+     "            (clobber (reg:CC 17 flags))\n"
+     "        ]) ../../src/gcc/testsuite/rtl.dg/test.c:4 -1\n"
+     "     (expr_list:REG_UNUSED (reg:CC 17 flags)\n"
+     "        (nil)))\n"),
+    "\tnegl\t%edx\n");
+}
+
+static void
+test_x86_64_cmpl ()
+{
+  TEST_ASM_FOR_INSN (
+    ("(insn 28 0 0 2 (set (reg:CCGC 17 flags)\n"
+     "        (compare:CCGC (reg/v:SI 5 di [orig:88 i ] [88])\n"
+     "            (reg/v:SI 4 si [orig:89 j ] [89]))) ../../src/gcc/testsuite/rtl.dg/test.c:4 -1\n"
+     "     (expr_list:REG_DEAD (reg/v:SI 5 di [orig:88 i ] [88])\n"
+     "        (expr_list:REG_DEAD (reg/v:SI 4 si [orig:89 j ] [89])\n"
+     "            (nil))))\n"),
+    "\tcmpl\t%esi, %edi\n");
+}
+
+static void
+test_x86_64_cmovge ()
+{
+  TEST_ASM_FOR_INSN (
+    ("(insn:TI 29 0 0 2 (set (reg:SI 0 ax [orig:87 <retval> ] [87])\n"
+     "        (if_then_else:SI (ge (reg:CCGC 17 flags)\n"
+     "                (const_int 0 [0]))\n"
+     "            (reg:SI 1 dx [92])\n"
+     "            (reg:SI 0 ax [93]))) ../../src/gcc/testsuite/rtl.dg/test.c:4 -1\n"
+     "     (expr_list:REG_DEAD (reg:CCGC 17 flags)\n"
+     "        (expr_list:REG_DEAD (reg:SI 1 dx [92])\n"
+     "            (nil))))\n"),
+    "\tcmovge\t%edx, %eax\n");
+}
+
+static void
+test_x86_64_ret ()
+{
+  TEST_ASM_FOR_INSN (
+    ("(jump_insn:TI 34 0 0 2 (simple_return) ../../src/gcc/testsuite/rtl.dg/test.c:7 -1\n"
+     "     (nil)\n"
+     " -> simple_return)\n"),
+    "\tret\n");
+}
+
+/* Run all of the selftests within this file.  */
+
+void
+final_c_tests ()
+{
+  /* Only run these tests for i386.  */
+#ifndef I386_OPTS_H
+  return;
+#endif
+
+  test_jump_insn ();
+  test_empty_function ();
+  test_x86_64_leal ();
+  test_x86_64_negl ();
+  test_x86_64_cmpl ();
+  test_x86_64_cmovge ();
+  test_x86_64_ret ();
+}
+
+} // namespace selftest
+#endif /* CHECKING_P */
diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
index 296fe00..015572c 100644
--- a/gcc/selftest-run-tests.c
+++ b/gcc/selftest-run-tests.c
@@ -76,6 +76,7 @@  selftest::run_tests ()
   spellcheck_c_tests ();
   spellcheck_tree_c_tests ();
   tree_cfg_c_tests ();
+  final_c_tests ();
 
   /* This one relies on most of the above.  */
   function_tests_c_tests ();
diff --git a/gcc/selftest.h b/gcc/selftest.h
index 8b94c3a..6ad6c88 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -198,6 +198,7 @@  extern void edit_context_c_tests ();
 extern void et_forest_c_tests ();
 extern void fold_const_c_tests ();
 extern void fibonacci_heap_c_tests ();
+extern void final_c_tests ();
 extern void function_tests_c_tests ();
 extern void gimple_c_tests ();
 extern void ggc_tests_c_tests ();