[PATCH] Move gen_* stubs from defaults.h to genflags

[PATCH] Move gen_* stubs from defaults.h to genflags

[Mikhail Maltsev]

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

Hi, all.

I noticed that defaults.h file contains stub generator functions which
simply call gcc_unreachable. FWIW, Trevor added them to remove some
conditional compilation which depends on HAVE_<insn_name> macros (I mean
something like r223624).

Because we still have ~80 more such conditions in GCC, and probably some
of them will be fixed in the same way, I propose a patch, which allows
to generate required stubs in genflags.

Bootstrapped and regtested on x86_64-unknown-linux-gnu, testing for full
target list in progress. OK for trunk?

-- 
Regards,
    Mikhail Maltsev

[-- Attachment #2: gen_defaults.clog --]
[-- Type: text/plain, Size: 681 bytes --]

gcc/ChangeLog:

2015-06-10  Mikhail Maltsev  <maltsevm@gmail.com>

	* defaults.h (gen_simple_return, gen_return, gen_mem_thread_fence,
	gen_memory_barrier, gen_mem_signal_fence, gen_load_multiple,
	gen_store_multiple, gen_tablejump): Remove.  Generate by genflags.
	(HAVE_*): Likewise.
	* genflags.c (struct stub_info_t): Define.
	(stubs): Define variable.
	(struct gflg_string_hasher): Define.
	(stubs_map): Define variable.
	(mark_as_done): New function.
	(gen_dummy): New function (move code out of gen_proto, generalize).
	(gen_proto): Use gen_dummy.
	(gen_insn): Call mark_as_done.
	(gen_stub): New function.
	(main): Initialize stubs_map, call gen_stubs for missing stubs.



[-- Attachment #3: gen_defaults.patch --]
[-- Type: text/plain, Size: 7043 bytes --]

diff --git a/gcc/defaults.h b/gcc/defaults.h
index 057b646..5beddea 100644
--- a/gcc/defaults.h
+++ b/gcc/defaults.h
@@ -1426,96 +1426,6 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 #define TARGET_VTABLE_USES_DESCRIPTORS 0
 #endif
 
-#ifndef HAVE_simple_return
-#define HAVE_simple_return 0
-static inline rtx
-gen_simple_return ()
-{
-  gcc_unreachable ();
-  return NULL;
-}
-#endif
-
-#ifndef HAVE_return
-#define HAVE_return 0
-static inline rtx
-gen_return ()
-{
-  gcc_unreachable ();
-  return NULL;
-}
-#endif
-
-#ifndef HAVE_epilogue
-#define HAVE_epilogue 0
-static inline rtx
-gen_epilogue ()
-{
-  gcc_unreachable ();
-  return NULL;
-}
-#endif
-
-#ifndef HAVE_mem_thread_fence
-#define HAVE_mem_thread_fence 0
-static inline rtx
-gen_mem_thread_fence (rtx)
-{
-  gcc_unreachable ();
-  return NULL;
-}
-#endif
-
-#ifndef HAVE_memory_barrier
-#define HAVE_memory_barrier 0
-static inline rtx
-gen_memory_barrier ()
-{
-  gcc_unreachable ();
-  return NULL;
-}
-#endif
-
-#ifndef HAVE_mem_signal_fence
-#define HAVE_mem_signal_fence 0
-static inline rtx
-gen_mem_signal_fence (rtx)
-{
-  gcc_unreachable ();
-  return NULL;
-}
-#endif
-
-#ifndef HAVE_load_multiple
-#define HAVE_load_multiple 0
-static inline rtx
-gen_load_multiple (rtx, rtx, rtx)
-{
-  gcc_unreachable ();
-  return NULL;
-}
-#endif
-
-#ifndef HAVE_store_multiple
-#define HAVE_store_multiple 0
-static inline rtx
-gen_store_multiple (rtx, rtx, rtx)
-{
-  gcc_unreachable ();
-  return NULL;
-}
-#endif
-
-#ifndef HAVE_tablejump
-#define HAVE_tablejump 0
-static inline rtx
-gen_tablejump (rtx, rtx)
-{
-  gcc_unreachable ();
-  return NULL;
-}
-#endif
-
 #endif /* GCC_INSN_FLAGS_H  */
 
 #endif  /* ! GCC_DEFAULTS_H */
diff --git a/gcc/genflags.c b/gcc/genflags.c
index 0da15f1..2a70b56 100644
--- a/gcc/genflags.c
+++ b/gcc/genflags.c
@@ -26,10 +26,65 @@ along with GCC; see the file COPYING3.  If not see
 #include "tm.h"
 #include "rtl.h"
 #include "obstack.h"
+#include "hash-map.h"
 #include "errors.h"
 #include "read-md.h"
 #include "gensupport.h"
 
+/* Structure which holds data, required for generating stub gen_* function.  */
+struct stub_info_t
+{
+  /* Instruction name.  */
+  const char *name;
+  /* Number of arguments (i.e., instruction operands).  */
+  int opno;
+  /* Set to true when generator is output, so no stub is needed.  */
+  bool done;
+};
+
+/* These instructions require default stub function.  Stubs are never called.
+   They allow to reduce the amount of conditionally-compiled code: if a stub is
+   defined there is no need to guard the call by #ifdef (plain if statement can
+   be used instead).  */
+
+#define DEF_STUB(name, opno) { name, opno, false }
+
+static stub_info_t stubs[] = {
+    DEF_STUB ("simple_return",	     0),
+    DEF_STUB ("return",		     0),
+    DEF_STUB ("epilogue",	     0),
+    DEF_STUB ("mem_thread_fence",    1),
+    DEF_STUB ("memory_barrier",      0),
+    DEF_STUB ("mem_signal_fence",    1),
+    DEF_STUB ("load_multiple",	     3),
+    DEF_STUB ("store_multiple",	     3),
+    DEF_STUB ("tablejump",	     2)
+};
+
+#undef DEF_STUB
+
+/* Helper traits for using null-terminated strings as keys in hash map.
+   FIXME: Unify various "string hashers" and move them to hash-map-traits.h.  */
+struct gflg_string_hasher : default_hashmap_traits
+{
+  typedef const char *value_type;
+  typedef const char *compare_type;
+
+  static inline hashval_t hash (const char *s)
+  {
+    return htab_hash_string (s);
+  }
+
+  static inline bool equal_keys (const char *p1, const char *p2)
+  {
+    return strcmp (p1, p2) == 0;
+  }
+};
+
+/* Mapping from insn name to corresponding stub_info_t entry.  */
+static hash_map<const char *, stub_info_t *, gflg_string_hasher>
+    stubs_map (ARRAY_SIZE (stubs), false, false);
+
 /* Obstack to remember insns with.  */
 static struct obstack obstack;
 
@@ -42,8 +97,18 @@ static int max_opno;
 static void max_operand_1 (rtx);
 static int num_operands (rtx);
 static void gen_proto (rtx);
+static void gen_dummy (const char *, int, bool);
 static void gen_macro (const char *, int, int);
 static void gen_insn (int, rtx);
+static void gen_stub (const char *, int);
+
+/* Remember that either real generator or dummy called NAME has been output.  */
+static inline void
+mark_as_done (const char *name)
+{
+  if (stub_info_t **stub = stubs_map.get (name))
+    (*stub)->done = true;
+}
 
 /* Count the number of match_operand's found.  */
 
@@ -119,6 +184,27 @@ gen_macro (const char *name, int real, int expect)
   printf ("(%c))\n", i + 'A');
 }
 
+/* Print out a dummy for generator for instruction NAME with NUM arguments
+   which either does nothing, or aborts (depending on UNREACHABLE).  */
+
+static void
+gen_dummy (const char *name, int num, bool unreachable)
+{
+  printf ("static inline rtx\ngen_%s (", name);
+
+  if (num > 0)
+    {
+      for (int i = 0; i < num-1; i++)
+	fputs ("rtx, ", stdout);
+      fputs ("rtx", stdout);
+    }
+
+  puts (")\n{");
+  if (unreachable)
+    puts ("  gcc_unreachable ();");
+  puts ("  return NULL;\n}");
+}
+
 /* Print out prototype information for a generator function.  If the
    insn pattern has been elided, print out a dummy generator that
    does nothing.  */
@@ -170,20 +256,9 @@ gen_proto (rtx insn)
   /* Some back ends want to take the address of generator functions,
      so we cannot simply use #define for these dummy definitions.  */
   if (truth == 0)
-    {
-      printf ("static inline rtx\ngen_%s", name);
-      if (num > 0)
-	{
-	  putchar ('(');
-	  for (i = 0; i < num-1; i++)
-	    printf ("rtx ARG_UNUSED (%c), ", 'a' + i);
-	  printf ("rtx ARG_UNUSED (%c))\n", 'a' + i);
-	}
-      else
-	puts ("(void)");
-      puts ("{\n  return 0;\n}");
-    }
+    gen_dummy (name, num, /*unreachable=*/false);
 
+  mark_as_done (name);
 }
 
 static void
