[PATCH] Generate an if instead of a switch with one case in genmatch.

[PATCH] Generate an if instead of a switch with one case in genmatch.

[Roger Sayle]

[-- Attachment #1: Type: text/plain, Size: 1035 bytes --]


This patch is the first 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 a switch with a single case statement,
instead preferring to use an "if (TREE_CODE (...) == SSA_NAME)" idiom.
These should compile to the same instructions, but the switch requires
more lines, especially when a debugger may set a break point on the
switch, the case, or the (obligatory) final "default:;".  This reduces
the size of gimple-match.o by 53K 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 (dt_node::gen_kids_1): Introduce use_switch
	logic that prefers to generate an if statement rather than a
	switch containing a single case (and a default).


Thanks in advance,
Roger
--


[-- Attachment #2: patchgm.txt --]
[-- Type: text/plain, Size: 3361 bytes --]

diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc
index 2eda730..c4e22ef 100644
--- a/gcc/genmatch.cc
+++ b/gcc/genmatch.cc
@@ -2999,6 +2999,7 @@ dt_node::gen_kids_1 (FILE *f, int indent, bool gimple, int depth,
   unsigned gexprs_len = generic_exprs.length ();
   unsigned fns_len = fns.length ();
   unsigned gfns_len = generic_fns.length ();
+  bool use_switch = true;
 
   if (exprs_len || fns_len || gexprs_len || gfns_len)
     {
@@ -3011,7 +3012,30 @@ dt_node::gen_kids_1 (FILE *f, int indent, bool gimple, int depth,
       else
 	generic_exprs[0]->get_name (kid_opname);
 
-      fprintf_indent (f, indent, "switch (TREE_CODE (%s))\n", kid_opname);
+      if (gexprs_len == 0 && gfns_len == 0)
+	{
+	  fprintf_indent (f, indent, "if (TREE_CODE (%s) == SSA_NAME)\n",
+			  kid_opname);
+	  use_switch = false;
+	}
+      else if (exprs_len == 0
+	       && fns_len == 0
+	       && gexprs_len == 1
+	       && gfns_len == 0)
+	{
+	  expr *e = as_a <expr *>(generic_exprs[0]->op);
+	  id_base *op = e->operation;
+	  /* id_base doesn't have a != operator.  */
+	  if (!(*op == CONVERT_EXPR || *op == NOP_EXPR))
+	    {
+	      fprintf_indent (f, indent, "if (TREE_CODE (%s) == %s)\n",
+			      kid_opname, op->id);
+	      use_switch = false;
+	    }
+	}
+
+      if (use_switch)
+	fprintf_indent (f, indent, "switch (TREE_CODE (%s))\n", kid_opname);
       fprintf_indent (f, indent, "  {\n");
       indent += 2;
     }
@@ -3019,8 +3043,8 @@ dt_node::gen_kids_1 (FILE *f, int indent, bool gimple, int depth,
   if (exprs_len || fns_len)
     {
       depth++;
-      fprintf_indent (f, indent,
-		      "case SSA_NAME:\n");
+      if (use_switch)
+	fprintf_indent (f, indent, "case SSA_NAME:\n");
       fprintf_indent (f, indent,
 		      "  if (gimple *_d%d = get_def (valueize, %s))\n",
 		      depth, kid_opname);
@@ -3103,7 +3127,8 @@ dt_node::gen_kids_1 (FILE *f, int indent, bool gimple, int depth,
 	    }
 	}
 
-      fprintf_indent (f, indent, "  break;\n");
+      if (use_switch)
+	fprintf_indent (f, indent, "  break;\n");
     }
 
   for (unsigned i = 0; i < generic_exprs.length (); ++i)
@@ -3115,12 +3140,18 @@ dt_node::gen_kids_1 (FILE *f, int indent, bool gimple, int depth,
       else if (*op == SSA_NAME && (exprs_len || fns_len))
 	/* Already handled above.  */
 	continue;
-      else
+      else if (use_switch)
 	fprintf_indent (f, indent, "case %s:\n", op->id);
-      fprintf_indent (f, indent, "  {\n");
-      generic_exprs[i]->gen (f, indent + 4, gimple, depth);
-      fprintf_indent (f, indent, "    break;\n");
-      fprintf_indent (f, indent, "  }\n");
+
+      if (use_switch)
+	{
+	  fprintf_indent (f, indent, "  {\n");
+	  generic_exprs[i]->gen (f, indent + 4, gimple, depth);
+	  fprintf_indent (f, indent, "    break;\n");
+	  fprintf_indent (f, indent, "  }\n");
+	}
+      else
+	generic_exprs[i]->gen (f, indent + 2, gimple, depth);
     }
 
   if (gfns_len)
@@ -3157,7 +3188,8 @@ dt_node::gen_kids_1 (FILE *f, int indent, bool gimple, int depth,
   if (exprs_len || fns_len || gexprs_len || gfns_len)
     {
       indent -= 4;
-      fprintf_indent (f, indent, "    default:;\n");
+      if (use_switch)
+	fprintf_indent (f, indent, "    default:;\n");
       fprintf_indent (f, indent, "    }\n");
     }
 

Re: [PATCH] Generate an if instead of a switch with one case in genmatch.

[Richard Biener]

On Wed, Mar 16, 2022 at 1:28 PM Roger Sayle <roger@nextmovesoftware.com> wrote:
>
>
> This patch is the first 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 a switch with a single case statement,
> instead preferring to use an "if (TREE_CODE (...) == SSA_NAME)" idiom.
> These should compile to the same instructions, but the switch requires
> more lines, especially when a debugger may set a break point on the
> switch, the case, or the (obligatory) final "default:;".  This reduces
> the size of gimple-match.o by 53K 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?

Hmm, this makes the complicated code emission in gen_kids_1 even more
complicated for a questionable gain which I'd rather not do, debuginfo
savings or not ...

Richard.

>
> 2022-03-16  Roger Sayle  <roger@nextmovesoftware.com>
>
> gcc/ChangeLog
>         * gcc/genmatch.cc (dt_node::gen_kids_1): Introduce use_switch
>         logic that prefers to generate an if statement rather than a
>         switch containing a single case (and a default).
>
>
> Thanks in advance,
> Roger
> --
>