| Message ID | ZD5u0Z9FB39C6nmv@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On Tue, Apr 18, 2023 at 12:22 PM Tamar Christina via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Hi All, > > This is a small improvement in QoL codegen for match.pd to save time not > re-evaluating the condition for printing debug information in every function. > > There is a small but consistent runtime and compile time win here. The runtime > win comes from not having to do the condition over again, and on Arm plaforms > we now use the new test-and-branch support for booleans to only have a single > instruction here. > > Compile time win is gotten from not having to do all the string parsing for the > printf and having less string interning to do. > > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > > Ok for master? Ugh, I don't like the new global very much. Can't we compute it in the toplevel entry and pass it down as parameter? Like passing down the actual dump FILE *? The file output in output_line_directive was because we originally had match.pd #includeing multiple match-*.pd files, we'd want to keep that supported I think. But since the line directives are commented and there's the same info available below, like /* #line 798 "/home/rguenther/src/gcc-13-branch/gcc/match.pd" */ tree captures[2] ATTRIBUTE_UNUSED = { _p0, _p1 }; if (UNLIKELY (dump_file && (dump_flags & TDF_FOLDING))) fprintf (dump_file, "Matching expression %s:%d, %s:%d\n", "match.pd", 798, __FILE__, __LINE__); there's probably no point in emitting them anymore (originally I emitted them non-commented but that didn't improve debugging much). We might want to emit more "proper" line directives for the natively copied parts of match.pd when code-generating c_expr parts, but that would be something separate. Can you split the patch into two things? A patch removing output of the commented line directives at the call sites is OK. Richard. > Thanks, > Tamar > > gcc/ChangeLog: > > PR bootstrap/84402 > * dumpfile.h (dump_folding_p): New. > * dumpfile.cc (set_dump_file): Use it. > * generic-match-head.cc (dump_debug): New. > * gimple-match-head.cc (dump_debug): New. > * genmatch.cc (output_line_directive): Support outputting only line > because file is implied. > (dt_simplify::gen_1): Call debug_dump instead of printf. > > --- inline copy of patch -- > diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h > index 7d5eca899dcc98676a9ce7a7efff8e439854ff89..e7b595ddecdcca9983d9584b8b2417ae1941c7d4 100644 > --- a/gcc/dumpfile.h > +++ b/gcc/dumpfile.h > @@ -522,6 +522,7 @@ parse_dump_option (const char *, const char **); > extern FILE *dump_file; > extern dump_flags_t dump_flags; > extern const char *dump_file_name; > +extern bool dump_folding_p; > > extern bool dumps_are_enabled; > > diff --git a/gcc/dumpfile.cc b/gcc/dumpfile.cc > index 51f68c8c6b40051ba3125c84298ee44ca52f5d17..f805aa73f3aa244d847149eec26505181ce4efe8 100644 > --- a/gcc/dumpfile.cc > +++ b/gcc/dumpfile.cc > @@ -63,6 +63,7 @@ FILE *dump_file = NULL; > const char *dump_file_name; > dump_flags_t dump_flags; > bool dumps_are_enabled = false; > +bool dump_folding_p = false; > > > /* Set global "dump_file" to NEW_DUMP_FILE, refreshing the "dumps_are_enabled" > @@ -73,6 +74,7 @@ set_dump_file (FILE *new_dump_file) > { > dumpfile_ensure_any_optinfo_are_flushed (); > dump_file = new_dump_file; > + dump_folding_p = dump_file && (dump_flags & TDF_FOLDING); > dump_context::get ().refresh_dumps_are_enabled (); > } > > diff --git a/gcc/generic-match-head.cc b/gcc/generic-match-head.cc > index f011204c5be450663231bdece0596317b37f9f9b..16b8f9f3b61d3d5651a5a41a8c0552f50b55cc7c 100644 > --- a/gcc/generic-match-head.cc > +++ b/gcc/generic-match-head.cc > @@ -102,3 +102,17 @@ optimize_successive_divisions_p (tree, tree) > { > return false; > } > + > +/* Helper method for debug printing to reducing string parsing overhead. Keep > + in sync with version in gimple-match-head.cc. */ > + > +static > +void dump_debug (bool simplify, int loc, const char *file, int lineno) > +{ > + if (simplify) > + fprintf (dump_file, "Applying pattern %s:%d, %s:%d\n", "match.pd", loc, > + file, lineno); > + else > + fprintf (dump_file, "Matching expression %s:%d, %s:%d\n", "match.pd", loc, > + file, lineno); > +} > \ No newline at end of file > diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc > index 638606b2502f640e59527fc5a0b23fa3bedd0cee..bd7c6ff4a3fb89d456b02242707fd823b737f20d 100644 > --- a/gcc/genmatch.cc > +++ b/gcc/genmatch.cc > @@ -185,7 +185,8 @@ fprintf_indent (FILE *f, unsigned int indent, const char *format, ...) > > static void > output_line_directive (FILE *f, location_t location, > - bool dumpfile = false, bool fnargs = false) > + bool dumpfile = false, bool fnargs = false, > + bool loc_only = false) > { > const line_map_ordinary *map; > linemap_resolve_location (line_table, location, LRK_SPELLING_LOCATION, &map); > @@ -204,7 +205,9 @@ output_line_directive (FILE *f, location_t location, > else > ++file; > > - if (fnargs) > + if (loc_only) > + fprintf (f, "%d", loc.line); > + else if (fnargs) > fprintf (f, "\"%s\", %d", file, loc.line); > else > fprintf (f, "%s:%d", file, loc.line); > @@ -3431,14 +3434,11 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result) > needs_label = true; > } > > - fprintf_indent (f, indent, "if (UNLIKELY (dump_file && (dump_flags & TDF_FOLDING))) " > - "fprintf (dump_file, \"%s ", > - s->kind == simplify::SIMPLIFY > - ? "Applying pattern" : "Matching expression"); > - fprintf (f, "%%s:%%d, %%s:%%d\\n\", "); > + fprintf_indent (f, indent, "if (UNLIKELY (dump_folding_p)) " > + "dump_debug (%s, ", s->kind == simplify::SIMPLIFY ? "true" : "false"); > output_line_directive (f, > result ? result->location : s->match->location, true, > - true); > + true, true); > fprintf (f, ", __FILE__, __LINE__);\n"); > > fprintf_indent (f, indent, "{\n"); > diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc > index ec603f9d043c3924ea442bb49b5300a3573503cf..ae0c5c8a74fd9f1acdb616014941b11961e96c04 100644 > --- a/gcc/gimple-match-head.cc > +++ b/gcc/gimple-match-head.cc > @@ -1412,3 +1412,17 @@ get_conditional_internal_fn (code_helper code, tree type) > auto cfn = combined_fn (code); > return get_conditional_internal_fn (associated_internal_fn (cfn, type)); > } > + > +/* Helper method for debug printing to reducing string parsing overhead. Keep > + in sync with version in generic-match-head.cc. */ > + > +static > +void dump_debug (bool simplify, int loc, const char *file, int lineno) > +{ > + if (simplify) > + fprintf (dump_file, "Applying pattern %s:%d, %s:%d\n", "match.pd", loc, > + file, lineno); > + else > + fprintf (dump_file, "Matching expression %s:%d, %s:%d\n", "match.pd", loc, > + file, lineno); > +} > \ No newline at end of file > > > > > --
> -----Original Message----- > From: Richard Biener <richard.guenther@gmail.com> > Sent: Tuesday, April 18, 2023 11:48 AM > To: Tamar Christina <Tamar.Christina@arm.com> > Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; rguenther@suse.de; > jlaw@ventanamicro.com > Subject: Re: [PATCH 2/3]middle-end match.pd: simplify debug dump checks > > On Tue, Apr 18, 2023 at 12:22 PM Tamar Christina via Gcc-patches <gcc- > patches@gcc.gnu.org> wrote: > > > > Hi All, > > > > This is a small improvement in QoL codegen for match.pd to save time > > not re-evaluating the condition for printing debug information in every > function. > > > > There is a small but consistent runtime and compile time win here. > > The runtime win comes from not having to do the condition over again, > > and on Arm plaforms we now use the new test-and-branch support for > > booleans to only have a single instruction here. > > > > Compile time win is gotten from not having to do all the string > > parsing for the printf and having less string interning to do. > > > > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > > > > Ok for master? > > Ugh, I don't like the new global very much. Can't we compute it in the toplevel > entry and pass it down as parameter? Like passing down the actual dump FILE > *? Yeah that would work too, will do. > > The file output in output_line_directive was because we originally had > match.pd #includeing multiple match-*.pd files, we'd want to keep that > supported I think. But since the line directives are commented and there's the > same info available below, like > > /* #line 798 "/home/rguenther/src/gcc-13-branch/gcc/match.pd" */ > tree captures[2] ATTRIBUTE_UNUSED = { _p0, _p1 }; > if (UNLIKELY (dump_file && (dump_flags & > TDF_FOLDING))) fprintf (dump_file, "Matching expression %s:%d, %s:%d\n", > "match.pd", 798, __FILE__, __LINE__); > > there's probably no point in emitting them anymore (originally I emitted them > non-commented but that didn't improve debugging much). We might want > to emit more "proper" line directives for the natively copied parts of match.pd > when code-generating c_expr parts, but that would be something separate. > > Can you split the patch into two things? A patch removing output of the > commented line directives at the call sites is OK. Sure, I'll hold up respinning waiting on the 3rd patch review since this one will change that one as well, so easier to handle all comments at once. Thanks for the review, Tamar > > Richard. > > > Thanks, > > Tamar > > > > gcc/ChangeLog: > > > > PR bootstrap/84402 > > * dumpfile.h (dump_folding_p): New. > > * dumpfile.cc (set_dump_file): Use it. > > * generic-match-head.cc (dump_debug): New. > > * gimple-match-head.cc (dump_debug): New. > > * genmatch.cc (output_line_directive): Support outputting only line > > because file is implied. > > (dt_simplify::gen_1): Call debug_dump instead of printf. > > > > --- inline copy of patch -- > > diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h index > > > 7d5eca899dcc98676a9ce7a7efff8e439854ff89..e7b595ddecdcca9983d958 > 4b8b24 > > 17ae1941c7d4 100644 > > --- a/gcc/dumpfile.h > > +++ b/gcc/dumpfile.h > > @@ -522,6 +522,7 @@ parse_dump_option (const char *, const char **); > > extern FILE *dump_file; extern dump_flags_t dump_flags; extern const > > char *dump_file_name; > > +extern bool dump_folding_p; > > > > extern bool dumps_are_enabled; > > > > diff --git a/gcc/dumpfile.cc b/gcc/dumpfile.cc index > > > 51f68c8c6b40051ba3125c84298ee44ca52f5d17..f805aa73f3aa244d84714 > 9eec265 > > 05181ce4efe8 100644 > > --- a/gcc/dumpfile.cc > > +++ b/gcc/dumpfile.cc > > @@ -63,6 +63,7 @@ FILE *dump_file = NULL; const char *dump_file_name; > > dump_flags_t dump_flags; bool dumps_are_enabled = false; > > +bool dump_folding_p = false; > > > > > > /* Set global "dump_file" to NEW_DUMP_FILE, refreshing the > "dumps_are_enabled" > > @@ -73,6 +74,7 @@ set_dump_file (FILE *new_dump_file) { > > dumpfile_ensure_any_optinfo_are_flushed (); > > dump_file = new_dump_file; > > + dump_folding_p = dump_file && (dump_flags & TDF_FOLDING); > > dump_context::get ().refresh_dumps_are_enabled (); } > > > > diff --git a/gcc/generic-match-head.cc b/gcc/generic-match-head.cc > > index > > > f011204c5be450663231bdece0596317b37f9f9b..16b8f9f3b61d3d5651a5 > a41a8c05 > > 52f50b55cc7c 100644 > > --- a/gcc/generic-match-head.cc > > +++ b/gcc/generic-match-head.cc > > @@ -102,3 +102,17 @@ optimize_successive_divisions_p (tree, tree) { > > return false; > > } > > + > > +/* Helper method for debug printing to reducing string parsing overhead. > Keep > > + in sync with version in gimple-match-head.cc. */ > > + > > +static > > +void dump_debug (bool simplify, int loc, const char *file, int > > +lineno) { > > + if (simplify) > > + fprintf (dump_file, "Applying pattern %s:%d, %s:%d\n", "match.pd", loc, > > + file, lineno); > > + else > > + fprintf (dump_file, "Matching expression %s:%d, %s:%d\n", "match.pd", > loc, > > + file, lineno); > > +} > > \ No newline at end of file > > diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc index > > > 638606b2502f640e59527fc5a0b23fa3bedd0cee..bd7c6ff4a3fb89d456b022 > 42707f > > d823b737f20d 100644 > > --- a/gcc/genmatch.cc > > +++ b/gcc/genmatch.cc > > @@ -185,7 +185,8 @@ fprintf_indent (FILE *f, unsigned int indent, > > const char *format, ...) > > > > static void > > output_line_directive (FILE *f, location_t location, > > - bool dumpfile = false, bool fnargs = false) > > + bool dumpfile = false, bool fnargs = false, > > + bool loc_only = false) > > { > > const line_map_ordinary *map; > > linemap_resolve_location (line_table, location, > > LRK_SPELLING_LOCATION, &map); @@ -204,7 +205,9 @@ > output_line_directive (FILE *f, location_t location, > > else > > ++file; > > > > - if (fnargs) > > + if (loc_only) > > + fprintf (f, "%d", loc.line); > > + else if (fnargs) > > fprintf (f, "\"%s\", %d", file, loc.line); > > else > > fprintf (f, "%s:%d", file, loc.line); @@ -3431,14 +3434,11 @@ > > dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result) > > needs_label = true; > > } > > > > - fprintf_indent (f, indent, "if (UNLIKELY (dump_file && (dump_flags & > TDF_FOLDING))) " > > - "fprintf (dump_file, \"%s ", > > - s->kind == simplify::SIMPLIFY > > - ? "Applying pattern" : "Matching expression"); > > - fprintf (f, "%%s:%%d, %%s:%%d\\n\", "); > > + fprintf_indent (f, indent, "if (UNLIKELY (dump_folding_p)) " > > + "dump_debug (%s, ", s->kind == simplify::SIMPLIFY ? "true" : > > + "false"); > > output_line_directive (f, > > result ? result->location : s->match->location, true, > > - true); > > + true, true); > > fprintf (f, ", __FILE__, __LINE__);\n"); > > > > fprintf_indent (f, indent, "{\n"); > > diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc index > > > ec603f9d043c3924ea442bb49b5300a3573503cf..ae0c5c8a74fd9f1acdb616 > 014941 > > b11961e96c04 100644 > > --- a/gcc/gimple-match-head.cc > > +++ b/gcc/gimple-match-head.cc > > @@ -1412,3 +1412,17 @@ get_conditional_internal_fn (code_helper code, > tree type) > > auto cfn = combined_fn (code); > > return get_conditional_internal_fn (associated_internal_fn (cfn, > > type)); } > > + > > +/* Helper method for debug printing to reducing string parsing overhead. > Keep > > + in sync with version in generic-match-head.cc. */ > > + > > +static > > +void dump_debug (bool simplify, int loc, const char *file, int > > +lineno) { > > + if (simplify) > > + fprintf (dump_file, "Applying pattern %s:%d, %s:%d\n", "match.pd", loc, > > + file, lineno); > > + else > > + fprintf (dump_file, "Matching expression %s:%d, %s:%d\n", "match.pd", > loc, > > + file, lineno); > > +} > > \ No newline at end of file > > > > > > > > > > --
> -----Original Message----- > From: Richard Biener <richard.guenther@gmail.com> > Sent: Tuesday, April 18, 2023 11:48 AM > To: Tamar Christina <Tamar.Christina@arm.com> > Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; rguenther@suse.de; > jlaw@ventanamicro.com > Subject: Re: [PATCH 2/3]middle-end match.pd: simplify debug dump checks > > On Tue, Apr 18, 2023 at 12:22 PM Tamar Christina via Gcc-patches <gcc- > patches@gcc.gnu.org> wrote: > > > > Hi All, > > > > This is a small improvement in QoL codegen for match.pd to save time > > not re-evaluating the condition for printing debug information in every > function. > > > > There is a small but consistent runtime and compile time win here. > > The runtime win comes from not having to do the condition over again, > > and on Arm plaforms we now use the new test-and-branch support for > > booleans to only have a single instruction here. > > > > Compile time win is gotten from not having to do all the string > > parsing for the printf and having less string interning to do. > > > > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > > > > Ok for master? > > Ugh, I don't like the new global very much. Can't we compute it in the toplevel > entry and pass it down as parameter? Like passing down the actual dump FILE > *? So the dumpfile itself is currently also a global, I did try wiring this down, but the problem here is that eventually at the very top level, I need to modify the gimple_simplify calls because of the overloads being created by the output, which also need the parameter. There things become interesting because this then conflicts with the definitions in gimple-fold which would also need to take an additional argument and it breaks the public API. Wiring the dbg value through all the generated and public function requires quite a lot of changes so I'm not sure this is worth it in that case. Should I just drop it? Thanks, Tamar > > The file output in output_line_directive was because we originally had > match.pd #includeing multiple match-*.pd files, we'd want to keep that > supported I think. But since the line directives are commented and there's the > same info available below, like > > /* #line 798 "/home/rguenther/src/gcc-13-branch/gcc/match.pd" */ > tree captures[2] ATTRIBUTE_UNUSED = { _p0, _p1 }; > if (UNLIKELY (dump_file && (dump_flags & > TDF_FOLDING))) fprintf (dump_file, "Matching expression %s:%d, %s:%d\n", > "match.pd", 798, __FILE__, __LINE__); > > there's probably no point in emitting them anymore (originally I emitted them > non-commented but that didn't improve debugging much). We might want > to emit more "proper" line directives for the natively copied parts of match.pd > when code-generating c_expr parts, but that would be something separate. > > Can you split the patch into two things? A patch removing output of the > commented line directives at the call sites is OK. > > Richard. > > > Thanks, > > Tamar > > > > gcc/ChangeLog: > > > > PR bootstrap/84402 > > * dumpfile.h (dump_folding_p): New. > > * dumpfile.cc (set_dump_file): Use it. > > * generic-match-head.cc (dump_debug): New. > > * gimple-match-head.cc (dump_debug): New. > > * genmatch.cc (output_line_directive): Support outputting only line > > because file is implied. > > (dt_simplify::gen_1): Call debug_dump instead of printf. > > > > --- inline copy of patch -- > > diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h index > > > 7d5eca899dcc98676a9ce7a7efff8e439854ff89..e7b595ddecdcca9983d958 > 4b8b24 > > 17ae1941c7d4 100644 > > --- a/gcc/dumpfile.h > > +++ b/gcc/dumpfile.h > > @@ -522,6 +522,7 @@ parse_dump_option (const char *, const char **); > > extern FILE *dump_file; extern dump_flags_t dump_flags; extern const > > char *dump_file_name; > > +extern bool dump_folding_p; > > > > extern bool dumps_are_enabled; > > > > diff --git a/gcc/dumpfile.cc b/gcc/dumpfile.cc index > > > 51f68c8c6b40051ba3125c84298ee44ca52f5d17..f805aa73f3aa244d84714 > 9eec265 > > 05181ce4efe8 100644 > > --- a/gcc/dumpfile.cc > > +++ b/gcc/dumpfile.cc > > @@ -63,6 +63,7 @@ FILE *dump_file = NULL; const char *dump_file_name; > > dump_flags_t dump_flags; bool dumps_are_enabled = false; > > +bool dump_folding_p = false; > > > > > > /* Set global "dump_file" to NEW_DUMP_FILE, refreshing the > "dumps_are_enabled" > > @@ -73,6 +74,7 @@ set_dump_file (FILE *new_dump_file) { > > dumpfile_ensure_any_optinfo_are_flushed (); > > dump_file = new_dump_file; > > + dump_folding_p = dump_file && (dump_flags & TDF_FOLDING); > > dump_context::get ().refresh_dumps_are_enabled (); } > > > > diff --git a/gcc/generic-match-head.cc b/gcc/generic-match-head.cc > > index > > > f011204c5be450663231bdece0596317b37f9f9b..16b8f9f3b61d3d5651a5 > a41a8c05 > > 52f50b55cc7c 100644 > > --- a/gcc/generic-match-head.cc > > +++ b/gcc/generic-match-head.cc > > @@ -102,3 +102,17 @@ optimize_successive_divisions_p (tree, tree) { > > return false; > > } > > + > > +/* Helper method for debug printing to reducing string parsing overhead. > Keep > > + in sync with version in gimple-match-head.cc. */ > > + > > +static > > +void dump_debug (bool simplify, int loc, const char *file, int > > +lineno) { > > + if (simplify) > > + fprintf (dump_file, "Applying pattern %s:%d, %s:%d\n", "match.pd", loc, > > + file, lineno); > > + else > > + fprintf (dump_file, "Matching expression %s:%d, %s:%d\n", "match.pd", > loc, > > + file, lineno); > > +} > > \ No newline at end of file > > diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc index > > > 638606b2502f640e59527fc5a0b23fa3bedd0cee..bd7c6ff4a3fb89d456b022 > 42707f > > d823b737f20d 100644 > > --- a/gcc/genmatch.cc > > +++ b/gcc/genmatch.cc > > @@ -185,7 +185,8 @@ fprintf_indent (FILE *f, unsigned int indent, > > const char *format, ...) > > > > static void > > output_line_directive (FILE *f, location_t location, > > - bool dumpfile = false, bool fnargs = false) > > + bool dumpfile = false, bool fnargs = false, > > + bool loc_only = false) > > { > > const line_map_ordinary *map; > > linemap_resolve_location (line_table, location, > > LRK_SPELLING_LOCATION, &map); @@ -204,7 +205,9 @@ > output_line_directive (FILE *f, location_t location, > > else > > ++file; > > > > - if (fnargs) > > + if (loc_only) > > + fprintf (f, "%d", loc.line); > > + else if (fnargs) > > fprintf (f, "\"%s\", %d", file, loc.line); > > else > > fprintf (f, "%s:%d", file, loc.line); @@ -3431,14 +3434,11 @@ > > dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result) > > needs_label = true; > > } > > > > - fprintf_indent (f, indent, "if (UNLIKELY (dump_file && (dump_flags & > TDF_FOLDING))) " > > - "fprintf (dump_file, \"%s ", > > - s->kind == simplify::SIMPLIFY > > - ? "Applying pattern" : "Matching expression"); > > - fprintf (f, "%%s:%%d, %%s:%%d\\n\", "); > > + fprintf_indent (f, indent, "if (UNLIKELY (dump_folding_p)) " > > + "dump_debug (%s, ", s->kind == simplify::SIMPLIFY ? "true" : > > + "false"); > > output_line_directive (f, > > result ? result->location : s->match->location, true, > > - true); > > + true, true); > > fprintf (f, ", __FILE__, __LINE__);\n"); > > > > fprintf_indent (f, indent, "{\n"); > > diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc index > > > ec603f9d043c3924ea442bb49b5300a3573503cf..ae0c5c8a74fd9f1acdb616 > 014941 > > b11961e96c04 100644 > > --- a/gcc/gimple-match-head.cc > > +++ b/gcc/gimple-match-head.cc > > @@ -1412,3 +1412,17 @@ get_conditional_internal_fn (code_helper code, > tree type) > > auto cfn = combined_fn (code); > > return get_conditional_internal_fn (associated_internal_fn (cfn, > > type)); } > > + > > +/* Helper method for debug printing to reducing string parsing overhead. > Keep > > + in sync with version in generic-match-head.cc. */ > > + > > +static > > +void dump_debug (bool simplify, int loc, const char *file, int > > +lineno) { > > + if (simplify) > > + fprintf (dump_file, "Applying pattern %s:%d, %s:%d\n", "match.pd", loc, > > + file, lineno); > > + else > > + fprintf (dump_file, "Matching expression %s:%d, %s:%d\n", "match.pd", > loc, > > + file, lineno); > > +} > > \ No newline at end of file > > > > > > > > > > --
On Tue, 25 Apr 2023, Tamar Christina wrote: > > -----Original Message----- > > From: Richard Biener <richard.guenther@gmail.com> > > Sent: Tuesday, April 18, 2023 11:48 AM > > To: Tamar Christina <Tamar.Christina@arm.com> > > Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; rguenther@suse.de; > > jlaw@ventanamicro.com > > Subject: Re: [PATCH 2/3]middle-end match.pd: simplify debug dump checks > > > > On Tue, Apr 18, 2023 at 12:22?PM Tamar Christina via Gcc-patches <gcc- > > patches@gcc.gnu.org> wrote: > > > > > > Hi All, > > > > > > This is a small improvement in QoL codegen for match.pd to save time > > > not re-evaluating the condition for printing debug information in every > > function. > > > > > > There is a small but consistent runtime and compile time win here. > > > The runtime win comes from not having to do the condition over again, > > > and on Arm plaforms we now use the new test-and-branch support for > > > booleans to only have a single instruction here. > > > > > > Compile time win is gotten from not having to do all the string > > > parsing for the printf and having less string interning to do. > > > > > > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > > > > > > Ok for master? > > > > Ugh, I don't like the new global very much. Can't we compute it in the toplevel > > entry and pass it down as parameter? Like passing down the actual dump FILE > > *? > > So the dumpfile itself is currently also a global, I did try wiring this down, but the > problem here is that eventually at the very top level, I need to modify the gimple_simplify > calls because of the overloads being created by the output, which also need the parameter. > > There things become interesting because this then conflicts with the definitions in gimple-fold > which would also need to take an additional argument and it breaks the public API. > > Wiring the dbg value through all the generated and public function requires quite a lot of changes > so I'm not sure this is worth it in that case. Should I just drop it? Yeah, just drop it then. Btw, I wasn't suggesting to pass it down from the actual API but compute it once (locally) at the toplevel entries in the generated gimple-match.cc and pass it down from there. So we'd retain one (unconditionally done ...) FILE *local_dump_file = (dump_flags & TDF_folding) ? dump_file : NULL; and pass that down to functions called. Richard. > Thanks, > Tamar > > > > > The file output in output_line_directive was because we originally had > > match.pd #includeing multiple match-*.pd files, we'd want to keep that > > supported I think. But since the line directives are commented and there's the > > same info available below, like > > > > /* #line 798 "/home/rguenther/src/gcc-13-branch/gcc/match.pd" */ > > tree captures[2] ATTRIBUTE_UNUSED = { _p0, _p1 }; > > if (UNLIKELY (dump_file && (dump_flags & > > TDF_FOLDING))) fprintf (dump_file, "Matching expression %s:%d, %s:%d\n", > > "match.pd", 798, __FILE__, __LINE__); > > > > there's probably no point in emitting them anymore (originally I emitted them > > non-commented but that didn't improve debugging much). We might want > > to emit more "proper" line directives for the natively copied parts of match.pd > > when code-generating c_expr parts, but that would be something separate. > > > > Can you split the patch into two things? A patch removing output of the > > commented line directives at the call sites is OK. > > > > Richard. > > > > > Thanks, > > > Tamar > > > > > > gcc/ChangeLog: > > > > > > PR bootstrap/84402 > > > * dumpfile.h (dump_folding_p): New. > > > * dumpfile.cc (set_dump_file): Use it. > > > * generic-match-head.cc (dump_debug): New. > > > * gimple-match-head.cc (dump_debug): New. > > > * genmatch.cc (output_line_directive): Support outputting only line > > > because file is implied. > > > (dt_simplify::gen_1): Call debug_dump instead of printf. > > > > > > --- inline copy of patch -- > > > diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h index > > > > > 7d5eca899dcc98676a9ce7a7efff8e439854ff89..e7b595ddecdcca9983d958 > > 4b8b24 > > > 17ae1941c7d4 100644 > > > --- a/gcc/dumpfile.h > > > +++ b/gcc/dumpfile.h > > > @@ -522,6 +522,7 @@ parse_dump_option (const char *, const char **); > > > extern FILE *dump_file; extern dump_flags_t dump_flags; extern const > > > char *dump_file_name; > > > +extern bool dump_folding_p; > > > > > > extern bool dumps_are_enabled; > > > > > > diff --git a/gcc/dumpfile.cc b/gcc/dumpfile.cc index > > > > > 51f68c8c6b40051ba3125c84298ee44ca52f5d17..f805aa73f3aa244d84714 > > 9eec265 > > > 05181ce4efe8 100644 > > > --- a/gcc/dumpfile.cc > > > +++ b/gcc/dumpfile.cc > > > @@ -63,6 +63,7 @@ FILE *dump_file = NULL; const char *dump_file_name; > > > dump_flags_t dump_flags; bool dumps_are_enabled = false; > > > +bool dump_folding_p = false; > > > > > > > > > /* Set global "dump_file" to NEW_DUMP_FILE, refreshing the > > "dumps_are_enabled" > > > @@ -73,6 +74,7 @@ set_dump_file (FILE *new_dump_file) { > > > dumpfile_ensure_any_optinfo_are_flushed (); > > > dump_file = new_dump_file; > > > + dump_folding_p = dump_file && (dump_flags & TDF_FOLDING); > > > dump_context::get ().refresh_dumps_are_enabled (); } > > > > > > diff --git a/gcc/generic-match-head.cc b/gcc/generic-match-head.cc > > > index > > > > > f011204c5be450663231bdece0596317b37f9f9b..16b8f9f3b61d3d5651a5 > > a41a8c05 > > > 52f50b55cc7c 100644 > > > --- a/gcc/generic-match-head.cc > > > +++ b/gcc/generic-match-head.cc > > > @@ -102,3 +102,17 @@ optimize_successive_divisions_p (tree, tree) { > > > return false; > > > } > > > + > > > +/* Helper method for debug printing to reducing string parsing overhead. > > Keep > > > + in sync with version in gimple-match-head.cc. */ > > > + > > > +static > > > +void dump_debug (bool simplify, int loc, const char *file, int > > > +lineno) { > > > + if (simplify) > > > + fprintf (dump_file, "Applying pattern %s:%d, %s:%d\n", "match.pd", loc, > > > + file, lineno); > > > + else > > > + fprintf (dump_file, "Matching expression %s:%d, %s:%d\n", "match.pd", > > loc, > > > + file, lineno); > > > +} > > > \ No newline at end of file > > > diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc index > > > > > 638606b2502f640e59527fc5a0b23fa3bedd0cee..bd7c6ff4a3fb89d456b022 > > 42707f > > > d823b737f20d 100644 > > > --- a/gcc/genmatch.cc > > > +++ b/gcc/genmatch.cc > > > @@ -185,7 +185,8 @@ fprintf_indent (FILE *f, unsigned int indent, > > > const char *format, ...) > > > > > > static void > > > output_line_directive (FILE *f, location_t location, > > > - bool dumpfile = false, bool fnargs = false) > > > + bool dumpfile = false, bool fnargs = false, > > > + bool loc_only = false) > > > { > > > const line_map_ordinary *map; > > > linemap_resolve_location (line_table, location, > > > LRK_SPELLING_LOCATION, &map); @@ -204,7 +205,9 @@ > > output_line_directive (FILE *f, location_t location, > > > else > > > ++file; > > > > > > - if (fnargs) > > > + if (loc_only) > > > + fprintf (f, "%d", loc.line); > > > + else if (fnargs) > > > fprintf (f, "\"%s\", %d", file, loc.line); > > > else > > > fprintf (f, "%s:%d", file, loc.line); @@ -3431,14 +3434,11 @@ > > > dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result) > > > needs_label = true; > > > } > > > > > > - fprintf_indent (f, indent, "if (UNLIKELY (dump_file && (dump_flags & > > TDF_FOLDING))) " > > > - "fprintf (dump_file, \"%s ", > > > - s->kind == simplify::SIMPLIFY > > > - ? "Applying pattern" : "Matching expression"); > > > - fprintf (f, "%%s:%%d, %%s:%%d\\n\", "); > > > + fprintf_indent (f, indent, "if (UNLIKELY (dump_folding_p)) " > > > + "dump_debug (%s, ", s->kind == simplify::SIMPLIFY ? "true" : > > > + "false"); > > > output_line_directive (f, > > > result ? result->location : s->match->location, true, > > > - true); > > > + true, true); > > > fprintf (f, ", __FILE__, __LINE__);\n"); > > > > > > fprintf_indent (f, indent, "{\n"); > > > diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc index > > > > > ec603f9d043c3924ea442bb49b5300a3573503cf..ae0c5c8a74fd9f1acdb616 > > 014941 > > > b11961e96c04 100644 > > > --- a/gcc/gimple-match-head.cc > > > +++ b/gcc/gimple-match-head.cc > > > @@ -1412,3 +1412,17 @@ get_conditional_internal_fn (code_helper code, > > tree type) > > > auto cfn = combined_fn (code); > > > return get_conditional_internal_fn (associated_internal_fn (cfn, > > > type)); } > > > + > > > +/* Helper method for debug printing to reducing string parsing overhead. > > Keep > > > + in sync with version in generic-match-head.cc. */ > > > + > > > +static > > > +void dump_debug (bool simplify, int loc, const char *file, int > > > +lineno) { > > > + if (simplify) > > > + fprintf (dump_file, "Applying pattern %s:%d, %s:%d\n", "match.pd", loc, > > > + file, lineno); > > > + else > > > + fprintf (dump_file, "Matching expression %s:%d, %s:%d\n", "match.pd", > > loc, > > > + file, lineno); > > > +} > > > \ No newline at end of file > > > > > > > > > > > > > > > -- >
> -----Original Message----- > From: Richard Biener <rguenther@suse.de> > Sent: Tuesday, April 25, 2023 2:14 PM > To: Tamar Christina <Tamar.Christina@arm.com> > Cc: Richard Biener <richard.guenther@gmail.com>; gcc-patches@gcc.gnu.org; > nd <nd@arm.com>; jlaw@ventanamicro.com > Subject: RE: [PATCH 2/3]middle-end match.pd: simplify debug dump checks > > On Tue, 25 Apr 2023, Tamar Christina wrote: > > > > -----Original Message----- > > > From: Richard Biener <richard.guenther@gmail.com> > > > Sent: Tuesday, April 18, 2023 11:48 AM > > > To: Tamar Christina <Tamar.Christina@arm.com> > > > Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; rguenther@suse.de; > > > jlaw@ventanamicro.com > > > Subject: Re: [PATCH 2/3]middle-end match.pd: simplify debug dump > > > checks > > > > > > On Tue, Apr 18, 2023 at 12:22?PM Tamar Christina via Gcc-patches > > > <gcc- patches@gcc.gnu.org> wrote: > > > > > > > > Hi All, > > > > > > > > This is a small improvement in QoL codegen for match.pd to save > > > > time not re-evaluating the condition for printing debug > > > > information in every > > > function. > > > > > > > > There is a small but consistent runtime and compile time win here. > > > > The runtime win comes from not having to do the condition over > > > > again, and on Arm plaforms we now use the new test-and-branch > > > > support for booleans to only have a single instruction here. > > > > > > > > Compile time win is gotten from not having to do all the string > > > > parsing for the printf and having less string interning to do. > > > > > > > > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > > > > > > > > Ok for master? > > > > > > Ugh, I don't like the new global very much. Can't we compute it in > > > the toplevel entry and pass it down as parameter? Like passing down > > > the actual dump FILE *? > > > > So the dumpfile itself is currently also a global, I did try wiring > > this down, but the problem here is that eventually at the very top > > level, I need to modify the gimple_simplify calls because of the overloads > being created by the output, which also need the parameter. > > > > There things become interesting because this then conflicts with the > > definitions in gimple-fold which would also need to take an additional > argument and it breaks the public API. > > > > Wiring the dbg value through all the generated and public function > > requires quite a lot of changes so I'm not sure this is worth it in that case. > Should I just drop it? > > Yeah, just drop it then. Btw, I wasn't suggesting to pass it down from the > actual API but compute it once (locally) at the toplevel entries in the generated > gimple-match.cc and pass it down from there. So we'd retain one > (unconditionally done ...) > > FILE *local_dump_file = (dump_flags & TDF_folding) ? dump_file : NULL; > > and pass that down to functions called. Oohhhh apologies, that makes much more sense! I could have indeed stopped at gimple-match.cc and compute there. Fair enough. Let me make the change. Thanks!, Tamar > > Richard. > > > > Thanks, > > Tamar > > > > > > > > The file output in output_line_directive was because we originally > > > had match.pd #includeing multiple match-*.pd files, we'd want to > > > keep that supported I think. But since the line directives are > > > commented and there's the same info available below, like > > > > > > /* #line 798 "/home/rguenther/src/gcc-13-branch/gcc/match.pd" */ > > > tree captures[2] ATTRIBUTE_UNUSED = { _p0, _p1 }; > > > if (UNLIKELY (dump_file && (dump_flags & > > > TDF_FOLDING))) fprintf (dump_file, "Matching expression %s:%d, > > > %s:%d\n", "match.pd", 798, __FILE__, __LINE__); > > > > > > there's probably no point in emitting them anymore (originally I > > > emitted them non-commented but that didn't improve debugging much). > > > We might want to emit more "proper" line directives for the natively > > > copied parts of match.pd when code-generating c_expr parts, but that > would be something separate. > > > > > > Can you split the patch into two things? A patch removing output of > > > the commented line directives at the call sites is OK. > > > > > > Richard. > > > > > > > Thanks, > > > > Tamar > > > > > > > > gcc/ChangeLog: > > > > > > > > PR bootstrap/84402 > > > > * dumpfile.h (dump_folding_p): New. > > > > * dumpfile.cc (set_dump_file): Use it. > > > > * generic-match-head.cc (dump_debug): New. > > > > * gimple-match-head.cc (dump_debug): New. > > > > * genmatch.cc (output_line_directive): Support outputting only line > > > > because file is implied. > > > > (dt_simplify::gen_1): Call debug_dump instead of printf. > > > > > > > > --- inline copy of patch -- > > > > diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h index > > > > > > > > 7d5eca899dcc98676a9ce7a7efff8e439854ff89..e7b595ddecdcca9983d958 > > > 4b8b24 > > > > 17ae1941c7d4 100644 > > > > --- a/gcc/dumpfile.h > > > > +++ b/gcc/dumpfile.h > > > > @@ -522,6 +522,7 @@ parse_dump_option (const char *, const char > > > > **); extern FILE *dump_file; extern dump_flags_t dump_flags; > > > > extern const char *dump_file_name; > > > > +extern bool dump_folding_p; > > > > > > > > extern bool dumps_are_enabled; > > > > > > > > diff --git a/gcc/dumpfile.cc b/gcc/dumpfile.cc index > > > > > > > > 51f68c8c6b40051ba3125c84298ee44ca52f5d17..f805aa73f3aa244d84714 > > > 9eec265 > > > > 05181ce4efe8 100644 > > > > --- a/gcc/dumpfile.cc > > > > +++ b/gcc/dumpfile.cc > > > > @@ -63,6 +63,7 @@ FILE *dump_file = NULL; const char > > > > *dump_file_name; dump_flags_t dump_flags; bool dumps_are_enabled > > > > = false; > > > > +bool dump_folding_p = false; > > > > > > > > > > > > /* Set global "dump_file" to NEW_DUMP_FILE, refreshing the > > > "dumps_are_enabled" > > > > @@ -73,6 +74,7 @@ set_dump_file (FILE *new_dump_file) { > > > > dumpfile_ensure_any_optinfo_are_flushed (); > > > > dump_file = new_dump_file; > > > > + dump_folding_p = dump_file && (dump_flags & TDF_FOLDING); > > > > dump_context::get ().refresh_dumps_are_enabled (); } > > > > > > > > diff --git a/gcc/generic-match-head.cc b/gcc/generic-match-head.cc > > > > index > > > > > > > > f011204c5be450663231bdece0596317b37f9f9b..16b8f9f3b61d3d5651a5 > > > a41a8c05 > > > > 52f50b55cc7c 100644 > > > > --- a/gcc/generic-match-head.cc > > > > +++ b/gcc/generic-match-head.cc > > > > @@ -102,3 +102,17 @@ optimize_successive_divisions_p (tree, tree) { > > > > return false; > > > > } > > > > + > > > > +/* Helper method for debug printing to reducing string parsing > overhead. > > > Keep > > > > + in sync with version in gimple-match-head.cc. */ > > > > + > > > > +static > > > > +void dump_debug (bool simplify, int loc, const char *file, int > > > > +lineno) { > > > > + if (simplify) > > > > + fprintf (dump_file, "Applying pattern %s:%d, %s:%d\n", "match.pd", > loc, > > > > + file, lineno); > > > > + else > > > > + fprintf (dump_file, "Matching expression %s:%d, %s:%d\n", > > > > +"match.pd", > > > loc, > > > > + file, lineno); > > > > +} > > > > \ No newline at end of file > > > > diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc index > > > > > > > > 638606b2502f640e59527fc5a0b23fa3bedd0cee..bd7c6ff4a3fb89d456b022 > > > 42707f > > > > d823b737f20d 100644 > > > > --- a/gcc/genmatch.cc > > > > +++ b/gcc/genmatch.cc > > > > @@ -185,7 +185,8 @@ fprintf_indent (FILE *f, unsigned int indent, > > > > const char *format, ...) > > > > > > > > static void > > > > output_line_directive (FILE *f, location_t location, > > > > - bool dumpfile = false, bool fnargs = false) > > > > + bool dumpfile = false, bool fnargs = false, > > > > + bool loc_only = false) > > > > { > > > > const line_map_ordinary *map; > > > > linemap_resolve_location (line_table, location, > > > > LRK_SPELLING_LOCATION, &map); @@ -204,7 +205,9 @@ > > > output_line_directive (FILE *f, location_t location, > > > > else > > > > ++file; > > > > > > > > - if (fnargs) > > > > + if (loc_only) > > > > + fprintf (f, "%d", loc.line); > > > > + else if (fnargs) > > > > fprintf (f, "\"%s\", %d", file, loc.line); > > > > else > > > > fprintf (f, "%s:%d", file, loc.line); @@ -3431,14 +3434,11 > > > > @@ > > > > dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result) > > > > needs_label = true; > > > > } > > > > > > > > - fprintf_indent (f, indent, "if (UNLIKELY (dump_file && > > > > (dump_flags & > > > TDF_FOLDING))) " > > > > - "fprintf (dump_file, \"%s ", > > > > - s->kind == simplify::SIMPLIFY > > > > - ? "Applying pattern" : "Matching expression"); > > > > - fprintf (f, "%%s:%%d, %%s:%%d\\n\", "); > > > > + fprintf_indent (f, indent, "if (UNLIKELY (dump_folding_p)) " > > > > + "dump_debug (%s, ", s->kind == simplify::SIMPLIFY ? "true" : > > > > + "false"); > > > > output_line_directive (f, > > > > result ? result->location : s->match->location, true, > > > > - true); > > > > + true, true); > > > > fprintf (f, ", __FILE__, __LINE__);\n"); > > > > > > > > fprintf_indent (f, indent, "{\n"); diff --git > > > > a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc index > > > > > > > > ec603f9d043c3924ea442bb49b5300a3573503cf..ae0c5c8a74fd9f1acdb616 > > > 014941 > > > > b11961e96c04 100644 > > > > --- a/gcc/gimple-match-head.cc > > > > +++ b/gcc/gimple-match-head.cc > > > > @@ -1412,3 +1412,17 @@ get_conditional_internal_fn (code_helper > > > > code, > > > tree type) > > > > auto cfn = combined_fn (code); > > > > return get_conditional_internal_fn (associated_internal_fn > > > > (cfn, type)); } > > > > + > > > > +/* Helper method for debug printing to reducing string parsing > overhead. > > > Keep > > > > + in sync with version in generic-match-head.cc. */ > > > > + > > > > +static > > > > +void dump_debug (bool simplify, int loc, const char *file, int > > > > +lineno) { > > > > + if (simplify) > > > > + fprintf (dump_file, "Applying pattern %s:%d, %s:%d\n", "match.pd", > loc, > > > > + file, lineno); > > > > + else > > > > + fprintf (dump_file, "Matching expression %s:%d, %s:%d\n", > > > > +"match.pd", > > > loc, > > > > + file, lineno); > > > > +} > > > > \ No newline at end of file > > > > > > > > > > > > > > > > > > > > -- > > > > -- > Richard Biener <rguenther@suse.de> > SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 > Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, > Boudien Moerman; HRB 36809 (AG Nuernberg)
--- a/gcc/dumpfile.h +++ b/gcc/dumpfile.h @@ -522,6 +522,7 @@ parse_dump_option (const char *, const char **); extern FILE *dump_file; extern dump_flags_t dump_flags; extern const char *dump_file_name; +extern bool dump_folding_p; extern bool dumps_are_enabled; diff --git a/gcc/dumpfile.cc b/gcc/dumpfile.cc index 51f68c8c6b40051ba3125c84298ee44ca52f5d17..f805aa73f3aa244d847149eec26505181ce4efe8 100644 --- a/gcc/dumpfile.cc +++ b/gcc/dumpfile.cc @@ -63,6 +63,7 @@ FILE *dump_file = NULL; const char *dump_file_name; dump_flags_t dump_flags; bool dumps_are_enabled = false; +bool dump_folding_p = false; /* Set global "dump_file" to NEW_DUMP_FILE, refreshing the "dumps_are_enabled" @@ -73,6 +74,7 @@ set_dump_file (FILE *new_dump_file) { dumpfile_ensure_any_optinfo_are_flushed (); dump_file = new_dump_file; + dump_folding_p = dump_file && (dump_flags & TDF_FOLDING); dump_context::get ().refresh_dumps_are_enabled (); } diff --git a/gcc/generic-match-head.cc b/gcc/generic-match-head.cc index f011204c5be450663231bdece0596317b37f9f9b..16b8f9f3b61d3d5651a5a41a8c0552f50b55cc7c 100644 --- a/gcc/generic-match-head.cc +++ b/gcc/generic-match-head.cc @@ -102,3 +102,17 @@ optimize_successive_divisions_p (tree, tree) { return false; } + +/* Helper method for debug printing to reducing string parsing overhead. Keep + in sync with version in gimple-match-head.cc. */ + +static +void dump_debug (bool simplify, int loc, const char *file, int lineno) +{ + if (simplify) + fprintf (dump_file, "Applying pattern %s:%d, %s:%d\n", "match.pd", loc, + file, lineno); + else + fprintf (dump_file, "Matching expression %s:%d, %s:%d\n", "match.pd", loc, + file, lineno); +} \ No newline at end of file diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc index 638606b2502f640e59527fc5a0b23fa3bedd0cee..bd7c6ff4a3fb89d456b02242707fd823b737f20d 100644 --- a/gcc/genmatch.cc +++ b/gcc/genmatch.cc @@ -185,7 +185,8 @@ fprintf_indent (FILE *f, unsigned int indent, const char *format, ...) static void output_line_directive (FILE *f, location_t location, - bool dumpfile = false, bool fnargs = false) + bool dumpfile = false, bool fnargs = false, + bool loc_only = false) { const line_map_ordinary *map; linemap_resolve_location (line_table, location, LRK_SPELLING_LOCATION, &map); @@ -204,7 +205,9 @@ output_line_directive (FILE *f, location_t location, else ++file; - if (fnargs) + if (loc_only) + fprintf (f, "%d", loc.line); + else if (fnargs) fprintf (f, "\"%s\", %d", file, loc.line); else fprintf (f, "%s:%d", file, loc.line); @@ -3431,14 +3434,11 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result) needs_label = true; } - fprintf_indent (f, indent, "if (UNLIKELY (dump_file && (dump_flags & TDF_FOLDING))) " - "fprintf (dump_file, \"%s ", - s->kind == simplify::SIMPLIFY - ? "Applying pattern" : "Matching expression"); - fprintf (f, "%%s:%%d, %%s:%%d\\n\", "); + fprintf_indent (f, indent, "if (UNLIKELY (dump_folding_p)) " + "dump_debug (%s, ", s->kind == simplify::SIMPLIFY ? "true" : "false"); output_line_directive (f, result ? result->location : s->match->location, true, - true); + true, true); fprintf (f, ", __FILE__, __LINE__);\n"); fprintf_indent (f, indent, "{\n"); diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc index ec603f9d043c3924ea442bb49b5300a3573503cf..ae0c5c8a74fd9f1acdb616014941b11961e96c04 100644 --- a/gcc/gimple-match-head.cc +++ b/gcc/gimple-match-head.cc @@ -1412,3 +1412,17 @@ get_conditional_internal_fn (code_helper code, tree type) auto cfn = combined_fn (code); return get_conditional_internal_fn (associated_internal_fn (cfn, type)); } + +/* Helper method for debug printing to reducing string parsing overhead. Keep + in sync with version in generic-match-head.cc. */ + +static +void dump_debug (bool simplify, int loc, const char *file, int lineno) +{ + if (simplify) + fprintf (dump_file, "Applying pattern %s:%d, %s:%d\n", "match.pd", loc, + file, lineno); + else + fprintf (dump_file, "Matching expression %s:%d, %s:%d\n", "match.pd", loc, + file, lineno); +} \ No newline at end of file
Hi All, This is a small improvement in QoL codegen for match.pd to save time not re-evaluating the condition for printing debug information in every function. There is a small but consistent runtime and compile time win here. The runtime win comes from not having to do the condition over again, and on Arm plaforms we now use the new test-and-branch support for booleans to only have a single instruction here. Compile time win is gotten from not having to do all the string parsing for the printf and having less string interning to do. Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. Ok for master? Thanks, Tamar gcc/ChangeLog: PR bootstrap/84402 * dumpfile.h (dump_folding_p): New. * dumpfile.cc (set_dump_file): Use it. * generic-match-head.cc (dump_debug): New. * gimple-match-head.cc (dump_debug): New. * genmatch.cc (output_line_directive): Support outputting only line because file is implied. (dt_simplify::gen_1): Call debug_dump instead of printf. --- inline copy of patch -- diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h index 7d5eca899dcc98676a9ce7a7efff8e439854ff89..e7b595ddecdcca9983d9584b8b2417ae1941c7d4 100644 -- diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h index 7d5eca899dcc98676a9ce7a7efff8e439854ff89..e7b595ddecdcca9983d9584b8b2417ae1941c7d4 100644 --- a/gcc/dumpfile.h +++ b/gcc/dumpfile.h @@ -522,6 +522,7 @@ parse_dump_option (const char *, const char **); extern FILE *dump_file; extern dump_flags_t dump_flags; extern const char *dump_file_name; +extern bool dump_folding_p; extern bool dumps_are_enabled; diff --git a/gcc/dumpfile.cc b/gcc/dumpfile.cc index 51f68c8c6b40051ba3125c84298ee44ca52f5d17..f805aa73f3aa244d847149eec26505181ce4efe8 100644 --- a/gcc/dumpfile.cc +++ b/gcc/dumpfile.cc @@ -63,6 +63,7 @@ FILE *dump_file = NULL; const char *dump_file_name; dump_flags_t dump_flags; bool dumps_are_enabled = false; +bool dump_folding_p = false; /* Set global "dump_file" to NEW_DUMP_FILE, refreshing the "dumps_are_enabled" @@ -73,6 +74,7 @@ set_dump_file (FILE *new_dump_file) { dumpfile_ensure_any_optinfo_are_flushed (); dump_file = new_dump_file; + dump_folding_p = dump_file && (dump_flags & TDF_FOLDING); dump_context::get ().refresh_dumps_are_enabled (); } diff --git a/gcc/generic-match-head.cc b/gcc/generic-match-head.cc index f011204c5be450663231bdece0596317b37f9f9b..16b8f9f3b61d3d5651a5a41a8c0552f50b55cc7c 100644 --- a/gcc/generic-match-head.cc +++ b/gcc/generic-match-head.cc @@ -102,3 +102,17 @@ optimize_successive_divisions_p (tree, tree) { return false; } + +/* Helper method for debug printing to reducing string parsing overhead. Keep + in sync with version in gimple-match-head.cc. */ + +static +void dump_debug (bool simplify, int loc, const char *file, int lineno) +{ + if (simplify) + fprintf (dump_file, "Applying pattern %s:%d, %s:%d\n", "match.pd", loc, + file, lineno); + else + fprintf (dump_file, "Matching expression %s:%d, %s:%d\n", "match.pd", loc, + file, lineno); +} \ No newline at end of file diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc index 638606b2502f640e59527fc5a0b23fa3bedd0cee..bd7c6ff4a3fb89d456b02242707fd823b737f20d 100644 --- a/gcc/genmatch.cc +++ b/gcc/genmatch.cc @@ -185,7 +185,8 @@ fprintf_indent (FILE *f, unsigned int indent, const char *format, ...) static void output_line_directive (FILE *f, location_t location, - bool dumpfile = false, bool fnargs = false) + bool dumpfile = false, bool fnargs = false, + bool loc_only = false) { const line_map_ordinary *map; linemap_resolve_location (line_table, location, LRK_SPELLING_LOCATION, &map); @@ -204,7 +205,9 @@ output_line_directive (FILE *f, location_t location, else ++file; - if (fnargs) + if (loc_only) + fprintf (f, "%d", loc.line); + else if (fnargs) fprintf (f, "\"%s\", %d", file, loc.line); else fprintf (f, "%s:%d", file, loc.line); @@ -3431,14 +3434,11 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result) needs_label = true; } - fprintf_indent (f, indent, "if (UNLIKELY (dump_file && (dump_flags & TDF_FOLDING))) " - "fprintf (dump_file, \"%s ", - s->kind == simplify::SIMPLIFY - ? "Applying pattern" : "Matching expression"); - fprintf (f, "%%s:%%d, %%s:%%d\\n\", "); + fprintf_indent (f, indent, "if (UNLIKELY (dump_folding_p)) " + "dump_debug (%s, ", s->kind == simplify::SIMPLIFY ? "true" : "false"); output_line_directive (f, result ? result->location : s->match->location, true, - true); + true, true); fprintf (f, ", __FILE__, __LINE__);\n"); fprintf_indent (f, indent, "{\n"); diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc index ec603f9d043c3924ea442bb49b5300a3573503cf..ae0c5c8a74fd9f1acdb616014941b11961e96c04 100644 --- a/gcc/gimple-match-head.cc +++ b/gcc/gimple-match-head.cc @@ -1412,3 +1412,17 @@ get_conditional_internal_fn (code_helper code, tree type) auto cfn = combined_fn (code); return get_conditional_internal_fn (associated_internal_fn (cfn, type)); } + +/* Helper method for debug printing to reducing string parsing overhead. Keep + in sync with version in generic-match-head.cc. */ + +static +void dump_debug (bool simplify, int loc, const char *file, int lineno) +{ + if (simplify) + fprintf (dump_file, "Applying pattern %s:%d, %s:%d\n", "match.pd", loc, + file, lineno); + else + fprintf (dump_file, "Matching expression %s:%d, %s:%d\n", "match.pd", loc, + file, lineno); +} \ No newline at end of file