@@ -244,6 +319,19 @@ gen_insn (int line_no, rtx insn)
     }
 
   obstack_grow (&obstack, &insn, sizeof (rtx));
+
+  if (truth)
+    mark_as_done (name);
+}
+
+/* Print out stub generator and flag for missing insn.  */
+
+static void
+gen_stub (const char *name, int num)
+{
+  gen_dummy (name, num, /*unreachable=*/true);
+  printf ("#define HAVE_%s 0\n", name);
+  printf ("#define CODE_FOR_%s CODE_FOR_nothing\n", name);
 }
 
 int
@@ -257,6 +345,9 @@ main (int argc, char **argv)
   progname = "genflags";
   obstack_init (&obstack);
 
+  for (unsigned i = 0; i < ARRAY_SIZE (stubs); i++)
+    stubs_map.put (stubs[i].name, &stubs[i]);
+
   /* We need to see all the possibilities.  Elided insns may have
      direct calls to their generators in C code.  */
   insn_elision = 0;
@@ -290,6 +381,11 @@ main (int argc, char **argv)
   for (insn_ptr = insns; *insn_ptr; insn_ptr++)
     gen_proto (*insn_ptr);
 
+  /* Output missing stubs.  */
+  for (unsigned i = 0; i < ARRAY_SIZE (stubs); i++)
+    if (!stubs[i].done)
+      gen_stub (stubs[i].name, stubs[i].opno);
+
   puts ("\n#endif /* GCC_INSN_FLAGS_H */");
 
   if (have_error || ferror (stdout) || fflush (stdout) || fclose (stdout))

