diff mbox series

[1/3] RFC match.pd: don't emit label if not needed

Message ID patch-17157-tamar@arm.com
State New
Headers show
Series [1/3] RFC match.pd: don't emit label if not needed | expand

Commit Message

Tamar Christina April 6, 2023, 10:56 a.m. UTC 
Hi All,

Just sending these so people can test the series.

This is a small QoL codegen improvement for match.pd to not emit labels when
they are not needed.  The codegen is nice and there is a small (but consistent)
improvement in compile time.

Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.

Ok for GCC 14?

Thanks,
Tamar

gcc/ChangeLog:

	PR bootstrap/84402
	* genmatch.cc (dt_simplify::gen_1): Only emit labels if used.

--- inline copy of patch -- 
diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc
index 4fab4135347c43d95546a7df0bb1c4d074937288..638606b2502f640e59527fc5a0b23fa3bedd0cee 100644




--
diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc
index 4fab4135347c43d95546a7df0bb1c4d074937288..638606b2502f640e59527fc5a0b23fa3bedd0cee 100644
--- a/gcc/genmatch.cc
+++ b/gcc/genmatch.cc
@@ -3352,6 +3352,7 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result)
   char local_fail_label[256];
   snprintf (local_fail_label, 256, "next_after_fail%u", ++fail_label_cnt);
   fail_label = local_fail_label;
+  bool needs_label = false;
 
   /* Analyze captures and perform early-outs on the incoming arguments
      that cover cases we cannot handle.  */
@@ -3366,6 +3367,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);
+		needs_label = true;
 		if (verbose >= 1)
 		  warning_at (as_a <expr *> (s->match)->ops[i]->location,
 			      "forcing toplevel operand to have no "
@@ -3381,6 +3383,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);
+		needs_label = true;
 		if (verbose >= 1)
 		  warning_at (cinfo.info[i].c->location,
 			      "forcing captured operand to have no "
@@ -3423,7 +3426,10 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result)
     }
 
   if (s->kind == simplify::SIMPLIFY)
-    fprintf_indent (f, indent, "if (UNLIKELY (!dbg_cnt (match))) goto %s;\n", fail_label);
+    {
+      fprintf_indent (f, indent, "if (UNLIKELY (!dbg_cnt (match))) goto %s;\n", fail_label);
+      needs_label = true;
+    }
 
   fprintf_indent (f, indent, "if (UNLIKELY (dump_file && (dump_flags & TDF_FOLDING))) "
 	   "fprintf (dump_file, \"%s ",
@@ -3496,9 +3502,12 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result)
 			      "res_op->resimplify (%s, valueize);\n",
 			      !e->force_leaf ? "lseq" : "NULL");
 	      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);
+		  needs_label = true;
+		}
 	    }
 	}
       else if (result->type == operand::OP_CAPTURE
@@ -3554,9 +3563,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);
+		    needs_label = true;
+		  }
 	      }
 	  for (unsigned j = 0; j < e->ops.length (); ++j)
 	    {
@@ -3607,6 +3619,7 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result)
 		    {
 		      fprintf_indent (f, indent, "if (!_r)\n");
 		      fprintf_indent (f, indent, "  goto %s;\n", fail_label);
+		      needs_label = true;
 		    }
 		}
 	    }
@@ -3647,7 +3660,8 @@ 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 (needs_label)
+    fprintf (f, "%s:;\n", fail_label);
   fail_label = NULL;
 }

Comments

Richard Biener April 18, 2023, 10:38 a.m. UTC | #1 
On Tue, Apr 18, 2023 at 12:21 PM Tamar Christina via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi All,
>
> This is a small QoL codegen improvement for match.pd to not emit labels when
> they are not needed.  The codegen is nice and there is a small (but consistent)
> improvement in compile time.
>
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>
> Ok for master?

OK.  Btw - how many labels does this remove? (just wc -l the generated files?)

Richard.

> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
>         PR bootstrap/84402
>         * genmatch.cc (dt_simplify::gen_1): Only emit labels if used.
>
> --- inline copy of patch --
> diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc
> index 4fab4135347c43d95546a7df0bb1c4d074937288..638606b2502f640e59527fc5a0b23fa3bedd0cee 100644
> --- a/gcc/genmatch.cc
> +++ b/gcc/genmatch.cc
> @@ -3352,6 +3352,7 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result)
>    char local_fail_label[256];
>    snprintf (local_fail_label, 256, "next_after_fail%u", ++fail_label_cnt);
>    fail_label = local_fail_label;
> +  bool needs_label = false;
>
>    /* Analyze captures and perform early-outs on the incoming arguments
>       that cover cases we cannot handle.  */
> @@ -3366,6 +3367,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);
> +               needs_label = true;
>                 if (verbose >= 1)
>                   warning_at (as_a <expr *> (s->match)->ops[i]->location,
>                               "forcing toplevel operand to have no "
> @@ -3381,6 +3383,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);
> +               needs_label = true;
>                 if (verbose >= 1)
>                   warning_at (cinfo.info[i].c->location,
>                               "forcing captured operand to have no "
> @@ -3423,7 +3426,10 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result)
>      }
>
>    if (s->kind == simplify::SIMPLIFY)
> -    fprintf_indent (f, indent, "if (UNLIKELY (!dbg_cnt (match))) goto %s;\n", fail_label);
> +    {
> +      fprintf_indent (f, indent, "if (UNLIKELY (!dbg_cnt (match))) goto %s;\n", fail_label);
> +      needs_label = true;
> +    }
>
>    fprintf_indent (f, indent, "if (UNLIKELY (dump_file && (dump_flags & TDF_FOLDING))) "
>            "fprintf (dump_file, \"%s ",
> @@ -3496,9 +3502,12 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result)
>                               "res_op->resimplify (%s, valueize);\n",
>                               !e->force_leaf ? "lseq" : "NULL");
>               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);
> +                 needs_label = true;
> +               }
>             }
>         }
>        else if (result->type == operand::OP_CAPTURE
> @@ -3554,9 +3563,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);
> +                   needs_label = true;
> +                 }
>               }
>           for (unsigned j = 0; j < e->ops.length (); ++j)
>             {
> @@ -3607,6 +3619,7 @@ dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result)
>                     {
>                       fprintf_indent (f, indent, "if (!_r)\n");
>                       fprintf_indent (f, indent, "  goto %s;\n", fail_label);
> +                     needs_label = true;
>                     }
>                 }
>             }
> @@ -3647,7 +3660,8 @@ 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 (needs_label)
> +    fprintf (f, "%s:;\n", fail_label);
>    fail_label = NULL;
>  }
>
>
>
>
>
> --
Tamar Christina April 18, 2023, 4:46 p.m. UTC | #2 
> -----Original Message-----
> From: Richard Biener <richard.guenther@gmail.com>
> Sent: Tuesday, April 18, 2023 11:38 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 1/3]middle-end match.pd: don't emit label if not needed
> 
> On Tue, Apr 18, 2023 at 12:21 PM Tamar Christina via Gcc-patches <gcc-
> patches@gcc.gnu.org> wrote:
> >
> > Hi All,
> >
> > This is a small QoL codegen improvement for match.pd to not emit
> > labels when they are not needed.  The codegen is nice and there is a
> > small (but consistent) improvement in compile time.
> >
> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> >
> > Ok for master?
> 
> OK.  Btw - how many labels does this remove? (just wc -l the generated files?)

Not terribly much anymore, it's about 160 lines.  Though when benchmarking it
shows a consistent 2-5% speedup in compile time (I take the geomean of about 100 compiles).

Regards,
Tamar

