diff mbox

Allowing filename-style globs in define_bypass

Message ID g439hzvpno.fsf@linaro.org
State New
Headers show

Commit Message

Richard Sandiford July 21, 2011, 11:28 a.m. UTC
define_bypass requires two lists of reservation names.  On targets with
fairly regular scheduling descriptions, it'd be nice to be able to use
filename-style globs to match several reservation names at once.

libiberty already provides an appropriate routine: fnmatch.  It also has
a fallback definition of strpbrk, which we can use to check whether a
string is a glob, and keep the current fast path if not.

The immediate motivation for this is a patch to model address writeback
in the ARM Cortex A8 and A9 schedulers.

Tested on x86_64-linux-gnu and arm-linux-gnueabi.  It doesn't change the
insn-attrtab.c or insn-automata.c output, and there was no measurable
slowdown in the generators.  OK to install?

Richard


gcc/
	* doc/md.texi (define_bypass): Say that the instruction names can
	be filename-style globs.
	* Makefile.in (FNMATCH_H): Define.
	(build/genattrtab.o, build/genautomata.o): Depend on $(FNMATCH_H).
	* genattrtab.c: Include fnmatch.h.
	(bypass_list): Change field name from "insn" to "pattern".
	(gen_bypass_1): Update accordingly.
	(process_bypasses): Use fnmatch to check for matches between
	insn reservations and define_bypasses.
	* genautomata.c: Include fnmatch.h.
	(bypass_decl): Rename in_insn_name and out_insn_name to in_pattern
	and out_pattern respectively.
	(gen_bypass, insert_bypass): Update accordingly.
	(for_each_matching_insn, process_bypass_2, process_bypass_1)
	(process_bypass): New functions.
	(process_decls): Use process_bypass.  Update after field name changes.

Comments

Bernd Schmidt July 21, 2011, 4:17 p.m. UTC | #1
On 07/21/11 13:28, Richard Sandiford wrote:

> +static void
> +process_bypass_2 (decl_t model, decl_t out_insn_reserv, void *data)
> +{
> +  struct bypass_decl *bypass;
> +  decl_t in_insn_reserv;
> +
> +  in_insn_reserv = (decl_t) data;
> +  bypass = XCNEW (struct bypass_decl);
> +  bypass->latency = DECL_BYPASS (model)->latency;
> +  bypass->out_pattern = DECL_INSN_RESERV (out_insn_reserv)->name;
> +  bypass->in_pattern = DECL_INSN_RESERV (in_insn_reserv)->name;
> +  bypass->bypass_guard_name = DECL_BYPASS (model)->bypass_guard_name;
> +  bypass->out_insn_reserv = DECL_INSN_RESERV (out_insn_reserv);
> +  bypass->in_insn_reserv = DECL_INSN_RESERV (in_insn_reserv);
> +  insert_bypass (bypass);

Doesn't this create a duplicate bypass structure in the case where
you're not using a wildcard? Could check first whether out_pattern and
in_pattern of the existing decl_t match.

Otherwise OK I think.


Bernd
diff mbox

Patch

Index: gcc/doc/md.texi
===================================================================
--- gcc/doc/md.texi	2011-07-21 11:13:02.000000000 +0100
+++ gcc/doc/md.texi	2011-07-21 11:19:30.000000000 +0100
@@ -7783,8 +7783,16 @@  latency time for given instruction pair.
 
 @var{number} defines when the result generated by the instructions
 given in string @var{out_insn_names} will be ready for the
-instructions given in string @var{in_insn_names}.  The instructions in
-the string are separated by commas.
+instructions given in string @var{in_insn_names}.  Each of these
+strings is a comma-separated list of filename-style globs and
+they refer to the names of @code{define_insn_reservation}s.
+For example:
+@smallexample
+(define_bypass 1 "cpu1_load_*, cpu1_store_*" "cpu1_load_*")
+@end smallexample
+defines a bypass between instructions that start with
+@samp{cpu1_load_} or @samp{cpu1_store_} and those that start with
+@samp{cpu1_load_}.
 
 @var{guard} is an optional string giving the name of a C function which
 defines an additional guard for the bypass.  The function will get the
Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in	2011-07-21 11:13:02.000000000 +0100
+++ gcc/Makefile.in	2011-07-21 11:19:30.000000000 +0100
@@ -445,6 +445,7 @@  PARTITION_H = $(srcdir)/../include/parti
 MD5_H	    = $(srcdir)/../include/md5.h
 DWARF2_H    = $(srcdir)/../include/dwarf2.h
 XREGEX_H    =  $(srcdir)/../include/xregex.h
+FNMATCH_H   = $(srcdir)/../include/fnmatch.h
 
 # Linker plugin API headers
 LINKER_PLUGIN_API_H = $(srcdir)/../include/plugin-api.h
@@ -3926,10 +3927,10 @@  build/genattr-common.o : genattr-common.
   $(SYSTEM_H) coretypes.h $(GTM_H) errors.h $(READ_MD_H) gensupport.h
 build/genattrtab.o : genattrtab.c $(RTL_BASE_H) $(OBSTACK_H)		\
   $(BCONFIG_H) $(SYSTEM_H) coretypes.h $(GTM_H) errors.h $(GGC_H)	\
-  $(READ_MD_H) gensupport.h vecprim.h
+  $(READ_MD_H) gensupport.h vecprim.h $(FNMATCH_H)
 build/genautomata.o : genautomata.c $(RTL_BASE_H) $(OBSTACK_H)		\
   $(BCONFIG_H) $(SYSTEM_H) coretypes.h $(GTM_H) errors.h $(VEC_H)	\
-  $(HASHTAB_H) gensupport.h
+  $(HASHTAB_H) gensupport.h $(FNMATCH_H)
 build/gencheck.o : gencheck.c all-tree.def $(BCONFIG_H) $(GTM_H)	\
 	$(SYSTEM_H) coretypes.h $(lang_tree_files) gimple.def
 build/genchecksum.o : genchecksum.c $(BCONFIG_H) $(SYSTEM_H) $(MD5_H)
Index: gcc/genattrtab.c
===================================================================
--- gcc/genattrtab.c	2011-07-21 11:13:02.000000000 +0100
+++ gcc/genattrtab.c	2011-07-21 11:19:30.000000000 +0100
@@ -114,6 +114,7 @@  #define strcmp_check(S1, S2) ((S1) != (S
 #include "read-md.h"
 #include "gensupport.h"
 #include "vecprim.h"
+#include "fnmatch.h"
 
 /* Flags for make_internal_attr's `special' parameter.  */
 #define ATTR_NONE		0
@@ -4553,7 +4554,7 @@  gen_insn_reserv (rtx def)
 struct bypass_list
 {
   struct bypass_list *next;
-  const char *insn;
+  const char *pattern;
 };
 
 static struct bypass_list *all_bypasses;
@@ -4569,11 +4570,11 @@  gen_bypass_1 (const char *s, size_t len)
 
   s = attr_string (s, len);
   for (b = all_bypasses; b; b = b->next)
-    if (s == b->insn)
+    if (s == b->pattern)
       return;  /* already got that one */
 
   b = oballoc (struct bypass_list);
-  b->insn = s;
+  b->pattern = s;
   b->next = all_bypasses;
   all_bypasses = b;
   n_bypasses++;
@@ -4607,7 +4608,7 @@  process_bypasses (void)
      list.  */
   for (r = all_insn_reservs; r; r = r->next)
     for (b = all_bypasses; b; b = b->next)
-      if (r->name == b->insn)
+      if (fnmatch (b->pattern, r->name, 0) == 0)
 	r->bypassed = true;
 }
 
Index: gcc/genautomata.c
===================================================================
--- gcc/genautomata.c	2011-07-21 11:13:02.000000000 +0100
+++ gcc/genautomata.c	2011-07-21 11:19:30.000000000 +0100
@@ -117,6 +117,7 @@  Free Software Foundation; either version
 #include <math.h>
 #include "hashtab.h"
 #include "vec.h"
+#include "fnmatch.h"
 
 #ifndef CHAR_BIT
 #define CHAR_BIT 8