Re: [PATCH] Move gen_* stubs from defaults.h to genflags

[Richard Sandiford]

Mikhail Maltsev <maltsevm@gmail.com> writes:
> Hi, all.
>
> I noticed that defaults.h file contains stub generator functions which
> simply call gcc_unreachable. FWIW, Trevor added them to remove some
> conditional compilation which depends on HAVE_<insn_name> macros (I mean
> something like r223624).
>
> Because we still have ~80 more such conditions in GCC, and probably some
> of them will be fixed in the same way, I propose a patch, which allows
> to generate required stubs in genflags.

Nice!

> +/* Structure which holds data, required for generating stub gen_* function.  */

No comma after "data"

> +/* These instructions require default stub function.  Stubs are never called.

"require a default"

> +/* Helper traits for using null-terminated strings as keys in hash map.
> +   FIXME: Unify various "string hashers" and move them to hash-map-traits.h.  */
> +struct gflg_string_hasher : default_hashmap_traits
> +{
> +  typedef const char *value_type;
> +  typedef const char *compare_type;
> +
> +  static inline hashval_t hash (const char *s)
> +  {
> +    return htab_hash_string (s);
> +  }
> +
> +  static inline bool equal_keys (const char *p1, const char *p2)
> +  {
> +    return strcmp (p1, p2) == 0;
> +  }
> +};
> +
> +/* Mapping from insn name to corresponding stub_info_t entry.  */
> +static hash_map<const char *, stub_info_t *, gflg_string_hasher>
> +    stubs_map (ARRAY_SIZE (stubs), false, false);

Seems like this is more naturally a hash_table rather than a hash_map.
I think there's also a preference to avoid static constructor-based
initialisation.

There again, this is a generator, so those kinds of concerns aren't
particularly important.  If we do keep the above though, I think we
should put the hasher in hash-map-table.h now.  Otherwise these FIXMEs
are just going to accumulate, and each time makes it less likely that
any consolidation will actually happen.

