On 11/11/2016 10:15 PM, David Malcolm wrote:
#include "gt-aarch64.h"
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 6c608e0..0dda786 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
I think we should separate out the target specific tests so as to give
port maintainers a chance to comment on them separately.
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 50cd388..179a91f 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -1371,6 +1371,19 @@ maybe_set_first_label_num (rtx_code_label *x)
if (CODE_LABEL_NUMBER (x) < first_label_num)
first_label_num = CODE_LABEL_NUMBER (x);
}
+
+/* For use by the RTL function loader, when mingling with normal
+ functions.
Not sure what this means.
if (str == 0)
- fputs (" \"\"", m_outfile);
+ fputs (" (nil)", m_outfile);
else
fprintf (m_outfile, " (\"%s\")", str);
m_sawclose = 1;
What does this affect?
/* Global singleton; constrast with md_reader_ptr above. */
diff --git a/gcc/read-rtl-function.c b/gcc/read-rtl-function.c
new file mode 100644
index 0000000..ff6c808
--- /dev/null
+++ b/gcc/read-rtl-function.c
@@ -0,0 +1,2124 @@
+/* read-rtl-function.c - Reader for RTL function dumps
+ Copyright (C) 2016 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+#include <mpfr.h>
Please double-check all these includes whether they are necessary.
+
+/* Fix up a NOTE_INSN_BASIC_BLOCK based on an integer block ID. */
+
+void
+fixup_note_insn_basic_block::apply (function_reader */*reader*/) const
Lose the /*reader*/, probably.
+
+/* Implementation of rtx_reader::handle_unknown_directive.
+
+ Require a top-level "function" elements, as emitted by
+ print_rtx_function, and parse it. */
"element"?
+void
+function_reader::create_function ()
+{
+ /* Currently we assume cfgrtl mode, rather than cfglayout mode. */
+ if (0)
+ cfg_layout_rtl_register_cfg_hooks ();
+ else
+ rtl_register_cfg_hooks ();
Do we expect to change this? I'd just get rid of the if (0), at least
for now.
+ /* cgraph_node::add_new_function does additional processing
+ based on symtab->state. We need to avoid it attempting to gimplify
+ things. Temporarily putting it in the PARSING state appears to
+ achieve this. */
+ enum symtab_state old_state = symtab->state;
+ symtab->state = PARSING;
+ cgraph_node::add_new_function (fndecl, true /*lowered*/);
+ /* Reset the state. */
+ symtab->state = old_state;
+ }
Does it do anything beside call finalize_function in that state? If
that's all you need, just call it directly.
+
+ /* Parse DECL_RTL. */
+ {
+ require_char_ws ('(');
+ require_word_ws ("DECL_RTL");
+ DECL_WRTL_CHECK (t_param)->decl_with_rtl.rtl = parse_rtx ();
+ require_char_ws (')');
+ }
Spurious { } blocks.
+ if (0)
+ fprintf (stderr, "parse_edge: %i flags 0x%x \n",
+ other_bb_idx, flags);
Remove this.
+ /* For now, only process the (edge-from) to this BB, and (edge-to)
+ that go to the exit block; we don't yet verify that the edge-from
+ and edge-to directives are consistent. */
That's probably worth a FIXME.
+ if (rtx_code_label *label = dyn_cast <rtx_code_label *> (insn))
+ maybe_set_max_label_num (label);
I keep forgetting why dyn_cast instead of as_a?
+ case 'e':
+ {
+ if (idx == 7 && CALL_P (return_rtx))
+ {
+ m_in_call_function_usage = true;
+ return rtx_reader::read_rtx_operand (return_rtx, idx);
+ m_in_call_function_usage = false;
+ }
+ else
+ return rtx_reader::read_rtx_operand (return_rtx, idx);
+ }
+ break;
Unnecessary { } blocks in several places.
+
+ case 'w':
+ {
+ if (!is_compact ())
+ {
+ /* Strip away the redundant hex dump of the value. */
+ require_char_ws ('[');
+ read_name (&name);
+ require_char_ws (']');
+ }
+ }
+ break;
Here too.
+
+/* Special-cased handling of codes 'i' and 'n' for reading function
+ dumps. */
+
+void
+function_reader::read_rtx_operand_i_or_n (rtx return_rtx, int idx,
+ char format_char)
Document arguments (everywhere). I think return_rtx (throughout these
functions) is a poor name that can cause confusion because it seems to
imply a (return).
+
+ /* Possibly wrote:
+ print_node_brief (outfile, "", SYMBOL_REF_DECL (in_rtx),
+ dump_flags); */
???
+ /* Skip the content for now. */
Does this relate to the above? Please clarify the comments.
+ case CODE_LABEL:
+ {
+ /* Assume that LABEL_NUSES was not dumped. */
+ /* TODO: parse LABEL_KIND. */
Unnecessary { }.
+ if (0 == strcmp (desc, "<retval>"))
+ {
+ return DECL_RESULT (fndecl);
+ }
+
+ /* Search within function parms. */
+ for (tree arg = DECL_ARGUMENTS (fndecl); arg; arg = TREE_CHAIN (arg))
+ {
+ if (strcmp (desc, IDENTIFIER_POINTER (DECL_NAME (arg))) == 0)
+ return arg;
+ }
No { } around single statements, both cases.
+ /* Not found? Create it.
+ This allows mimicing of real data but avoids having to specify
"mimicking".
+rtx
+function_reader::consolidate_singletons (rtx x)
+{
+ if (!x)
+ return x;
+
+ switch (GET_CODE (x))
Formatting.
+ {
+ /* FIXME: do we need to check for VOIDmode for these? */
Don't see why we would.
+ case CONST_INT:
+ return gen_rtx_CONST_INT (GET_MODE (x), INTVAL (x));
Ugh, really? Can't this be done earlier?
+
+ add_fixup_source_location (loc, insn, operand_idx,
+ filename, atoi(name.string));
Space before open paren.
+ /* Verify lookup of hard registers. */
+#ifdef GCC_AARCH64_H
+ ASSERT_EQ (0, lookup_reg_by_dump_name ("x0"));
+ ASSERT_EQ (1, lookup_reg_by_dump_name ("x1"));
+#endif
+#ifdef I386_OPTS_H
+ ASSERT_EQ (0, lookup_reg_by_dump_name ("ax"));
+ ASSERT_EQ (1, lookup_reg_by_dump_name ("dx"));
+#endif
Same as before, please don't do this here.
+/* Verify that the src and dest indices and flags of edge E are as
+ expected, using LOC as the effective location when reporting
+ failures. */
Again, please document all args (everywhere).
+
+static void
+test_loading_dump_fragment_1 ()
+{
+ // TODO: filter on target?
+ rtl_dump_test t (SELFTEST_LOCATION, locate_file ("asr_div1.rtl"));
This is in a generic file, isn't it? Yeah, I don't think we want to load
any RTL from here. Split out such selftests into a separate patch so we
can figure out what to do with them.
+
+ rtx_insn *jump_insn = get_insn_by_uid (1);
+ ASSERT_EQ (JUMP_INSN, GET_CODE (jump_insn));
+ ASSERT_EQ (ret_rtx, JUMP_LABEL (jump_insn));
+ // FIXME: ^^^ use ASSERT_RTX_PTR_EQ here ^^^
Why is this a fixme and not just done that way (several instances)?
+
+/* An optional policy class for class function_reader. */
+
+struct function_reader_policy
+{
+ function_reader_policy ()
+ {
+ }
+};
+
+extern bool read_rtl_function_body (int argc, const char **argv,
+ bool (*parse_opt) (const char *),
+ function_reader_policy *policy);
The policy seems completely unused, please remove. Also, can't this
single declaration go into an existing header?
+ /* Handle insn codes in compact dumps. In such dumps, the code of insns
+ is prefixed with "c", giving "cinsn", "cnote" etc, and CODE_LABEL is
+ special-cased as "clabel". */
+ if (name[0] == 'c')
+ {
+ for (i = 0; i < NUM_RTX_CODE; i++)
+ if (strcmp (GET_RTX_NAME (i), name + 1) == 0)
+ return i;
I'd rather check specifically for the ones we expect, to avoid surprises.
+#ifdef GENERATOR_FILE
There's a lot of this. Is there a clean split where stuff could be moved
into a separate file instead?
@@ -1103,13 +1233,39 @@ rtx_reader::read_rtx_code (const char *code_name)
rtx value; /* Value of this node. */
};
+ one_time_initialization ();
Isn't there a better place to call this?
+
+ /* Use the format_ptr to parse the various operands of this rtx.
+ read_rtx_operand is a vfunc, allowing the parser to vary between
+ parsing .md files and parsing .rtl dumps; the function_reader subclass
+ overrides the defaults when loading RTL dumps, to handle the
+ various extra attributes emitted by print_rtx. */
Not sure that documentation is really necessary at this point. If
someone is looking for the definition of read_rtx_operand they'll figure
out it's a virtual function anyway.
for (int idx = 0; format_ptr[idx] != 0; idx++)
- read_rtx_operand (return_rtx, idx);
+ return_rtx = read_rtx_operand (return_rtx, idx);
+
+ /* Call a vfunc to handle the various additional information that
+ print-rtl.c can write after the regular fields; does nothing when
+ parsing .md files. */
+ handle_any_trailing_information (return_rtx);
Same here really.
+
+ /* "stringbuf" was allocated within string_obstack and thus has
+ the its lifetime restricted to that of the rtx_reader. This is
+ OK for the generator programs, but for non-generator programs,
+ XSTR and XTMPL fields are meant to be allocated in the GC-managed
+ heap. Hence we need to allocate a copy in the GC-managed heap
+ for the non-generator case. */
+ const char *string_ptr = stringbuf;
+#ifndef GENERATOR_FILE
+ string_ptr = ggc_strdup (stringbuf);
+#endif /* #ifndef GENERATOR_FILE */
Encapsulate in a virtual finalize_string perhaps?
Bernd
