On Mon, Sep 3, 2018 at 2:09 PM Martin Liška <mli...@suse.cz> wrote:
>
> On 09/03/2018 10:19 AM, Richard Biener wrote:
> > On Mon, Sep 3, 2018 at 9:56 AM Martin Liška <mli...@suse.cz> 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.
Hmm, but that was the point of the exercise? Not inlining it? Or was
the point to have
the __builtin_expect()?
> Ready to install after proper testing?
Just occured to me you need a copy of that in generic-match-head.c.
But then, why not just add the __builtin_expect()...
> Martin
>
> >
> > Richard.
> >
> >>
> >> Martin
> >>
> >> gcc/ChangeLog:
> >>
> >> 2018-08-31 Martin Liska <mli...@suse.cz>
> >>
> >> * 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(-)
> >>
> >>
>