Thanks,
Richard

Re: [PATCH] Move gen_* stubs from defaults.h to genflags

[Richard Sandiford]

Richard Sandiford <richard.sandiford@arm.com> writes:
> There again, this is a generator, so those kinds of concerns aren't
> particularly important.  If we do keep the above though, I think we
> should put the hasher in hash-map-table.h now.

Of course I mean hash-map-traits.h :-)

Re: [PATCH] Move gen_* stubs from defaults.h to genflags

[Mikhail Maltsev]

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

On 10.06.2015 10:05, Richard Sandiford wrote:
>> +/* Structure which holds data, required for generating stub gen_* function.  */
> 
> No comma after "data"
> 
>> +/* These instructions require default stub function.  Stubs are never called.
> 
> "require a default"
> 
[snip]
> Seems like this is more naturally a hash_table rather than a hash_map.
> I think there's also a preference to avoid static constructor-based
> initialisation.
Fixed.

> There again, this is a generator, so those kinds of concerns aren't
> particularly important.  If we do keep the above though, I think we
> should put the hasher in hash-map-table.h now.  Otherwise these FIXMEs
> are just going to accumulate, and each time makes it less likely that
> any consolidation will actually happen.
Well, after changing hash_map to hash_table, the hasher class is no
longer identical to other hash traits classes. As for fixing other
occurrences, I think I'd better leave it for another patch.

On 10.06.2015 17:55, Trevor Saunders wrote:
>> +  /* Number of arguments (i.e., instruction operands).  */
>> +  int opno;
> 
> unsigned?
Fixed.

>> +  /* Set to true when generator is output, so no stub is needed.  */
>> +  bool done;
>> +};
>> +
>> +/* These instructions require default stub function.  Stubs are never called.
> 
> are the ones that don't call gcc_unreachable () called?
> 
Well, bootstrap on x86_64 passes without such calls, but in general
case, I think they may get called (comment from genflags.c:main):

  /* We need to see all the possibilities.  Elided insns may have
     direct calls to their generators in C code.  */

For example, this would work if result of gen_* function is passed
directly to some emit_pattern_* function (they can handle NULL pointers).

>> +/* Print out a dummy for generator for instruction NAME with NUM arguments
>> +   which either does nothing, or aborts (depending on UNREACHABLE).  */
> 
> I believe you should drop the first "for" in this sentence.
Fixed.

-- 
Regards,
    Mikhail Maltsev

