| Message ID | 001a01d83931$8d6c5ee0$a8451ca0$@nextmovesoftware.com |
|---|---|
| State | New |
| Headers | show |
| Series | Avoid generating unused labels in genmatch. | expand |
On Wed, Mar 16, 2022 at 1:31 PM Roger Sayle <roger@nextmovesoftware.com> wrote: > > > This patch is the second of two changes to genmatch that don't affect > the executable code, but reduce the amount of debugging information > generated in stage3 of a build, but adhering more closely to GNU style > guidelines. > > This patch avoids generating "next_after_fail1:;" label statements > in genmatch, if this label is unused/never referenced as the target > of a goto. Because jumps to these labels are always forwards, we > know at the point the label is emitted whether it is used or not. > Because a debugger may set a breakpoint these labels, this increase > the size of g{imple,eneric}-match.o in a stage3 build. To save a > little extra space, I also shorten the label to "Lfail1:;" and only > increment the counter when necessary. This reduces the size of > gimple-match.o by 58K on x86_64-pc-linux-gnu. > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap > and make -k check with no new failures. Ok for mainline? I wonder if we could make this nicer by abstracting sth like class fail_label { fail_label (unsigned num); emit_goto () { fprintf (..., "goto L%u", num); used = true; } emit_label () { fprintf ("L%u:;\n", num); } unsigned num; bool used; }; or some better abstraction capturing all of the global state. > > > 2022-03-16 Roger Sayle <roger@nextmovesoftware.com> > > gcc/ChangeLog > * gcc/genmatch.cc (fail_label_used): New global variable. > (expr::gen_transform): Set fail_label_used whenever a goto > FAIL_LABEL is generated. > (dt_simplify::gen_1): Clear fail_label_used when generating > a new (provisional) fail_label. Set fail_label used whenever > a goto fail_label is generated. Avoid emitting fail_label: > if fail_label_used is false, instead decrement fail_label_cnt. > > > Thanks in advance, > Roger > -- >
On Wed, Mar 16, 2022 at 1:59 PM Richard Biener <richard.guenther@gmail.com> wrote: > > On Wed, Mar 16, 2022 at 1:31 PM Roger Sayle <roger@nextmovesoftware.com> wrote: > > > > > > This patch is the second of two changes to genmatch that don't affect > > the executable code, but reduce the amount of debugging information > > generated in stage3 of a build, but adhering more closely to GNU style > > guidelines. > > > > This patch avoids generating "next_after_fail1:;" label statements > > in genmatch, if this label is unused/never referenced as the target > > of a goto. Because jumps to these labels are always forwards, we > > know at the point the label is emitted whether it is used or not. > > Because a debugger may set a breakpoint these labels, this increase > > the size of g{imple,eneric}-match.o in a stage3 build. To save a > > little extra space, I also shorten the label to "Lfail1:;" and only > > increment the counter when necessary. This reduces the size of > > gimple-match.o by 58K on x86_64-pc-linux-gnu. > > > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap > > and make -k check with no new failures. Ok for mainline? > > I wonder if we could make this nicer by abstracting sth like > > class fail_label > { > fail_label (unsigned num); > emit_goto () { fprintf (..., "goto L%u", num); used = true; } > emit_label () { fprintf ("L%u:;\n", num); } > unsigned num; > bool used; > }; > > or some better abstraction capturing all of the global state. Btw, the unused labels were one reason for having gimple-match.o-warn = -Wno-unused generic-match.o-warn = -Wno-unused maybe that's then no longer needed. > > > > > > 2022-03-16 Roger Sayle <roger@nextmovesoftware.com> > > > > gcc/ChangeLog > > * gcc/genmatch.cc (fail_label_used): New global variable. > > (expr::gen_transform): Set fail_label_used whenever a goto > > FAIL_LABEL is generated. > > (dt_simplify::gen_1): Clear fail_label_used when generating > > a new (provisional) fail_label. Set fail_label used whenever > > a goto fail_label is generated. Avoid emitting fail_label: > > if fail_label_used is false, instead decrement fail_label_cnt. > > > > > > Thanks in advance, > > Roger > > -- > >
diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc index 2eda730..4a61f4e 100644 --- a/gcc/genmatch.cc +++ b/gcc/genmatch.cc @@ -2356,6 +2356,8 @@ capture_info::walk_c_expr (c_expr *e) /* The current label failing the current matched pattern during code generation. */ static char *fail_label; +/* Record that a reference/goto to the above label been generated. */ +static bool fail_label_used; /* Code generation off the decision tree and the refered AST nodes. */ @@ -2533,6 +2535,7 @@ expr::gen_transform (FILE *f, int indent, const char *dest, bool gimple, fprintf_indent (f, indent, "if (!_r%d) goto %s;\n", depth, fail_label); + fail_label_used = true; if (*opr == CONVERT_EXPR) { indent -= 4; @@ -2560,13 +2563,15 @@ expr::gen_transform (FILE *f, int indent, const char *dest, bool gimple, fprintf (f, ");\n"); if (opr->kind != id_base::CODE) { - fprintf_indent (f, indent, "if (!_r%d)\n", depth); - fprintf_indent (f, indent, " goto %s;\n", fail_label); + fprintf_indent (f, indent, "if (!_r%d) goto %s;\n", + depth, fail_label); + fail_label_used = true; } if (force_leaf) { fprintf_indent (f, indent, "if (EXPR_P (_r%d))\n", depth); fprintf_indent (f, indent, " goto %s;\n", fail_label); + fail_label_used = true; } if (*opr == CONVERT_EXPR) { @@ -3285,8 +3290,9 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result) static unsigned fail_label_cnt; char local_fail_label[256]; - snprintf (local_fail_label, 256, "next_after_fail%u", ++fail_label_cnt); + snprintf (local_fail_label, 256, "Lfail%u", ++fail_label_cnt); fail_label = local_fail_label; + fail_label_used = false; /* Analyze captures and perform early-outs on the incoming arguments that cover cases we cannot handle. */ @@ -3301,6 +3307,7 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result) fprintf_indent (f, indent, "if (TREE_SIDE_EFFECTS (_p%d)) goto %s;\n", i, fail_label); + fail_label_used = true; if (verbose >= 1) warning_at (as_a <expr *> (s->match)->ops[i]->location, "forcing toplevel operand to have no " @@ -3316,6 +3323,7 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result) fprintf_indent (f, indent, "if (TREE_SIDE_EFFECTS (captures[%d])) " "goto %s;\n", i, fail_label); + fail_label_used = true; if (verbose >= 1) warning_at (cinfo.info[i].c->location, "forcing captured operand to have no " @@ -3358,7 +3366,12 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result) } if (s->kind == simplify::SIMPLIFY) - fprintf_indent (f, indent, "if (__builtin_expect (!dbg_cnt (match), 0)) goto %s;\n", fail_label); + { + fprintf_indent (f, indent, + "if (__builtin_expect (!dbg_cnt (match), 0)) goto %s;\n", + fail_label); + fail_label_used = true; + } fprintf_indent (f, indent, "if (__builtin_expect (dump_file && (dump_flags & TDF_FOLDING), 0)) " "fprintf (dump_file, \"%s ", @@ -3430,9 +3443,12 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result) fprintf_indent (f, indent, "res_op->resimplify (lseq, valueize);\n"); if (e->force_leaf) - fprintf_indent (f, indent, - "if (!maybe_push_res_to_seq (res_op, NULL)) " - "goto %s;\n", fail_label); + { + fprintf_indent (f, indent, + "if (!maybe_push_res_to_seq (res_op, NULL))" + " goto %s;\n", fail_label); + fail_label_used = true; + } } } else if (result->type == operand::OP_CAPTURE @@ -3488,9 +3504,12 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result) continue; if (cinfo.info[i].result_use_count > cinfo.info[i].match_use_count) - fprintf_indent (f, indent, - "if (! tree_invariant_p (captures[%d])) " - "goto %s;\n", i, fail_label); + { + fprintf_indent (f, indent, + "if (! tree_invariant_p (captures[%d]))" + " goto %s;\n", i, fail_label); + fail_label_used = true; + } } for (unsigned j = 0; j < e->ops.length (); ++j) { @@ -3539,8 +3558,9 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result) fprintf (f, ");\n"); if (!is_a <operator_id *> (opr)) { - fprintf_indent (f, indent, "if (!_r)\n"); - fprintf_indent (f, indent, " goto %s;\n", fail_label); + fprintf_indent (f, indent, "if (!_r) goto %s;\n", + fail_label); + fail_label_used = true; } } } @@ -3581,7 +3601,10 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result) } indent -= 2; fprintf_indent (f, indent, "}\n"); - fprintf (f, "%s:;\n", fail_label); + if (fail_label_used) + fprintf (f, "%s:;\n", fail_label); + else + fail_label_cnt--; fail_label = NULL; }
This patch is the second of two changes to genmatch that don't affect the executable code, but reduce the amount of debugging information generated in stage3 of a build, but adhering more closely to GNU style guidelines. This patch avoids generating "next_after_fail1:;" label statements in genmatch, if this label is unused/never referenced as the target of a goto. Because jumps to these labels are always forwards, we know at the point the label is emitted whether it is used or not. Because a debugger may set a breakpoint these labels, this increase the size of g{imple,eneric}-match.o in a stage3 build. To save a little extra space, I also shorten the label to "Lfail1:;" and only increment the counter when necessary. This reduces the size of gimple-match.o by 58K on x86_64-pc-linux-gnu. This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and make -k check with no new failures. Ok for mainline? 2022-03-16 Roger Sayle <roger@nextmovesoftware.com> gcc/ChangeLog * gcc/genmatch.cc (fail_label_used): New global variable. (expr::gen_transform): Set fail_label_used whenever a goto FAIL_LABEL is generated. (dt_simplify::gen_1): Clear fail_label_used when generating a new (provisional) fail_label. Set fail_label used whenever a goto fail_label is generated. Avoid emitting fail_label: if fail_label_used is false, instead decrement fail_label_cnt. Thanks in advance, Roger --