diff mbox

Move gen_* stubs from defaults.h to genflags

Message ID 557E04A7.60902@gmail.com
State New
Headers show

Commit Message

Mikhail Maltsev June 14, 2015, 10:48 p.m. UTC
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.

Comments

Richard Sandiford June 15, 2015, 9:15 a.m. UTC | #1
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
diff mbox

Patch

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))