[-- Attachment #2: gen_defaults2.patch --]
[-- Type: text/plain, Size: 7401 bytes --]

diff --git a/gcc/defaults.h b/gcc/defaults.h
index 057b646..5beddea 100644
--- a/gcc/defaults.h
+++ b/gcc/defaults.h
@@ -1426,96 +1426,6 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 #define TARGET_VTABLE_USES_DESCRIPTORS 0
 #endif
 
-#ifndef HAVE_simple_return
-#define HAVE_simple_return 0
-static inline rtx
-gen_simple_return ()
-{
-  gcc_unreachable ();
-  return NULL;
-}
-#endif
-
-#ifndef HAVE_return
-#define HAVE_return 0
-static inline rtx
-gen_return ()
-{
-  gcc_unreachable ();
-  return NULL;
-}
-#endif
-
-#ifndef HAVE_epilogue
-#define HAVE_epilogue 0
-static inline rtx
-gen_epilogue ()
-{
-  gcc_unreachable ();
-  return NULL;
-}
-#endif
-
-#ifndef HAVE_mem_thread_fence
-#define HAVE_mem_thread_fence 0
-static inline rtx
-gen_mem_thread_fence (rtx)
-{
-  gcc_unreachable ();
-  return NULL;
-}
-#endif
-
-#ifndef HAVE_memory_barrier
-#define HAVE_memory_barrier 0
-static inline rtx
-gen_memory_barrier ()
-{
-  gcc_unreachable ();
-  return NULL;
-}
-#endif
-
-#ifndef HAVE_mem_signal_fence
-#define HAVE_mem_signal_fence 0
-static inline rtx
-gen_mem_signal_fence (rtx)
-{
-  gcc_unreachable ();
-  return NULL;
-}
-#endif
-
-#ifndef HAVE_load_multiple
-#define HAVE_load_multiple 0
-static inline rtx
-gen_load_multiple (rtx, rtx, rtx)
-{
-  gcc_unreachable ();
-  return NULL;
-}
-#endif
-
-#ifndef HAVE_store_multiple
-#define HAVE_store_multiple 0
-static inline rtx
-gen_store_multiple (rtx, rtx, rtx)
-{
-  gcc_unreachable ();
-  return NULL;
-}
-#endif
-
-#ifndef HAVE_tablejump
-#define HAVE_tablejump 0
-static inline rtx
-gen_tablejump (rtx, rtx)
-{
-  gcc_unreachable ();
-  return NULL;
-}
-#endif
-
 #endif /* GCC_INSN_FLAGS_H  */
 
 #endif  /* ! GCC_DEFAULTS_H */
diff --git a/gcc/genflags.c b/gcc/genflags.c
index 0da15f1..2fdf54f 100644
--- a/gcc/genflags.c
+++ b/gcc/genflags.c
@@ -26,10 +26,70 @@ along with GCC; see the file COPYING3.  If not see
 #include "tm.h"
 #include "rtl.h"
 #include "obstack.h"
+#include "hash-table.h"
 #include "errors.h"
 #include "read-md.h"
 #include "gensupport.h"
 
+/* Structure which holds data required for generating stub gen_* function.  */
+
+struct stub_info_t : typed_noop_remove<stub_info_t>
+{
+  stub_info_t (const char *, unsigned);
+
+  /* Instruction name.  */
+  const char *name;
+  /* Number of arguments (i.e., instruction operands).  */
+  unsigned opno;
+  /* Set to true when generator is output, so no stub is needed.  */
+  bool done;
+
+  /* Helpers for hash_table.  */
+  typedef stub_info_t *value_type;
+  typedef const char *compare_type;
+  static inline hashval_t hash (const stub_info_t *);
+  static inline bool equal (const stub_info_t *, const char *);
+};
+
+stub_info_t::stub_info_t (const char *name_, unsigned opno_) : name (name_),
+							       opno (opno_),
+							       done (false)
+{
+}
+
+inline hashval_t
+stub_info_t::hash (const stub_info_t *elem)
+{
+  return htab_hash_string (elem->name);
+}
+
+inline bool
+stub_info_t::equal (const stub_info_t *elem, const char *key)
+{
+  return strcmp (elem->name, key) == 0;
+}
+
+/* These instructions require a default stub function.  Stubs are never called.
+   They allow to reduce the amount of conditionally-compiled code: if a stub is
+   defined there is no need to guard the call by #ifdef (plain if statement can
+   be used instead).  */
+
+static stub_info_t stubs[] = {
+    stub_info_t ("simple_return",	    0),
+    stub_info_t ("return",		    0),
+    stub_info_t ("epilogue",		    0),
+    stub_info_t ("mem_thread_fence",	    1),
+    stub_info_t ("memory_barrier",	    0),
+    stub_info_t ("mem_signal_fence",	    1),
+    stub_info_t ("load_multiple",	    3),
+    stub_info_t ("store_multiple",	    3),
+    stub_info_t ("tablejump",		    2)
+};
+
+/* Mapping from insn name to corresponding stub_info_t entry.  */
+typedef hash_table<stub_info_t> stubs_htab_t;
+static stubs_htab_t *stubs_htab;
+
 /* Obstack to remember insns with.  */
 static struct obstack obstack;
 
@@ -42,8 +102,19 @@ static int max_opno;
 static void max_operand_1 (rtx);
 static int num_operands (rtx);
 static void gen_proto (rtx);
+static void gen_dummy (const char *, unsigned, bool);
 static void gen_macro (const char *, int, int);
 static void gen_insn (int, rtx);
+static void gen_stub (const char *, unsigned);
+
+/* Remember that either real generator or dummy called NAME has been output.  */
+static inline void
+mark_as_done (const char *name)
+{
+  if (stub_info_t **stub = stubs_htab->find_slot_with_hash (name,
+					htab_hash_string (name), NO_INSERT))
+    (*stub)->done = true;
+}
 
 /* Count the number of match_operand's found.  */
 
@@ -119,6 +190,27 @@ gen_macro (const char *name, int real, int expect)
   printf ("(%c))\n", i + 'A');
 }
 
