From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x236.google.com (mail-lj1-x236.google.com [IPv6:2a00:1450:4864:20::236]) by sourceware.org (Postfix) with ESMTPS id CFAFE3857724 for ; Fri, 4 Aug 2023 09:05:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CFAFE3857724 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lj1-x236.google.com with SMTP id 38308e7fff4ca-2b9fa64db41so29427471fa.1 for ; Fri, 04 Aug 2023 02:05:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1691139949; x=1691744749; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=kfYVMxYLPjeupknYLcLdp8pATgIFt0PWAXJaZHIOTr4=; b=CzDY1JhvFlgGO2O6ApmH54fy/0Vg9qSMd6QP141mAw4N8TColjL+8srKMKSJ2dPnA9 UhfNe67HB6IqsmSmMQpcgaMb/aW2XTasKUSwTfWlDxdDltpG1EUoTcvWKCMLaex6kngW o8xACBQQIxj5j2doYTRZhCWqCC12eXzxatvICr+rv2kLiQDuCZOMSCshs4lhIOitg/hH IMvD+ky3PrEU5H4hWr8VPIxGHyaO0qSzYesAbU5Oil4Fru6aetagy/wZPygUelp5smLc uV3KipaWC9DIy4aMXwbIomjQjmeC26gL+57a3Fo20MY+OJjPwkgbAWvdlZtOh4B1ATR+ 6DLw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691139949; x=1691744749; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=kfYVMxYLPjeupknYLcLdp8pATgIFt0PWAXJaZHIOTr4=; b=EKddODHBCQ/i9x+lC/bRllmwiHKUTU+3RAdioaJqY6bp3dz5J8j7/4/lsSrNLkjLJ6 pbUASNP4taRx3kHP4cpnNygXkO0q4Oeg1gl/anAN4IZmXWW/MatiAIFuqlBgfOG88Ekb mgnWvZxYehOlKofWICFgepikwTdhJdPIlcoQfo0J+aTaSCLx05BSp30sCcSpKChEBM4c 4DYKbenJReVA+jaDpMi/TRT75ba4DBNBEy50Dn5lMp6aWR0dtlij3GHWFiyhW2iBdhxL Cwpq1A3j9gIEy5BT9n7GYdhvPqhEcCwdGvsPkpIoGMzXNr72g6Uqle3hTtVnmjmFq3P+ em/g== X-Gm-Message-State: AOJu0Yz5kztNVL/B7djjwaOTgDVCwzv6NSo9tgzrKS8vQwrLvvaER4D0 /ICJpkzafw8fHtTE9eIICgbMhowhAsMZXEWawMnysy46PPE= X-Google-Smtp-Source: AGHT+IHQkUhBQKU4BjmDpAajShri8vNVpXXe+26lX8ROHxogiwGHkN4woL6ugOLryFghs/dR+ooU3rdn7F47rUl8w/Y= X-Received: by 2002:a2e:9a8a:0:b0:2b7:25b2:e37a with SMTP id p10-20020a2e9a8a000000b002b725b2e37amr1016194lji.44.1691139949122; Fri, 04 Aug 2023 02:05:49 -0700 (PDT) MIME-Version: 1.0 References: <20230803142131.250087-1-andrzej.turko@gmail.com> <20230803142131.250087-4-andrzej.turko@gmail.com> In-Reply-To: <20230803142131.250087-4-andrzej.turko@gmail.com> From: Richard Biener Date: Fri, 4 Aug 2023 11:05:37 +0200 Message-ID: Subject: Re: [PATCH 3/3 v3] genmatch: Log line numbers indirectly To: Andrzej Turko Cc: gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Thu, Aug 3, 2023 at 4:24=E2=80=AFPM Andrzej Turko via Gcc-patches 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. > > This change replaces those logging fprintf calls with calls to > a dedicated logging function. Because it reads the line numbers > from the lookup table, it is enough to pass a corresponding index. > Thanks to this, when match.pd changes, it is enough to rebuild > the file containing the lookup table and, of course, those > actually affected by the change. > > Signed-off-by: Andrzej Turko > > gcc/ChangeLog: > > * genmatch.cc: Log line numbers indirectly. > --- > gcc/genmatch.cc | 89 ++++++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 74 insertions(+), 15 deletions(-) > > diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc > index 1deca505603..63d6ba6dab0 100644 > --- a/gcc/genmatch.cc > +++ b/gcc/genmatch.cc > @@ -217,9 +217,57 @@ fp_decl_done (FILE *f, const char *trailer) > fprintf (header_file, "%s;", trailer); > } > > +/* Line numbers for use by indirect line directives. */ > +static vec dbg_line_numbers; > + > +static void > +write_header_declarations (bool gimple, FILE *f) > +{ > + fprintf (f, "\nextern void\n%s_dump_logs (const char *file1, int line1= _id, " > + "const char *file2, int line2, bool simplify);\n", > + gimple ? "gimple" : "generic"); > +} > + > +static void > +define_dump_logs (bool gimple, FILE *f) > +{ > + extra vertical space is unwanted here. > + if (dbg_line_numbers.is_empty ()) > + { > + fprintf (f, "};\n\n"); > + return; > + } shouldn't the above come after ... > + fprintf (f , "void\n%s_dump_logs (const char *file1, int line1_id, " > + "const char *file2, int line2, bool simplify)\n{\n", > + gimple ? "gimple" : "generic"); ... this? > + fprintf_indent (f, 2, "static int dbg_line_numbers[%d] =3D {", > + dbg_line_numbers.length ()); > + > + for (int i =3D 0; i < (int)dbg_line_numbers.length () - 1; i++) use an unsigned int to avoid the cast? > + { > + if (i % 20 =3D=3D 0) > + fprintf (f, "\n\t"); > + > + fprintf (f, "%d, ", dbg_line_numbers[i]); > + } > + fprintf (f, "%d\n };\n\n", dbg_line_numbers.last ()); > + > + > + fprintf_indent (f, 2, "fprintf (dump_file, \"%%s " > + "%%s:%%d, %%s:%%d\\n\",\n"); > + fprintf_indent (f, 10, "simplify ? \"Applying pattern\" : " > + "\"Matching expression\", file1, " > + "dbg_line_numbers[line1_id], file2, line2);"); > + > + fprintf (f, "\n}\n\n"); > +} > + > static void > output_line_directive (FILE *f, location_t location, > - bool dumpfile =3D false, bool fnargs =3D false) > + bool dumpfile =3D false, bool fnargs =3D false, > + bool indirect_line_numbers =3D false) > { > const line_map_ordinary *map; > linemap_resolve_location (line_table, location, LRK_SPELLING_LOCATION,= &map); > @@ -239,7 +287,15 @@ output_line_directive (FILE *f, location_t location, > ++file; > > if (fnargs) > - fprintf (f, "\"%s\", %d", file, loc.line); > + { > + if (indirect_line_numbers) > + { > + fprintf (f, "\"%s\", %d", file, dbg_line_numbers.length ()); > + dbg_line_numbers.safe_push (loc.line); > + } > + else > + fprintf (f, "\"%s\", %d", file, loc.line); > + } The indent is off here. I notice the same lines often appear repeatedly so a simple optimization like doing if (indirect_line_numbers) { if (!dbg_line_numbers.is_empty () && dbg_line_numbers.last () =3D=3D loc.line) ; else dbg_line_numbers.safe_push (loc.line); fprintf (f, "\"%s\", %d", file, dbg_line_numbers.length () - 1); } shrinks the table quite a bit (not all duplicates are gone this way). It doesn't seem we can easily keep the list sorted, adding another hash-map could avoid duplicates completely, maybe worth pursuing. Otherwise the patch series looks fine to me. Thanks, Richard. > else > fprintf (f, "%s:%d", file, loc.line); > } > @@ -3375,20 +3431,19 @@ 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 > +/* Emit a logging call 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 nul= l. */ > static void > -emit_debug_printf (FILE *f, int indent, class simplify *s, operand *resu= lt) > +emit_logging_call (FILE *f, int indent, class simplify *s, operand *resu= lt, > + bool gimple) > { > fprintf_indent (f, indent, "if (UNLIKELY (debug_dump)) " > - "fprintf (dump_file, \"%s ", > - s->kind =3D=3D simplify::SIMPLIFY > - ? "Applying pattern" : "Matching expression"); > - fprintf (f, "%%s:%%d, %%s:%%d\\n\", "); > + "%s_dump_logs (", gimple ? "gimple" : "generic"); > output_line_directive (f, > - result ? result->location : s->match->location, = true, > - true); > - fprintf (f, ", __FILE__, __LINE__);\n"); > + result ? result->location : s->match->location, > + true, true, true); > + fprintf (f, ", __FILE__, __LINE__, %s);\n", > + s->kind =3D=3D simplify::SIMPLIFY ? "true" : "false"); > } > > /* Generate code for the '(if ...)', '(with ..)' and actual transform > @@ -3524,7 +3579,7 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimpl= e, operand *result) > if (!result) > { > /* If there is no result then this is a predicate implementation. = */ > - emit_debug_printf (f, indent, s, result); > + emit_logging_call (f, indent, s, result, gimple); > fprintf_indent (f, indent, "return true;\n"); > } > else if (gimple) > @@ -3615,7 +3670,7 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimpl= e, operand *result) > } > else > gcc_unreachable (); > - emit_debug_printf (f, indent, s, result); > + emit_logging_call (f, indent, s, result, gimple); > fprintf_indent (f, indent, "return true;\n"); > } > else /* GENERIC */ > @@ -3670,7 +3725,7 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimpl= e, operand *result) > } > if (is_predicate) > { > - emit_debug_printf (f, indent, s, result); > + emit_logging_call (f, indent, s, result, gimple); > fprintf_indent (f, indent, "return true;\n"); > } > else > @@ -3738,7 +3793,7 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimpl= e, operand *result) > i); > } > } > - emit_debug_printf (f, indent, s, result); > + emit_logging_call (f, indent, s, result, gimple); > fprintf_indent (f, indent, "return _r;\n"); > } > } > @@ -5447,6 +5502,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 +5516,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 +5559,8 @@ main (int argc, char **argv) > > dt.gen (parts, gimple); > > + define_dump_logs (gimple, choose_output (parts)); > + > for (FILE *f : parts) > { > fprintf (f, "#pragma GCC diagnostic pop\n"); > -- > 2.34.1 >