Thank you for the review.
Yes, this increases the binary size.
I have implemented this in the third version of this patch series, is that
what you had in mind?
Originally, this change increased the sizes of binaries 8-12 kB, but after
updating the master branch, this change would actually decrease the
executable size. I described in more detail in the new cover letter.
Andrzej
On Mon, Jul 31, 2023, 15:49 Richard Biener <richard.guent...@gmail.com>
wrote:
> On Mon, Jul 31, 2023 at 1:07 PM Andrzej Turko via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Currently fprintf calls logging to a dump file take line numbers
> > in the match.pd file directly as arguments.
> > When match.pd is edited, referenced code changes line numbers,
> > which causes changes to many fprintf calls and, thus, to many
> > (usually all) .cc files generated by genmatch. This forces make
> > to (unnecessarily) rebuild many .o files.
> >
> > With this change those logging fprintf calls reference an array
> > of line numbers, which is defined in one of the produced files.
> > Thanks to this, when match.pd changes, it is enough to rebuild
> > that single file and, of course, those actually affected by the
> > change.
> >
> > Signed-off-by: Andrzej Turko <andrzej.tu...@gmail.com>
>
> How does this affect the size of the executable? We are replacing
> pushing a small immediate to the stack with an indexed load plus push.
>
> Maybe further indirecting the whole dumping, passing an index of the
> match and __FILE__/__LINE__ would help here, so instead of
>
> if (UNLIKELY (debug_dump)) fprintf
> (dump_file, "Matching expression %s:%d, %s:%d\n", "match.pd", 2522,
> __FILE__, __LINE__);
>
> we emit sth like
>
> if (UNLIKELY (debug_dump)) dump_match (2522,
> __FILE__, __LINE__);
>
> with 2522 replaced by the ID? That would also get rid of the inline
> varargs invocation which might help code size as well (on some targets).
>
> Richard.
>
> > gcc/ChangeLog:
> >
> > * genmatch.cc: Keep line numbers from match.pd in an array.
> >
> > Signed-off-by: Andrzej Turko <andrzej.tu...@gmail.com>
> > ---
> > gcc/genmatch.cc | 73 +++++++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 65 insertions(+), 8 deletions(-)
> >
> > diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc
> > index 1deca505603..0a480a140c9 100644
> > --- a/gcc/genmatch.cc
> > +++ b/gcc/genmatch.cc
> > @@ -217,9 +217,48 @@ fp_decl_done (FILE *f, const char *trailer)
> > fprintf (header_file, "%s;", trailer);
> > }
> >
> > +/* Line numbers for use by indirect line directives. */
> > +static vec<int> dbg_line_numbers;
> > +
> > +static void
> > +write_header_declarations (bool gimple, FILE *f)
> > +{
> > + if (gimple)
> > + fprintf (f, "\nextern int __gimple_dbg_line_numbers[];\n");
> > + else
> > + fprintf (f, "\nextern int __generic_dbg_line_numbers[];\n");
> > +}
> > +
> > +static void
> > +define_dbg_line_numbers (bool gimple, FILE *f)
> > +{
> > + if (gimple)
> > + fprintf (f, "\nint __gimple_dbg_line_numbers[%d] = {",
> > + dbg_line_numbers.length ());
> > + else
> > + fprintf (f, "\nint __generic_dbg_line_numbers[%d] = {",
> > + dbg_line_numbers.length ());
> > +
> > + if (dbg_line_numbers.is_empty ())
> > + {
> > + fprintf (f, "};\n\n");
> > + return;
> > + }
> > +
> > + for (int i = 0; i < (int)dbg_line_numbers.length () - 1; i++)
> > + {
> > + if (i % 20 == 0)
> > + fprintf (f, "\n\t");
> > +
> > + fprintf (f, "%d, ", dbg_line_numbers[i]);
> > + }
> > + fprintf (f, "%d\n};\n\n", dbg_line_numbers.last ());
> > +}
> > +
> > static void
> > output_line_directive (FILE *f, location_t location,
> > - bool dumpfile = false, bool fnargs = false)
> > + bool dumpfile = false, bool fnargs = false,
> > + bool indirect_line_numbers = false, bool gimple =
> false)
> > {
> > const line_map_ordinary *map;
> > linemap_resolve_location (line_table, location,
> LRK_SPELLING_LOCATION, &map);
> > @@ -239,7 +278,20 @@ output_line_directive (FILE *f, location_t location,
> > ++file;
> >
> > if (fnargs)
> > - fprintf (f, "\"%s\", %d", file, loc.line);
> > + {
> > + if (indirect_line_numbers)
> > + {
> > + if (gimple)
> > + fprintf (f, "\"%s\", __gimple_dbg_line_numbers[%d]",
> > + file, dbg_line_numbers.length ());
> > + else
> > + fprintf (f, "\"%s\", __generic_dbg_line_numbers[%d]",
> > + file, dbg_line_numbers.length ());
> > + dbg_line_numbers.safe_push (loc.line);
> > + }
> > + else
> > + fprintf (f, "\"%s\", %d", file, loc.line);
> > + }
> > else
> > fprintf (f, "%s:%d", file, loc.line);
> > }
> > @@ -3378,7 +3430,8 @@ dt_operand::gen (FILE *f, int indent, bool gimple,
> int depth)
> > /* Emit a fprintf to the debug file to the file F, with the INDENT from
> > either the RESULT location or the S's match location if RESULT is
> null. */
> > static void
> > -emit_debug_printf (FILE *f, int indent, class simplify *s, operand
> *result)
> > +emit_debug_printf (FILE *f, int indent, class simplify *s, operand
> *result,
> > + bool gimple)
> > {
> > fprintf_indent (f, indent, "if (UNLIKELY (debug_dump)) "
> > "fprintf (dump_file, \"%s ",
> > @@ -3387,7 +3440,7 @@ emit_debug_printf (FILE *f, int indent, class
> simplify *s, operand *result)
> > fprintf (f, "%%s:%%d, %%s:%%d\\n\", ");
> > output_line_directive (f,
> > result ? result->location : s->match->location,
> true,
> > - true);
> > + true, true, gimple);
> > fprintf (f, ", __FILE__, __LINE__);\n");
> > }
> >
> > @@ -3524,7 +3577,7 @@ dt_simplify::gen_1 (FILE *f, int indent, bool
> gimple, operand *result)
> > if (!result)
> > {
> > /* If there is no result then this is a predicate
> implementation. */
> > - emit_debug_printf (f, indent, s, result);
> > + emit_debug_printf (f, indent, s, result, gimple);
> > fprintf_indent (f, indent, "return true;\n");
> > }
> > else if (gimple)
> > @@ -3615,7 +3668,7 @@ dt_simplify::gen_1 (FILE *f, int indent, bool
> gimple, operand *result)
> > }
> > else
> > gcc_unreachable ();
> > - emit_debug_printf (f, indent, s, result);
> > + emit_debug_printf (f, indent, s, result, gimple);
> > fprintf_indent (f, indent, "return true;\n");
> > }
> > else /* GENERIC */
> > @@ -3670,7 +3723,7 @@ dt_simplify::gen_1 (FILE *f, int indent, bool
> gimple, operand *result)
> > }
> > if (is_predicate)
> > {
> > - emit_debug_printf (f, indent, s, result);
> > + emit_debug_printf (f, indent, s, result, gimple);
> > fprintf_indent (f, indent, "return true;\n");
> > }
> > else
> > @@ -3738,7 +3791,7 @@ dt_simplify::gen_1 (FILE *f, int indent, bool
> gimple, operand *result)
> > i);
> > }
> > }
> > - emit_debug_printf (f, indent, s, result);
> > + emit_debug_printf (f, indent, s, result, gimple);
> > fprintf_indent (f, indent, "return _r;\n");
> > }
> > }
> > @@ -5447,6 +5500,7 @@ main (int argc, char **argv)
> > parts.quick_push (stdout);
> > write_header (stdout, s_include_file);
> > write_header_includes (gimple, stdout);
> > + write_header_declarations (gimple, stdout);
> > }
> > else
> > {
> > @@ -5460,6 +5514,7 @@ main (int argc, char **argv)
> > fprintf (header_file, "#ifndef GCC_GIMPLE_MATCH_AUTO_H\n"
> > "#define GCC_GIMPLE_MATCH_AUTO_H\n");
> > write_header_includes (gimple, header_file);
> > + write_header_declarations (gimple, header_file);
> > }
> >
> > /* Go over all predicates defined with patterns and perform
> > @@ -5502,6 +5557,8 @@ main (int argc, char **argv)
> >
> > dt.gen (parts, gimple);
> >
> > + define_dbg_line_numbers (gimple, choose_output (parts));
> > +
> > for (FILE *f : parts)
> > {
> > fprintf (f, "#pragma GCC diagnostic pop\n");
> > --
> > 2.34.1
> >
>