+/* Print out a dummy generator for instruction NAME with NUM arguments which
+   either does nothing, or aborts (depending on UNREACHABLE).  */
+
+static void
+gen_dummy (const char *name, unsigned num, bool unreachable)
+{
+  printf ("static inline rtx\ngen_%s (", name);
+
+  if (num > 0)
+    {
+      for (unsigned i = 0; i < num-1; i++)
+	fputs ("rtx, ", stdout);
+      fputs ("rtx", stdout);
+    }
+
+  puts (")\n{");
+  if (unreachable)
+    puts ("  gcc_unreachable ();");
+  puts ("  return NULL;\n}");
+}
+
 /* Print out prototype information for a generator function.  If the
    insn pattern has been elided, print out a dummy generator that
    does nothing.  */
@@ -170,20 +262,9 @@ gen_proto (rtx insn)
   /* Some back ends want to take the address of generator functions,
      so we cannot simply use #define for these dummy definitions.  */
   if (truth == 0)
-    {
-      printf ("static inline rtx\ngen_%s", name);
-      if (num > 0)
-	{
-	  putchar ('(');
-	  for (i = 0; i < num-1; i++)
-	    printf ("rtx ARG_UNUSED (%c), ", 'a' + i);
-	  printf ("rtx ARG_UNUSED (%c))\n", 'a' + i);
-	}
-      else
-	puts ("(void)");
-      puts ("{\n  return 0;\n}");
-    }
+    gen_dummy (name, num, /*unreachable=*/false);
 
+  mark_as_done (name);
 }
 
 static void
@@ -244,6 +325,19 @@ gen_insn (int line_no, rtx insn)
     }
 
   obstack_grow (&obstack, &insn, sizeof (rtx));
+
+  if (truth)
+    mark_as_done (name);
+}
+
+/* Print out stub generator and flag for missing insn.  */
+
+static void
+gen_stub (const char *name, unsigned num)
+{
+  gen_dummy (name, num, /*unreachable=*/true);
+  printf ("#define HAVE_%s 0\n", name);
+  printf ("#define CODE_FOR_%s CODE_FOR_nothing\n", name);
 }
 
 int
@@ -257,6 +351,11 @@ main (int argc, char **argv)
   progname = "genflags";
   obstack_init (&obstack);
 
+  stubs_htab = new stubs_htab_t (ARRAY_SIZE (stubs), false, false);
+  for (unsigned i = 0; i < ARRAY_SIZE (stubs); i++)
+    *(stubs_htab->find_slot_with_hash (stubs[i].name,
+			htab_hash_string (stubs[i].name), INSERT)) = &stubs[i];
+
   /* We need to see all the possibilities.  Elided insns may have
      direct calls to their generators in C code.  */
   insn_elision = 0;
@@ -290,6 +389,12 @@ main (int argc, char **argv)
   for (insn_ptr = insns; *insn_ptr; insn_ptr++)
     gen_proto (*insn_ptr);
 
+  /* Output missing stubs.  */
+  for (unsigned i = 0; i < ARRAY_SIZE (stubs); i++)
+    if (!stubs[i].done)
+      gen_stub (stubs[i].name, stubs[i].opno);
+  delete stubs_htab;
+
   puts ("\n#endif /* GCC_INSN_FLAGS_H */");
 
   if (have_error || ferror (stdout) || fflush (stdout) || fclose (stdout))