@@ -384,8 +385,8 @@  struct unit_decl
 struct bypass_decl
 {
   int latency;
-  const char *out_insn_name;
-  const char *in_insn_name;
+  const char *out_pattern;
+  const char *in_pattern;
   const char *bypass_guard_name;
 
   /* The following fields are defined by checker.  */
@@ -1306,17 +1307,17 @@  gen_query_cpu_unit (rtx def)
 gen_bypass (rtx def)
 {
   decl_t decl;
-  char **out_insns;
+  char **out_patterns;
   int out_length;
-  char **in_insns;
+  char **in_patterns;
   int in_length;
   int i, j;
 
-  out_insns = get_str_vect (XSTR (def, 1), &out_length, ',', FALSE);
-  if (out_insns == NULL)
+  out_patterns = get_str_vect (XSTR (def, 1), &out_length, ',', FALSE);
+  if (out_patterns == NULL)
     fatal ("invalid string `%s' in define_bypass", XSTR (def, 1));
-  in_insns = get_str_vect (XSTR (def, 2), &in_length, ',', FALSE);
-  if (in_insns == NULL)
+  in_patterns = get_str_vect (XSTR (def, 2), &in_length, ',', FALSE);
+  if (in_patterns == NULL)
     fatal ("invalid string `%s' in define_bypass", XSTR (def, 2));
   for (i = 0; i < out_length; i++)
     for (j = 0; j < in_length; j++)
@@ -1325,8 +1326,8 @@  gen_bypass (rtx def)
 	decl->mode = dm_bypass;
 	decl->pos = 0;
 	DECL_BYPASS (decl)->latency = XINT (def, 0);
-	DECL_BYPASS (decl)->out_insn_name = out_insns [i];
-	DECL_BYPASS (decl)->in_insn_name = in_insns [j];
+	DECL_BYPASS (decl)->out_pattern = out_patterns[i];
+	DECL_BYPASS (decl)->in_pattern = in_patterns[j];
 	DECL_BYPASS (decl)->bypass_guard_name = XSTR (def, 3);
 	VEC_safe_push (decl_t, heap, decls, decl);
       }
@@ -2397,19 +2398,19 @@  insert_bypass (struct bypass_decl *bypas
 	      {
 		if (!w_flag)
 		  error ("the same bypass `%s - %s' is already defined",
-			 bypass->out_insn_name, bypass->in_insn_name);
+			 bypass->out_pattern, bypass->in_pattern);
 		else
 		  warning ("the same bypass `%s - %s' is already defined",
-			   bypass->out_insn_name, bypass->in_insn_name);
+			   bypass->out_pattern, bypass->in_pattern);
 	      }
 	    else if (!w_flag)
 	      error ("the same bypass `%s - %s' (guard %s) is already defined",
-		     bypass->out_insn_name, bypass->in_insn_name,
+		     bypass->out_pattern, bypass->in_pattern,
 		     bypass->bypass_guard_name);
 	    else
 	      warning
 		("the same bypass `%s - %s' (guard %s) is already defined",
-		 bypass->out_insn_name, bypass->in_insn_name,
+		 bypass->out_pattern, bypass->in_pattern,
 		 bypass->bypass_guard_name);
 	    return;
 	  }
@@ -2434,6 +2435,84 @@  insert_bypass (struct bypass_decl *bypas
     }
 }
 