> 
> Richard.
> 
> > Thanks,
> > Tamar
> >
> > gcc/ChangeLog:
> >
> >         PR bootstrap/84402
> >         * genmatch.cc (dt_simplify::gen_1): Only emit labels if used.
> >
> > --- inline copy of patch --
> > diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc index
> >
> 4fab4135347c43d95546a7df0bb1c4d074937288..638606b2502f640e595
> 27fc5a0b2
> > 3fa3bedd0cee 100644
> > --- a/gcc/genmatch.cc
> > +++ b/gcc/genmatch.cc
> > @@ -3352,6 +3352,7 @@ dt_simplify::gen_1 (FILE *f, int indent, bool
> gimple, operand *result)
> >    char local_fail_label[256];
> >    snprintf (local_fail_label, 256, "next_after_fail%u", ++fail_label_cnt);
> >    fail_label = local_fail_label;
> > +  bool needs_label = false;
> >
> >    /* Analyze captures and perform early-outs on the incoming arguments
> >       that cover cases we cannot handle.  */ @@ -3366,6 +3367,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);
> > +               needs_label = true;
> >                 if (verbose >= 1)
> >                   warning_at (as_a <expr *> (s->match)->ops[i]->location,
> >                               "forcing toplevel operand to have no "
> > @@ -3381,6 +3383,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);
> > +               needs_label = true;
> >                 if (verbose >= 1)
> >                   warning_at (cinfo.info[i].c->location,
> >                               "forcing captured operand to have no "
> > @@ -3423,7 +3426,10 @@ dt_simplify::gen_1 (FILE *f, int indent, bool
> gimple, operand *result)
> >      }
> >
> >    if (s->kind == simplify::SIMPLIFY)
> > -    fprintf_indent (f, indent, "if (UNLIKELY (!dbg_cnt (match))) goto %s;\n",
> fail_label);
> > +    {
> > +      fprintf_indent (f, indent, "if (UNLIKELY (!dbg_cnt (match))) goto %s;\n",
> fail_label);
> > +      needs_label = true;
> > +    }
> >
> >    fprintf_indent (f, indent, "if (UNLIKELY (dump_file && (dump_flags &
> TDF_FOLDING))) "
> >            "fprintf (dump_file, \"%s ", @@ -3496,9 +3502,12 @@
> > dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result)
> >                               "res_op->resimplify (%s, valueize);\n",
> >                               !e->force_leaf ? "lseq" : "NULL");
> >               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);
> > +                 needs_label = true;
> > +               }
> >             }
> >         }
> >        else if (result->type == operand::OP_CAPTURE @@ -3554,9
> > +3563,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);
> > +                   needs_label = true;
> > +                 }
> >               }
> >           for (unsigned j = 0; j < e->ops.length (); ++j)
> >             {
> > @@ -3607,6 +3619,7 @@ dt_simplify::gen_1 (FILE *f, int indent, bool
> gimple, operand *result)
> >                     {
> >                       fprintf_indent (f, indent, "if (!_r)\n");
> >                       fprintf_indent (f, indent, "  goto %s;\n",
> > fail_label);
> > +                     needs_label = true;
> >                     }
> >                 }
> >             }
> > @@ -3647,7 +3660,8 @@ 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 (needs_label)
> > +    fprintf (f, "%s:;\n", fail_label);
> >    fail_label = NULL;
> >  }
> >
> >
> >
> >
> >
> > --
diff mbox series

Patch

--- a/gcc/genmatch.cc
+++ b/gcc/genmatch.cc
@@ -3352,6 +3352,7 @@  dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result)
   char local_fail_label[256];
   snprintf (local_fail_label, 256, "next_after_fail%u", ++fail_label_cnt);
   fail_label = local_fail_label;
+  bool needs_label = false;
 
   /* Analyze captures and perform early-outs on the incoming arguments
      that cover cases we cannot handle.  */
@@ -3366,6 +3367,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);
+		needs_label = true;
 		if (verbose >= 1)
 		  warning_at (as_a <expr *> (s->match)->ops[i]->location,
 			      "forcing toplevel operand to have no "
@@ -3381,6 +3383,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);
+		needs_label = true;
 		if (verbose >= 1)
 		  warning_at (cinfo.info[i].c->location,
 			      "forcing captured operand to have no "
@@ -3423,7 +3426,10 @@  dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result)
     }
 
   if (s->kind == simplify::SIMPLIFY)
-    fprintf_indent (f, indent, "if (UNLIKELY (!dbg_cnt (match))) goto %s;\n", fail_label);
+    {
+      fprintf_indent (f, indent, "if (UNLIKELY (!dbg_cnt (match))) goto %s;\n", fail_label);
+      needs_label = true;
+    }
 
   fprintf_indent (f, indent, "if (UNLIKELY (dump_file && (dump_flags & TDF_FOLDING))) "
 	   "fprintf (dump_file, \"%s ",
@@ -3496,9 +3502,12 @@  dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result)
 			      "res_op->resimplify (%s, valueize);\n",
 			      !e->force_leaf ? "lseq" : "NULL");
 	      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);
+		  needs_label = true;
+		}
 	    }
 	}
       else if (result->type == operand::OP_CAPTURE
@@ -3554,9 +3563,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);
+		    needs_label = true;
+		  }
 	      }
 	  for (unsigned j = 0; j < e->ops.length (); ++j)
 	    {
@@ -3607,6 +3619,7 @@  dt_simplify::gen_1 (FILE *f, int indent, bool gimple, operand *result)
 		    {
 		      fprintf_indent (f, indent, "if (!_r)\n");
 		      fprintf_indent (f, indent, "  goto %s;\n", fail_label);
+		      needs_label = true;
 		    }
 		}
 	    }
@@ -3647,7 +3660,8 @@  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 (needs_label)
+    fprintf (f, "%s:;\n", fail_label);
   fail_label = NULL;
 }