[-- Attachment #3: gen_defaults.clog --]
[-- Type: text/plain, Size: 645 bytes --]

gcc/ChangeLog:

2015-06-10  Mikhail Maltsev  <maltsevm@gmail.com>

	* defaults.h (gen_simple_return, gen_return, gen_mem_thread_fence,
	gen_memory_barrier, gen_mem_signal_fence, gen_load_multiple,
	gen_store_multiple, gen_tablejump): Remove.  Generate by genflags.
	(HAVE_*): Likewise.
	* genflags.c (struct stub_info_t): Define.
	(stubs): Define variable.
	(stubs_htab): Define variable.
	(mark_as_done): New function.
	(gen_dummy): New function (move code out of gen_proto, generalize).
	(gen_proto): Use gen_dummy.
	(gen_insn): Call mark_as_done.
	(gen_stub): New function.
	(main): Initialize stubs_htab, call gen_stubs for missing stubs.

Re: [PATCH] Move gen_* stubs from defaults.h to genflags

[Richard Sandiford]

Mikhail Maltsev <maltsevm@gmail.com> writes:
> On 10.06.2015 10:05, Richard Sandiford wrote:
>>> +/* Structure which holds data, required for generating stub gen_* function.  */
>> 
>> No comma after "data"
>> 
>>> +/* These instructions require default stub function.  Stubs are never called.
>> 
>> "require a default"
>> 
> [snip]
>> Seems like this is more naturally a hash_table rather than a hash_map.
>> I think there's also a preference to avoid static constructor-based
>> initialisation.
> Fixed.
>
>> There again, this is a generator, so those kinds of concerns aren't
>> particularly important.  If we do keep the above though, I think we
>> should put the hasher in hash-map-table.h now.  Otherwise these FIXMEs
>> are just going to accumulate, and each time makes it less likely that
>> any consolidation will actually happen.
> Well, after changing hash_map to hash_table, the hasher class is no
> longer identical to other hash traits classes. As for fixing other
> occurrences, I think I'd better leave it for another patch.

There are other hash_table string traits though.  E.g. config/i386/winnt.c
and java/jcf-io.c.  Let's not add any more.

FWIW I have some patches to try to clean up the hashing traits.
I hope to post them later today.


Also, sorry for the runaround, but it occured to me later that if we're
getting the generators to help us with the default definitions, we might
as well go one step further and move the HAVE_foo/gen_foo interface to
the target structure, with the structure initialiser being filled in by
the generators.  I.e. rather than generating a default HAVE_foo and dummy
gen_foo, we generate definitions for TARGET_HAVE_FOO and TARGET_GEN_FOO.
This should remove the insn-flags.h dependency from most of the
target-independent code.  There'd be a .def file to list the instructions
involved in the HAVE/GEN interface.

I have a patch, but also needed a string hash, so ended up spending
rather too long on that instead.  Hope to post it when the hashing
stuff is done.

Thanks,
Richard

Re: [PATCH] Move gen_* stubs from defaults.h to genflags

[Trevor Saunders]

On Wed, Jun 10, 2015 at 07:28:10AM +0300, Mikhail Maltsev wrote:
> Hi, all.
> 
> I noticed that defaults.h file contains stub generator functions which
> simply call gcc_unreachable. FWIW, Trevor added them to remove some
> conditional compilation which depends on HAVE_<insn_name> macros (I mean
> something like r223624).
> 
> Because we still have ~80 more such conditions in GCC, and probably some
> of them will be fixed in the same way, I propose a patch, which allows
> to generate required stubs in genflags.

yeah, I've been meaning to do this, thanks for taking care of it.  My
plan had been to do it after all the defaults had been added, but if you
can do it now great.

See https://github.com/tbsaunde/gcc/tree/ifdef for the patches I have in
this area, unfortunately I don't expect to commit any of that till july
when I get back from travel.

> diff --git a/gcc/genflags.c b/gcc/genflags.c
> index 0da15f1..2a70b56 100644
> --- a/gcc/genflags.c
> +++ b/gcc/genflags.c
> @@ -26,10 +26,65 @@ along with GCC; see the file COPYING3.  If not see
>  #include "tm.h"
>  #include "rtl.h"
>  #include "obstack.h"
> +#include "hash-map.h"
>  #include "errors.h"
>  #include "read-md.h"
>  #include "gensupport.h"
>  
> +/* Structure which holds data, required for generating stub gen_* function.  */
> +struct stub_info_t
> +{
> +  /* Instruction name.  */
> +  const char *name;
> +  /* Number of arguments (i.e., instruction operands).  */
> +  int opno;

unsigned?

> +  /* Set to true when generator is output, so no stub is needed.  */
> +  bool done;
> +};
> +
> +/* These instructions require default stub function.  Stubs are never called.

are the ones that don't call gcc_unreachable () called?

> +/* Print out a dummy for generator for instruction NAME with NUM arguments
> +   which either does nothing, or aborts (depending on UNREACHABLE).  */

I believe you should drop the first "for" in this sentence.

Trev