+/* BYPASS is a define_bypass decl that includes glob pattern PATTERN.
+   Call FN (BYPASS, INSN, DATA) for each matching instruction INSN.  */
+
+static void
+for_each_matching_insn (decl_t bypass, const char *pattern,
+			void (*fn) (decl_t, decl_t, void *), void *data)
+{
+  decl_t insn_reserv;
+  bool matched_p;
+  int i;
+
+  matched_p = false;
+  if (strpbrk (pattern, "*?["))
+    for (i = 0; i < description->decls_num; i++)
+      {
+	insn_reserv = description->decls[i];
+	if (insn_reserv->mode == dm_insn_reserv
+	    && fnmatch (pattern, DECL_INSN_RESERV (insn_reserv)->name, 0) == 0)
+	  {
+	    fn (bypass, insn_reserv, data);
+	    matched_p = true;
+	  }
+      }
+  else
+    {
+      insn_reserv = find_insn_decl (pattern);
+      if (insn_reserv)
+	{
+	  fn (bypass, insn_reserv, data);
+	  matched_p = true;
+	}
+    }
+  if (!matched_p)
+    error ("there is no insn reservation that matches `%s'", pattern);
+}
+
+/* A subroutine of process_bypass that is called for each pair
+   of matching instructions.  OUT_INSN_RESERV is the output
+   instruction and DATA is the input instruction.  */
+
+static void
+process_bypass_2 (decl_t model, decl_t out_insn_reserv, void *data)
+{
+  struct bypass_decl *bypass;
+  decl_t in_insn_reserv;
+
+  in_insn_reserv = (decl_t) data;
+  bypass = XCNEW (struct bypass_decl);
+  bypass->latency = DECL_BYPASS (model)->latency;
+  bypass->out_pattern = DECL_INSN_RESERV (out_insn_reserv)->name;
+  bypass->in_pattern = DECL_INSN_RESERV (in_insn_reserv)->name;
+  bypass->bypass_guard_name = DECL_BYPASS (model)->bypass_guard_name;
+  bypass->out_insn_reserv = DECL_INSN_RESERV (out_insn_reserv);
+  bypass->in_insn_reserv = DECL_INSN_RESERV (in_insn_reserv);
+  insert_bypass (bypass);
+}
+
+/* A subroutine of process_bypass that is called for each input
+   instruction IN_INSN_RESERV.  */
+
+static void
+process_bypass_1 (decl_t bypass, decl_t in_insn_reserv,
+		  void *data ATTRIBUTE_UNUSED)
+{
+  for_each_matching_insn (bypass, DECL_BYPASS (bypass)->out_pattern,
+			  process_bypass_2, in_insn_reserv);
+}
+
+/* Process define_bypass decl BYPASS, inserting a bypass for each specific
+   pair of insn reservations.  */
+
+static void
+process_bypass (decl_t bypass)
+{
+  for_each_matching_insn (bypass, DECL_BYPASS (bypass)->in_pattern,
+			  process_bypass_1, NULL);
+}
+
 /* The function processes pipeline description declarations, checks
    their correctness, and forms exclusion/presence/absence sets.  */
 static void
@@ -2442,8 +2521,6 @@  process_decls (void)
   decl_t decl;
   decl_t automaton_decl;
   decl_t decl_in_table;
-  decl_t out_insn_reserv;
-  decl_t in_insn_reserv;
   int automaton_presence;
   int i;
 
@@ -2489,8 +2566,8 @@  process_decls (void)
 	{
 	  if (DECL_BYPASS (decl)->latency < 0)
 	    error ("define_bypass `%s - %s' has negative latency time",
-		   DECL_BYPASS (decl)->out_insn_name,
-		   DECL_BYPASS (decl)->in_insn_name);
+		   DECL_BYPASS (decl)->out_pattern,
+		   DECL_BYPASS (decl)->in_pattern);
 	}
       else if (decl->mode == dm_unit || decl->mode == dm_reserv)
 	{
@@ -2551,24 +2628,7 @@  process_decls (void)
     {
       decl = description->decls [i];
       if (decl->mode == dm_bypass)
-	{
-	  out_insn_reserv = find_insn_decl (DECL_BYPASS (decl)->out_insn_name);
-	  in_insn_reserv = find_insn_decl (DECL_BYPASS (decl)->in_insn_name);
-	  if (out_insn_reserv == NULL)
-	    error ("there is no insn reservation `%s'",
-		   DECL_BYPASS (decl)->out_insn_name);
-	  else if (in_insn_reserv == NULL)
-	    error ("there is no insn reservation `%s'",
-		   DECL_BYPASS (decl)->in_insn_name);
-	  else
-	    {
-	      DECL_BYPASS (decl)->out_insn_reserv
-		= DECL_INSN_RESERV (out_insn_reserv);
-	      DECL_BYPASS (decl)->in_insn_reserv
-		= DECL_INSN_RESERV (in_insn_reserv);
-	      insert_bypass (DECL_BYPASS (decl));
-	    }
-	}
+	process_bypass (decl);
     }
 
   /* Check exclusion set declarations and form exclusion sets.  */
@@ -8757,8 +8817,8 @@  output_description (void)
       else if (decl->mode == dm_bypass)
 	fprintf (output_description_file, "bypass %d %s %s\n",
 		 DECL_BYPASS (decl)->latency,
-		 DECL_BYPASS (decl)->out_insn_name,
-		 DECL_BYPASS (decl)->in_insn_name);
+		 DECL_BYPASS (decl)->out_pattern,
+		 DECL_BYPASS (decl)->in_pattern);
     }
   fprintf (output_description_file, "\n\f\n");
 }