On 09/03/2018 10:19 AM, Richard Biener wrote:
> On Mon, Sep 3, 2018 at 9:56 AM Martin Liška <[email protected]> wrote:
>>
>> Hi.
>>
>> This is small improvement for {gimple,generic}-match.c files.
>> The code path that reports application of a pattern is now guarded
>> with __builtin_expect. And reporting function lives in gimple.c.
>>
>> For gimple-match.o, difference is:
>>
>> bloaty /tmp/after.o -- /tmp/before.o
>> VM SIZE FILE SIZE
>> ++++++++++++++ GROWING ++++++++++++++
>> [ = ] 0 .rela.debug_loc +58.5Ki +0.5%
>> +0.7% +7.70Ki .text +7.70Ki +0.7%
>> [ = ] 0 .debug_info +3.53Ki +0.6%
>> [ = ] 0 .rela.debug_ranges +2.02Ki +0.0%
>> [ = ] 0 .debug_loc +1.86Ki +0.7%
>> +0.7% +448 .eh_frame +448 +0.7%
>> [ = ] 0 .rela.eh_frame +192 +0.7%
>> [ = ] 0 .rela.debug_line +48 +0.4%
>> [ = ] 0 .debug_str +26 +0.0%
>> +6.9% +9 .rodata.str1.1 +9 +6.9%
>>
>> -------------- SHRINKING --------------
>> -97.5% -24.8Ki .rodata.str1.8 -24.8Ki -97.5%
>> [ = ] 0 .symtab -14.7Ki -26.1%
>> [ = ] 0 .strtab -3.57Ki -2.2%
>> [ = ] 0 .rela.debug_info -2.81Ki -0.0%
>> [ = ] 0 .debug_line -2.14Ki -0.6%
>> [ = ] 0 .rela.text -816 -0.1%
>> [ = ] 0 .rela.text.unlikely -288 -0.1%
>> -0.1% -131 .text.unlikely -131 -0.1%
>> [ = ] 0 [Unmapped] -23 -14.0%
>> [ = ] 0 .debug_abbrev -2 -0.1%
>>
>> -1.2% -16.8Ki TOTAL +25.1Ki +0.1%
>>
>> I'm testing the patch.
>> Thoughts?
>
> Looks good in principle but why have the function in gimple.c
> rather than in gimple-match-head.c where it could be static?
> Do we still end up inlining it even though it is guarded with
> __builtin_expect?
Done that transformation:
#include "gimple-match-head.c"
static void report_match_pattern (const char *match_file, unsigned int
match_file_line, const char *generated_file, unsigned int generate_file_line)
{
fprintf (dump_file, "Applying pattern %s:%d, %s:%d\n",match_file,
match_file_line, generated_file, generate_file_line);
}
Yes, I can confirm it's inlined now.
Ready to install after proper testing?
Martin
>
> Richard.
>
>>
>> Martin
>>
>> gcc/ChangeLog:
>>
>> 2018-08-31 Martin Liska <[email protected]>
>>
>> * genmatch.c (output_line_directive): Add new argument
>> fnargs.
>> (dt_simplify::gen_1): Generate call to report_match_pattern
>> function and wrap that into __builtin_expect.
>> * gimple.c (report_match_pattern): New function.
>> * gimple.h (report_match_pattern): Likewise.
>> ---
>> gcc/genmatch.c | 18 ++++++++++++------
>> gcc/gimple.c | 14 ++++++++++++++
>> gcc/gimple.h | 4 ++++
>> 3 files changed, 30 insertions(+), 6 deletions(-)
>>
>>
>From 79c11daed376e1e157d0e7d867d690524f9df686 Mon Sep 17 00:00:00 2001
From: marxin <[email protected]>
Date: Fri, 31 Aug 2018 16:23:35 +0200
Subject: [PATCH] genmatch: isolate reporting into a function
gcc/ChangeLog:
2018-08-31 Martin Liska <[email protected]>
* genmatch.c (output_line_directive): Add new argument
fnargs.
(write_header): Generate static void report_match_pattern.
(dt_simplify::gen_1): Generate call to report_match_pattern
function and wrap that into __builtin_expect.
* genmatch.c:
---
gcc/genmatch.c | 29 ++++++++++++++++++++++-------
gcc/gimple.h | 1 -
2 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/gcc/genmatch.c b/gcc/genmatch.c
index 50d72f8f1e7..b73ba1bc8e9 100644
--- a/gcc/genmatch.c
+++ b/gcc/genmatch.c
@@ -184,7 +184,7 @@ fprintf_indent (FILE *f, unsigned int indent, const char *format, ...)
static void
output_line_directive (FILE *f, source_location location,
- bool dumpfile = false)
+ bool dumpfile = false, bool fnargs = false)
{
const line_map_ordinary *map;
linemap_resolve_location (line_table, location, LRK_SPELLING_LOCATION, &map);
@@ -202,7 +202,11 @@ output_line_directive (FILE *f, source_location location,
file = loc.file;
else
++file;
- fprintf (f, "%s:%d", file, loc.line);
+
+ if (fnargs)
+ fprintf (f, "\"%s\", %d", file, loc.line);
+ else
+ fprintf (f, "%s:%d", file, loc.line);
}
else
/* Other gen programs really output line directives here, at least for
@@ -3305,11 +3309,13 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result)
}
}
- fprintf_indent (f, indent, "if (dump_file && (dump_flags & TDF_FOLDING)) "
- "fprintf (dump_file, \"Applying pattern ");
+ fprintf_indent (f, indent, "if (__builtin_expect (dump_file && "
+ "(dump_flags & TDF_FOLDING), 0)) report_match_pattern (");
+
output_line_directive (f,
- result ? result->location : s->match->location, true);
- fprintf (f, ", %%s:%%d\\n\", __FILE__, __LINE__);\n");
+ result ? result->location : s->match->location, true,
+ true);
+ fprintf (f, ", __FILE__, __LINE__);\n");
if (!result)
{
@@ -3888,8 +3894,17 @@ write_header (FILE *f, const char *head)
/* Include the header instead of writing it awkwardly quoted here. */
fprintf (f, "\n#include \"%s\"\n", head);
-}
+ /* Generate report_match_pattern function. */
+ fprintf (f, "static void report_match_pattern (const char *match_file, "
+ "unsigned int match_file_line, "
+ "const char *generated_file, "
+ "unsigned int generate_file_line)\n");
+ fprintf (f, "{\n");
+ fprintf (f, "fprintf (dump_file, \"Applying pattern %%s:%%d, %%s:%%d\\n\","
+ "match_file, match_file_line, generated_file, generate_file_line);\n");
+ fprintf (f, "}\n");
+}
/* AST parsing. */
diff --git a/gcc/gimple.h b/gcc/gimple.h
index a5dda9369bc..2e1de5d5490 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -6404,7 +6404,6 @@ gimple_set_do_not_emit_location (gimple *g)
gimple_set_plf (g, GF_PLF_1, true);
}
-
/* Macros for showing usage statistics. */
#define SCALE(x) ((unsigned long) ((x) < 1024*10 \
? (x) \
--
2.18.0
