Add .def file for public target instructions

Add .def file for public target instructions

[Richard Sandiford]

[A fair bit later than promised, sorry...]

Mikhail posted a patch to make genflags generate the default HAVE_foo
and gen_foo definitions that have recently been added to defaults.h:

  https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00723.html

I agree it'd be a good idea to generate this kind of thing automatically,
but I think we should take the opportunity to move the interface to the
target structure.  I.e.:

  HAVE_foo -> targetm.have_foo ()
  gen_foo -> targetm.gen_foo ()

This should move us closer to the pipedream goal of supporting multiple
targets at once.  It should also mean that only the target code depends
on insn-flags.h.

The patch just moves return and simple_return as an example.  I have more
locally (in order to test other code paths), but they're just an obvious
extension of this one.

The patch relies on the hashing changes in:

  https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01066.html
  https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01564.html

and on this trivial patch:

  https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01604.html

It seems a bit heavyweight when you just look at these two instructions,
but I think it'll be a saving in the end.

Bootstrapped & regression-tested on x86_64-linux-gnu.  Also tested
via config-list.mk.  OK to install?

Thanks,
Richard


gcc/
	* Makefile.in (TARGET_DEF): Add target-insns.def.
	(.PRECIOUS, simple_rtl_generated_h): Add insn-target-def.h.
	(build/gentarget-def.o): New rule.
	(genprogrtl): Add target-def.
	* target-insns.def, gentarget-def.c: New files.
	* target.def: Add targetm.have_* and targetm.gen_* hooks,
	based on the contents of target-insns.def.
	* defaults.h (HAVE_simple_return, gen_simple_return): Delete.
	(HAVE_return, gen_return): Delete.
	* target-def.h: Include insn-target-def.h.
	* cfgrtl.c (force_nonfallthru_and_redirect): Use targetm interface
	instead of direct calls.  Rely on them to do the appropriate assertions.
	* function.c (gen_return_pattern): Likewise.  Return an rtx_insn *.
	(convert_jumps_to_returns): Use targetm interface instead of
	direct calls.
	(thread_prologue_and_epilogue_insns): Likewise.
	* reorg.c (find_end_label, dbr_schedule): Likewise.
	* shrink-wrap.h (SHRINK_WRAPPING_ENABLED): Likewise.
	* shrink-wrap.c (convert_to_simple_return): Likewise.
	(try_shrink_wrapping): Use SHRINK_WRAPPING_ENABLED.

Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in	2015-06-22 14:03:10.985503368 +0100
+++ gcc/Makefile.in	2015-06-22 14:04:26.689971484 +0100
@@ -866,7 +866,7 @@ DUMPFILE_H = $(srcdir)/../libcpp/include
 VEC_H = vec.h statistics.h $(GGC_H)
 HASH_TABLE_H = $(HASHTAB_H) hash-table.h
 EXCEPT_H = except.h $(HASHTAB_H)
-TARGET_DEF = target.def target-hooks-macros.h
+TARGET_DEF = target.def target-hooks-macros.h target-insns.def
 C_TARGET_DEF = c-family/c-target.def target-hooks-macros.h
 COMMON_TARGET_DEF = common/common-target.def target-hooks-macros.h
 TARGET_H = $(TM_H) target.h $(TARGET_DEF) insn-modes.h insn-codes.h
@@ -2078,7 +2078,8 @@ $(common_out_object_file): $(common_out_
 .PRECIOUS: insn-config.h insn-flags.h insn-codes.h insn-constants.h \
   insn-emit.c insn-recog.c insn-extract.c insn-output.c insn-peep.c \
   insn-attr.h insn-attr-common.h insn-attrtab.c insn-dfatab.c \
-  insn-latencytab.c insn-preds.c gimple-match.c generic-match.c
+  insn-latencytab.c insn-preds.c gimple-match.c generic-match.c \
+  insn-target-def.h
 
 # Dependencies for the md file.  The first time through, we just assume
 # the md file itself and the generated dependency file (in order to get
@@ -2099,7 +2100,7 @@ s-mddeps: $(md_file) $(MD_INCLUDES) buil
 # the target file.
 
 simple_rtl_generated_h	= insn-attr.h insn-attr-common.h insn-codes.h \
-			  insn-config.h insn-flags.h
+			  insn-config.h insn-flags.h insn-target-def.h
 
 simple_rtl_generated_c	= insn-automata.c insn-emit.c \
 			  insn-extract.c insn-output.c \
@@ -2498,6 +2499,9 @@ build/genextract.o : genextract.c $(RTL_
   $(SYSTEM_H) coretypes.h $(GTM_H) errors.h $(READ_MD_H) gensupport.h
 build/genflags.o : genflags.c $(RTL_BASE_H) $(OBSTACK_H) $(BCONFIG_H)	\
   $(SYSTEM_H) coretypes.h $(GTM_H) errors.h $(READ_MD_H) gensupport.h
+build/gentarget-def.o : gentarget-def.c $(BCONFIG_H) $(SYSTEM_H)	\
+  coretypes.h $(GTM_H) $(RTL_BASE_H) errors.h $(READ_MD_H) gensupport.h	\
+  $(HASH_TABLE_H) target-insns.def
 build/gengenrtl.o : gengenrtl.c $(BCONFIG_H) $(SYSTEM_H) rtl.def
 
 # The gengtype generator program is special: Two versions are built.
@@ -2562,7 +2566,7 @@ build/genmatch.o : genmatch.c $(BCONFIG_
 
 # All these programs use the RTL reader ($(BUILD_RTL)).
 genprogrtl = attr attr-common attrtab automata codes conditions config emit \
-	     extract flags opinit output peep preds recog mddump
+	     extract flags mddump opinit output peep preds recog target-def
 $(genprogrtl:%=build/gen%$(build_exeext)): $(BUILD_RTL)
 
 # All these programs use the MD reader ($(BUILD_MD)).
Index: gcc/target-insns.def
===================================================================
--- /dev/null	2015-06-22 13:54:06.971314091 +0100
+++ gcc/target-insns.def	2015-06-22 14:04:26.690971419 +0100
@@ -0,0 +1,34 @@
+/* Target instruction definitions.
+   Copyright (C) 2015 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by the
+   Free Software Foundation; either version 3, or (at your option) any
+   later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; see the file COPYING3.  If not see
+   <http://www.gnu.org/licenses/>.  */
+
+/* This file has one entry for each public pattern name that the target
+   can provide.  It is only used if no distinction between operand modes
+   is necessary.  If separate patterns are needed for different modes
+   (so as to distinguish addition of QImode values from addition of
+   HImode values, for example) then an optab should be used instead.
+
+   Each entry has the form:
+
+     DEF_TARGET_INSN (name, prototype)
+
+   where NAME is the name of the pattern and PROTOTYPE is its C prototype.
+   The prototype should use parameter names of the form "x0", "x1", etc.
+   Patterns that take no operands should have a prototype "(void)".
+
+   Instructions should be documented in md.texi rather than here.  */
+DEF_TARGET_INSN (return, (void))
+DEF_TARGET_INSN (simple_return, (void))
Index: gcc/gentarget-def.c
===================================================================
--- /dev/null	2015-06-22 13:54:06.971314091 +0100
+++ gcc/gentarget-def.c	2015-06-22 14:04:26.689971484 +0100
@@ -0,0 +1,265 @@
+/* Generate insn-target-def.h, an automatically-generated part of targetm.
+   Copyright (C) 1987-2015 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#include "bconfig.h"
+#include "system.h"
+#include "coretypes.h"
+#include "tm.h"
+#include "rtl.h"
+#include "errors.h"
+#include "read-md.h"
+#include "gensupport.h"
+#include "hash-table.h"
+
+/* This class hashes define_insns and define_expands by name.  */
+struct insn_hasher : nofree_ptr_hash <rtx_def>
+{
+  typedef rtx value_type;
+  typedef const char *compare_type;
+
+  static inline hashval_t hash (rtx);
+  static inline bool equal (rtx, const char *);
+};
+
+hashval_t
+insn_hasher::hash (rtx x)
+{
+  return htab_hash_string (XSTR (x, 0));
+}
+
+bool
+insn_hasher::equal (rtx x, const char *y)
+{
+  return strcmp (XSTR (x, 0), y) == 0;
+}
+
+/* All define_insns and define_expands, hashed by name.  */
+static hash_table <insn_hasher> *insns;
+
+/* Records the prototype suffix X for each invalid_X stub that has been
+   generated.  */
+static hash_table <nofree_string_hash> *stubs;
+
+/* Records which C conditions have been wrapped in functions, as a mapping
+   from the C condition to the function name.  */
+static hash_map <nofree_string_hash, const char *> *have_funcs;
+
+/* Output hook definitions for pattern NAME, which has target-insns.def
+   prototype PROTOTYPE.  */
+
+static void
+def_target_insn (const char *name, const char *prototype)
+{
+  /* Get an upper-case form of NAME.  */
+  unsigned int i;
+  char *upper_name = XALLOCAVEC (char, strlen (name) + 1);
+  for (i = 0; name[i]; ++i)
+    upper_name[i] = TOUPPER (name[i]);
+  upper_name[i] = 0;
+
+  /* Check that the prototype is valid and concatenate the types
+     together to get a suffix.  */
+  char *suffix = XALLOCAVEC (char, strlen (prototype) + 1);
+  i = 0;
+  unsigned int opno = 0;
+  for (const char *p = prototype; *p; ++p)
+    if (*p == 'x' && ISDIGIT (p[1]))
+      {
+	/* This should be a parameter name of the form "x<OPNO>".
+	   That doesn't contribute to the suffix, so skip ahead and
+	   process the following character.  */
+	char *endptr;
+	if (strtol (p + 1, &endptr, 10) != opno
+	    || (*endptr != ',' && *endptr != ')'))
+	  {
+	    error ("invalid prototype for '%s'", name);
+	    exit (FATAL_EXIT_CODE);
+	  }
+	opno += 1;
+	p = endptr;
+	if (*p == ',')
+	  suffix[i++] = '_';
+      }
+    else if (*p == ')' || *p == ',')
+      {
+	/* We found the end of a parameter without finding a
+	   parameter name.  */
+	if (strcmp (prototype, "(void)") != 0)
+	  {
+	    error ("argument %d of '%s' did not have the expected name",
+		   opno, name);
+	    exit (FATAL_EXIT_CODE);
+	  }
+      }
+    else if (*p != '(' && !ISSPACE (*p))
+      suffix[i++] = *p;
+  suffix[i] = 0;
+
+  /* See whether we have an implementation of this pattern.  */
+  hashval_t hash = htab_hash_string (name);
+  int truth = 0;
+  const char *have_name = name;
+  if (rtx insn = insns->find_with_hash (name, hash))
+    {
+      const char *test = XSTR (insn, 2);
+      truth = maybe_eval_c_test (test);
+      gcc_assert (truth != 0);
+      if (truth < 0)
+	{
+	  /* Try to reuse an existing function that performs the same test.  */
+	  bool existed;
+	  const char *&entry = have_funcs->get_or_insert (test, &existed);
+	  if (!existed)
+	    {
+	      entry = name;
+	      printf ("\nstatic bool\n");
+	      printf ("target_have_%s (void)\n", name);
+	      printf ("{\n");
+	      printf ("  return ");
+	      print_c_condition (test);
+	      printf (";\n");
+	      printf ("}\n");
+	    }
+	  have_name = entry;
+	}
+      printf ("\nstatic rtx_insn *\n");
+      printf ("target_gen_%s %s\n", name, prototype);
+      printf ("{\n");
+      if (truth < 0)
+	printf ("  gcc_checking_assert (targetm.have_%s ());\n", name);
+      printf ("  return insnify (gen_%s (", name);
+      for (i = 0; i < opno; ++i)
+	printf ("%sx%d", i == 0 ? "" : ", ", i);
+      printf ("));\n");
+      printf ("}\n");
+    }
+  else
+    {
+      const char **slot = stubs->find_slot (suffix, INSERT);
+      if (!*slot)
+	{
+	  *slot = xstrdup (suffix);
+	  printf ("\nstatic rtx_insn *\n");
+	  printf ("invalid_%s ", suffix);
+	  const char *p = prototype;
+	  while (*p)
+	    {
+	      if (p[0] == 'x' && ISDIGIT (p[1]))
+		{
+		  char *endptr;
+		  strtol (p + 1, &endptr, 10);
+		  p = endptr;
+		}
+	      else
+		fputc (*p++, stdout);
+	    }
+	  printf ("\n{\n");
+	  printf ("  gcc_unreachable ();\n");
+	  printf ("}\n");
+	}
+    }
+  printf ("\n#undef TARGET_HAVE_%s\n", upper_name);
+  printf ("#define TARGET_HAVE_%s ", upper_name);
+  if (truth == 0)
+    printf ("hook_bool_void_false\n");
+  else if (truth == 1)
+    printf ("hook_bool_void_true\n");
+  else
+    printf ("target_have_%s\n", have_name);
+
+  printf ("#undef TARGET_GEN_%s\n", upper_name);
+  printf ("#define TARGET_GEN_%s ", upper_name);
+  if (truth == 0)
+    printf ("invalid_%s\n", suffix);
+  else
+    printf ("target_gen_%s\n", name);
+}
+
+int
+main (int argc, char **argv)
+{
+  int insn_code_number = 0;
+
+  progname = "gentarget-def";
+
+  if (!init_rtx_reader_args (argc, argv))
+    return (FATAL_EXIT_CODE);
+
+  insns = new hash_table <insn_hasher> (31);
+  stubs = new hash_table <nofree_string_hash> (31);
+  have_funcs = new hash_map <nofree_string_hash, const char *>;
+
+  while (1)
+    {
+      int line_no;
+      rtx desc = read_md_rtx (&line_no, &insn_code_number);
+      if (desc == NULL)
+	break;
+      if (GET_CODE (desc) == DEFINE_INSN || GET_CODE (desc) == DEFINE_EXPAND)
+	{
+	  const char *name = XSTR (desc, 0);
+	  if (name[0] != 0 && name[0] != '*')
+	    {
+	      hashval_t hash = htab_hash_string (name);
+	      rtx *slot = insns->find_slot_with_hash (name, hash, INSERT);
+	      if (*slot)
+		{
+		  message_with_line (line_no, "duplicate definition of '%s'",
+				     name);
+		  have_error = 1;
+		}
+	      else
+		*slot = desc;
+	    }
+	}
+    }
+
+  printf ("/* Generated automatically by the program `gentarget-def'.  */\n");
+  printf ("#ifndef GCC_INSN_TARGET_DEF_H\n");
+  printf ("#define GCC_INSN_TARGET_DEF_H\n");
+
+  /* Output a routine to convert an rtx to an rtx_insn sequence.
+     ??? At some point the gen_* functions themselves should return
+	 rtx_insns.  */
+  printf ("\nstatic inline rtx_insn *\n");
+  printf ("insnify (rtx x)\n");
+  printf ("{\n");
+  printf ("  if (!x)\n");
+  printf ("    return NULL;\n");
+  printf ("  if (rtx_insn *insn = dyn_cast <rtx_insn *> (x))\n");
+  printf ("    return insn;\n");
+  printf ("  start_sequence ();\n");
+  printf ("  emit (x);\n");
+  printf ("  rtx_insn *res = get_insns ();\n");
+  printf ("  end_sequence ();\n");
+  printf ("  return res;\n");
+  printf ("}\n");
+
+#define DEF_TARGET_INSN(INSN, ARGS) \
+  def_target_insn (#INSN, #ARGS);
+#include "target-insns.def"
+#undef DEF_TARGET_INSN
+
+  printf ("\n#endif /* GCC_INSN_TARGET_DEF_H */\n");
+
+  if (have_error || ferror (stdout) || fflush (stdout) || fclose (stdout))
+    return FATAL_EXIT_CODE;
+
+  return SUCCESS_EXIT_CODE;
+}
Index: gcc/target.def
===================================================================
--- gcc/target.def	2015-06-22 14:03:10.985503368 +0100
+++ gcc/target.def	2015-06-22 14:04:26.690971419 +0100
@@ -5864,6 +5864,19 @@ DEFHOOK
 
 HOOK_VECTOR_END (mode_switching)
 
+#undef HOOK_PREFIX
+#define HOOK_PREFIX "TARGET_"
+
+#define DEF_TARGET_INSN(NAME, PROTO) \
+  DEFHOOK_UNDOC (have_##NAME, "", bool, (void), false)
+#include "target-insns.def"
+#undef DEF_TARGET_INSN
+
+#define DEF_TARGET_INSN(NAME, PROTO) \
+  DEFHOOK_UNDOC (gen_##NAME, "", rtx_insn *, PROTO, NULL)
+#include "target-insns.def"
+#undef DEF_TARGET_INSN
+
 /* Close the 'struct gcc_target' definition.  */
 HOOK_VECTOR_END (C90_EMPTY_HACK)
 
Index: gcc/defaults.h
===================================================================
--- gcc/defaults.h	2015-06-22 14:03:10.985503368 +0100
+++ gcc/defaults.h	2015-06-22 14:04:26.691971355 +0100
@@ -1426,26 +1426,6 @@ #define STACK_CHECK_MAX_VAR_SIZE (STACK_
 #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
Index: gcc/target-def.h
===================================================================
--- gcc/target-def.h	2015-06-22 14:03:10.985503368 +0100
+++ gcc/target-def.h	2015-06-22 14:04:26.689971484 +0100
@@ -107,3 +107,4 @@ #define TARGET_FUNCTION_INCOMING_ARG TAR
 
 #include "hooks.h"
 #include "targhooks.h"
+#include "insn-target-def.h"
Index: gcc/cfgrtl.c
===================================================================
--- gcc/cfgrtl.c	2015-06-22 14:02:23.724223518 +0100
+++ gcc/cfgrtl.c	2015-06-22 14:04:26.693971225 +0100
@@ -1695,19 +1695,12 @@ force_nonfallthru_and_redirect (edge e,
   if (target == EXIT_BLOCK_PTR_FOR_FN (cfun))
     {
       if (jump_label == ret_rtx)
-	{
-	  if (!HAVE_return)
-	    gcc_unreachable ();
-
-	  emit_jump_insn_after_setloc (gen_return (), BB_END (jump_block), loc);
-	}
+	emit_jump_insn_after_setloc (targetm.gen_return (),
+				     BB_END (jump_block), loc);
       else
 	{
 	  gcc_assert (jump_label == simple_return_rtx);
-	  if (!HAVE_simple_return)
-	    gcc_unreachable ();
-
-	  emit_jump_insn_after_setloc (gen_simple_return (),
+	  emit_jump_insn_after_setloc (targetm.gen_simple_return (),
 				       BB_END (jump_block), loc);
 	}
       set_return_jump_label (BB_END (jump_block));
Index: gcc/function.c
===================================================================
--- gcc/function.c	2015-06-22 14:04:03.369476746 +0100
+++ gcc/function.c	2015-06-22 14:04:26.692971290 +0100
@@ -5657,13 +5657,12 @@ emit_use_return_register_into_block (bas
 /* Create a return pattern, either simple_return or return, depending on
    simple_p.  */
 
-static rtx
+static rtx_insn *
 gen_return_pattern (bool simple_p)
 {
-  if (!HAVE_simple_return)
-    gcc_assert (!simple_p);
-
-  return simple_p ? gen_simple_return () : gen_return ();
+  return (simple_p
+	  ? targetm.gen_simple_return ()
+	  : targetm.gen_return ());
 }
 
 /* Insert an appropriate return pattern at the end of block BB.  This
@@ -5769,7 +5768,7 @@ convert_jumps_to_returns (basic_block la
 	    dest = ret_rtx;
 	  if (!redirect_jump (as_a <rtx_jump_insn *> (jump), dest, 0))
 	    {
-	      if (HAVE_simple_return && simple_p)
+	      if (targetm.have_simple_return () && simple_p)
 		{
 		  if (dump_file)
 		    fprintf (dump_file,
@@ -5790,7 +5789,7 @@ convert_jumps_to_returns (basic_block la
 	}
       else
 	{
-	  if (HAVE_simple_return && simple_p)
+	  if (targetm.have_simple_return () && simple_p)
 	    {
 	      if (dump_file)
 		fprintf (dump_file,
@@ -5981,12 +5980,12 @@ thread_prologue_and_epilogue_insns (void
 
   exit_fallthru_edge = find_fallthru_edge (EXIT_BLOCK_PTR_FOR_FN (cfun)->preds);
 
-  if (HAVE_simple_return && entry_edge != orig_entry_edge)
+  if (targetm.have_simple_return () && entry_edge != orig_entry_edge)
     exit_fallthru_edge
 	= get_unconverted_simple_return (exit_fallthru_edge, bb_flags,
 					 &unconverted_simple_returns,
 					 &returnjump);
-  if (HAVE_return)
+  if (targetm.have_return ())
     {
       if (exit_fallthru_edge == NULL)
 	goto epilogue_done;
@@ -6007,7 +6006,8 @@ thread_prologue_and_epilogue_insns (void
 
 	      /* Emitting the return may add a basic block.
 		 Fix bb_flags for the added block.  */
-	      if (HAVE_simple_return && last_bb != exit_fallthru_edge->src)
+	      if (targetm.have_simple_return ()
+		  && last_bb != exit_fallthru_edge->src)
 		bitmap_set_bit (&bb_flags, last_bb->index);
 
 	      goto epilogue_done;
@@ -6123,7 +6123,7 @@ thread_prologue_and_epilogue_insns (void
 	}
     }
 
-  if (HAVE_simple_return)
+  if (targetm.have_simple_return ())
     convert_to_simple_return (entry_edge, orig_entry_edge, bb_flags,
 			      returnjump, unconverted_simple_returns);
 
@@ -6139,8 +6139,9 @@ thread_prologue_and_epilogue_insns (void
 
       if (!CALL_P (insn)
 	  || ! SIBLING_CALL_P (insn)
-	  || (HAVE_simple_return && (entry_edge != orig_entry_edge
-				     && !bitmap_bit_p (&bb_flags, bb->index))))
+	  || (targetm.have_simple_return ()
+	      && entry_edge != orig_entry_edge
+	      && !bitmap_bit_p (&bb_flags, bb->index)))
 	{
 	  ei_next (&ei);
 	  continue;
Index: gcc/reorg.c
===================================================================
--- gcc/reorg.c	2015-06-22 14:03:10.985503368 +0100
+++ gcc/reorg.c	2015-06-22 14:04:26.692971290 +0100
@@ -473,7 +473,7 @@ find_end_label (rtx kind)
 	}
       else
 	{
-	  if (HAVE_epilogue && ! HAVE_return)
+	  if (HAVE_epilogue && ! targetm.have_return ())
 	    /* The RETURN insn has its delay slot filled so we cannot
 	       emit the label just before it.  Since we already have
 	       an epilogue and cannot emit a new RETURN, we cannot
@@ -483,10 +483,10 @@ find_end_label (rtx kind)
 	  /* Otherwise, make a new label and emit a RETURN and BARRIER,
 	     if needed.  */
 	  emit_label (label);
-	  if (HAVE_return)
+	  if (targetm.have_return ())
 	    {
 	      /* The return we make may have delay slots too.  */
-	      rtx pat = gen_return ();
+	      rtx_insn *pat = targetm.gen_return ();
 	      rtx_insn *insn = emit_jump_insn (pat);
 	      set_return_jump_label (insn);
 	      emit_barrier ();
@@ -3815,8 +3815,9 @@ dbr_schedule (rtx_insn *first)
     delete_related_insns (function_simple_return_label);
 
   need_return_insns = false;
-  need_return_insns |= HAVE_return && function_return_label != 0;
-  need_return_insns |= HAVE_simple_return && function_simple_return_label != 0;
+  need_return_insns |= targetm.have_return () && function_return_label != 0;
+  need_return_insns |= (targetm.have_simple_return ()
+			&& function_simple_return_label != 0);
   if (need_return_insns)
     make_return_insns (first);
 
Index: gcc/shrink-wrap.h
===================================================================
--- gcc/shrink-wrap.h	2015-06-22 14:03:10.985503368 +0100
+++ gcc/shrink-wrap.h	2015-06-22 14:04:26.692971290 +0100
@@ -36,7 +36,8 @@ extern void convert_to_simple_return (ed
 				      bitmap_head bb_flags,
 				      rtx_insn *returnjump,
 				      vec<edge> unconverted_simple_returns);
-#define SHRINK_WRAPPING_ENABLED (flag_shrink_wrap && HAVE_simple_return)
+#define SHRINK_WRAPPING_ENABLED \
+  (flag_shrink_wrap && targetm.have_simple_return ())
 
 #endif  /* GCC_SHRINK_WRAP_H  */
 
Index: gcc/shrink-wrap.c
===================================================================
--- gcc/shrink-wrap.c	2015-06-22 14:03:10.985503368 +0100
+++ gcc/shrink-wrap.c	2015-06-22 14:04:26.692971290 +0100
@@ -541,7 +541,7 @@ try_shrink_wrapping (edge *entry_edge, e
 	break;
       }
 
-  if (flag_shrink_wrap && HAVE_simple_return
+  if (SHRINK_WRAPPING_ENABLED
       && (targetm.profile_before_prologue () || !crtl->profile)
       && nonempty_prologue && !crtl->calls_eh_return)
     {
@@ -1004,8 +1004,8 @@ convert_to_simple_return (edge entry_edg
 
 	      bb = create_basic_block (NULL, NULL, exit_pred);
 	      BB_COPY_PARTITION (bb, e->src);
-	      rtx_jump_insn *start = emit_jump_insn_after (gen_simple_return (),
-							   BB_END (bb));
+	      rtx_insn *ret = targetm.gen_simple_return ();
+	      rtx_jump_insn *start = emit_jump_insn_after (ret, BB_END (bb));
 	      JUMP_LABEL (start) = simple_return_rtx;
 	      emit_barrier_after (start);
 

Re: Add .def file for public target instructions

[Jeff Law]

On 06/23/2015 12:41 PM, Richard Sandiford wrote:
> [A fair bit later than promised, sorry...]
>
> Mikhail posted a patch to make genflags generate the default HAVE_foo
> and gen_foo definitions that have recently been added to defaults.h:
>
>    https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00723.html
>
> I agree it'd be a good idea to generate this kind of thing automatically,
> but I think we should take the opportunity to move the interface to the
> target structure.  I.e.:
>
>    HAVE_foo -> targetm.have_foo ()
>    gen_foo -> targetm.gen_foo ()
>
> This should move us closer to the pipedream goal of supporting multiple
> targets at once.  It should also mean that only the target code depends
> on insn-flags.h.
>
> The patch just moves return and simple_return as an example.  I have more
> locally (in order to test other code paths), but they're just an obvious
> extension of this one.
>
> The patch relies on the hashing changes in:
>
>    https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01066.html
>    https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01564.html
>
> and on this trivial patch:
>
>    https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01604.html
>
> It seems a bit heavyweight when you just look at these two instructions,
> but I think it'll be a saving in the end.
>
> Bootstrapped & regression-tested on x86_64-linux-gnu.  Also tested
> via config-list.mk.  OK to install?
>
> Thanks,
> Richard
>
>
> gcc/
> 	* Makefile.in (TARGET_DEF): Add target-insns.def.
> 	(.PRECIOUS, simple_rtl_generated_h): Add insn-target-def.h.
> 	(build/gentarget-def.o): New rule.
> 	(genprogrtl): Add target-def.
> 	* target-insns.def, gentarget-def.c: New files.
> 	* target.def: Add targetm.have_* and targetm.gen_* hooks,
> 	based on the contents of target-insns.def.
> 	* defaults.h (HAVE_simple_return, gen_simple_return): Delete.
> 	(HAVE_return, gen_return): Delete.
> 	* target-def.h: Include insn-target-def.h.
> 	* cfgrtl.c (force_nonfallthru_and_redirect): Use targetm interface
> 	instead of direct calls.  Rely on them to do the appropriate assertions.
> 	* function.c (gen_return_pattern): Likewise.  Return an rtx_insn *.
> 	(convert_jumps_to_returns): Use targetm interface instead of
> 	direct calls.
> 	(thread_prologue_and_epilogue_insns): Likewise.
> 	* reorg.c (find_end_label, dbr_schedule): Likewise.
> 	* shrink-wrap.h (SHRINK_WRAPPING_ENABLED): Likewise.
> 	* shrink-wrap.c (convert_to_simple_return): Likewise.
> 	(try_shrink_wrapping): Use SHRINK_WRAPPING_ENABLED.
As you're the gen* maintainer, I'm going to assume you got those bits 
right and if there's a problem you'll address them :-)

This is fine once the prereqs are installed.  Follow-ups to move more 
insns to this style are pre-approved assuming their of a similar 
mechanical nature.

jeff
>

Re: Add .def file for public target instructions

[H.J. Lu]

On Tue, Jun 23, 2015 at 11:41 AM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> [A fair bit later than promised, sorry...]
>
> Mikhail posted a patch to make genflags generate the default HAVE_foo
> and gen_foo definitions that have recently been added to defaults.h:
>
>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00723.html
>
> I agree it'd be a good idea to generate this kind of thing automatically,
> but I think we should take the opportunity to move the interface to the
> target structure.  I.e.:
>
>   HAVE_foo -> targetm.have_foo ()
>   gen_foo -> targetm.gen_foo ()
>
> This should move us closer to the pipedream goal of supporting multiple
> targets at once.  It should also mean that only the target code depends
> on insn-flags.h.
>
> The patch just moves return and simple_return as an example.  I have more
> locally (in order to test other code paths), but they're just an obvious
> extension of this one.
>
> The patch relies on the hashing changes in:
>
>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01066.html
>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01564.html
>
> and on this trivial patch:
>
>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01604.html
>
> It seems a bit heavyweight when you just look at these two instructions,
> but I think it'll be a saving in the end.
>
> Bootstrapped & regression-tested on x86_64-linux-gnu.  Also tested
> via config-list.mk.  OK to install?
>
> Thanks,
> Richard
>
>
> gcc/
>         * Makefile.in (TARGET_DEF): Add target-insns.def.
>         (.PRECIOUS, simple_rtl_generated_h): Add insn-target-def.h.
>         (build/gentarget-def.o): New rule.
>         (genprogrtl): Add target-def.
>         * target-insns.def, gentarget-def.c: New files.
>         * target.def: Add targetm.have_* and targetm.gen_* hooks,
>         based on the contents of target-insns.def.
>         * defaults.h (HAVE_simple_return, gen_simple_return): Delete.
>         (HAVE_return, gen_return): Delete.
>         * target-def.h: Include insn-target-def.h.
>         * cfgrtl.c (force_nonfallthru_and_redirect): Use targetm interface
>         instead of direct calls.  Rely on them to do the appropriate assertions.
>         * function.c (gen_return_pattern): Likewise.  Return an rtx_insn *.
>         (convert_jumps_to_returns): Use targetm interface instead of
>         direct calls.
>         (thread_prologue_and_epilogue_insns): Likewise.
>         * reorg.c (find_end_label, dbr_schedule): Likewise.
>         * shrink-wrap.h (SHRINK_WRAPPING_ENABLED): Likewise.
>         * shrink-wrap.c (convert_to_simple_return): Likewise.
>         (try_shrink_wrapping): Use SHRINK_WRAPPING_ENABLED.
>

This breaks bootstrap on Linux/ia32:

https://gcc.gnu.org/ml/gcc-regression/2015-06/msg00649.html

../../src-trunk/gcc/gentarget-def.c: In function âvoid
def_target_insn(const char*, const char*)â:
../../src-trunk/gcc/gentarget-def.c:88:34: error: comparison between
signed and unsigned integer expressions [-Werror=sign-compare]
  if (strtol (p + 1, &endptr, 10) != opno

-- 
H.J.

Re: Add .def file for public target instructions

[H.J. Lu]

On Thu, Jun 25, 2015 at 1:09 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Jun 23, 2015 at 11:41 AM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> [A fair bit later than promised, sorry...]
>>
>> Mikhail posted a patch to make genflags generate the default HAVE_foo
>> and gen_foo definitions that have recently been added to defaults.h:
>>
>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00723.html
>>
>> I agree it'd be a good idea to generate this kind of thing automatically,
>> but I think we should take the opportunity to move the interface to the
>> target structure.  I.e.:
>>
>>   HAVE_foo -> targetm.have_foo ()
>>   gen_foo -> targetm.gen_foo ()
>>
>> This should move us closer to the pipedream goal of supporting multiple
>> targets at once.  It should also mean that only the target code depends
>> on insn-flags.h.
>>
>> The patch just moves return and simple_return as an example.  I have more
>> locally (in order to test other code paths), but they're just an obvious
>> extension of this one.
>>
>> The patch relies on the hashing changes in:
>>
>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01066.html
>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01564.html
>>
>> and on this trivial patch:
>>
>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01604.html
>>
>> It seems a bit heavyweight when you just look at these two instructions,
>> but I think it'll be a saving in the end.
>>
>> Bootstrapped & regression-tested on x86_64-linux-gnu.  Also tested
>> via config-list.mk.  OK to install?
>>
>> Thanks,
>> Richard
>>
>>
>> gcc/
>>         * Makefile.in (TARGET_DEF): Add target-insns.def.
>>         (.PRECIOUS, simple_rtl_generated_h): Add insn-target-def.h.
>>         (build/gentarget-def.o): New rule.
>>         (genprogrtl): Add target-def.
>>         * target-insns.def, gentarget-def.c: New files.
>>         * target.def: Add targetm.have_* and targetm.gen_* hooks,
>>         based on the contents of target-insns.def.
>>         * defaults.h (HAVE_simple_return, gen_simple_return): Delete.
>>         (HAVE_return, gen_return): Delete.
>>         * target-def.h: Include insn-target-def.h.
>>         * cfgrtl.c (force_nonfallthru_and_redirect): Use targetm interface
>>         instead of direct calls.  Rely on them to do the appropriate assertions.
>>         * function.c (gen_return_pattern): Likewise.  Return an rtx_insn *.
>>         (convert_jumps_to_returns): Use targetm interface instead of
>>         direct calls.
>>         (thread_prologue_and_epilogue_insns): Likewise.
>>         * reorg.c (find_end_label, dbr_schedule): Likewise.
>>         * shrink-wrap.h (SHRINK_WRAPPING_ENABLED): Likewise.
>>         * shrink-wrap.c (convert_to_simple_return): Likewise.
>>         (try_shrink_wrapping): Use SHRINK_WRAPPING_ENABLED.
>>
>
> This breaks bootstrap on Linux/ia32:
>
> https://gcc.gnu.org/ml/gcc-regression/2015-06/msg00649.html
>
> ../../src-trunk/gcc/gentarget-def.c: In function âvoid
> def_target_insn(const char*, const char*)â:
> ../../src-trunk/gcc/gentarget-def.c:88:34: error: comparison between
> signed and unsigned integer expressions [-Werror=sign-compare]
>   if (strtol (p + 1, &endptr, 10) != opno
>

There are

  unsigned int opno = 0;
  for (const char *p = prototype; *p; ++p)
    if (*p == 'x' && ISDIGIT (p[1]))
      {
        /* This should be a parameter name of the form "x<OPNO>".
           That doesn't contribute to the suffix, so skip ahead and
           process the following character.  */
        char *endptr;
        if (strtol (p + 1, &endptr, 10) != opno
            || (*endptr != ',' && *endptr != ')'))

strtol returns long int.  Somehow, there is no warning on x86-64.

-- 
H.J.

Re: Add .def file for public target instructions

[Andrew Pinski]

On Thu, Jun 25, 2015 at 3:57 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Jun 25, 2015 at 1:09 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Tue, Jun 23, 2015 at 11:41 AM, Richard Sandiford
>> <rdsandiford@googlemail.com> wrote:
>>> [A fair bit later than promised, sorry...]
>>>
>>> Mikhail posted a patch to make genflags generate the default HAVE_foo
>>> and gen_foo definitions that have recently been added to defaults.h:
>>>
>>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00723.html
>>>
>>> I agree it'd be a good idea to generate this kind of thing automatically,
>>> but I think we should take the opportunity to move the interface to the
>>> target structure.  I.e.:
>>>
>>>   HAVE_foo -> targetm.have_foo ()
>>>   gen_foo -> targetm.gen_foo ()
>>>
>>> This should move us closer to the pipedream goal of supporting multiple
>>> targets at once.  It should also mean that only the target code depends
>>> on insn-flags.h.
>>>
>>> The patch just moves return and simple_return as an example.  I have more
>>> locally (in order to test other code paths), but they're just an obvious
>>> extension of this one.
>>>
>>> The patch relies on the hashing changes in:
>>>
>>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01066.html
>>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01564.html
>>>
>>> and on this trivial patch:
>>>
>>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01604.html
>>>
>>> It seems a bit heavyweight when you just look at these two instructions,
>>> but I think it'll be a saving in the end.
>>>
>>> Bootstrapped & regression-tested on x86_64-linux-gnu.  Also tested
>>> via config-list.mk.  OK to install?
>>>
>>> Thanks,
>>> Richard
>>>
>>>
>>> gcc/
>>>         * Makefile.in (TARGET_DEF): Add target-insns.def.
>>>         (.PRECIOUS, simple_rtl_generated_h): Add insn-target-def.h.
>>>         (build/gentarget-def.o): New rule.
>>>         (genprogrtl): Add target-def.
>>>         * target-insns.def, gentarget-def.c: New files.
>>>         * target.def: Add targetm.have_* and targetm.gen_* hooks,
>>>         based on the contents of target-insns.def.
>>>         * defaults.h (HAVE_simple_return, gen_simple_return): Delete.
>>>         (HAVE_return, gen_return): Delete.
>>>         * target-def.h: Include insn-target-def.h.
>>>         * cfgrtl.c (force_nonfallthru_and_redirect): Use targetm interface
>>>         instead of direct calls.  Rely on them to do the appropriate assertions.
>>>         * function.c (gen_return_pattern): Likewise.  Return an rtx_insn *.
>>>         (convert_jumps_to_returns): Use targetm interface instead of
>>>         direct calls.
>>>         (thread_prologue_and_epilogue_insns): Likewise.
>>>         * reorg.c (find_end_label, dbr_schedule): Likewise.
>>>         * shrink-wrap.h (SHRINK_WRAPPING_ENABLED): Likewise.
>>>         * shrink-wrap.c (convert_to_simple_return): Likewise.
>>>         (try_shrink_wrapping): Use SHRINK_WRAPPING_ENABLED.
>>>
>>
>> This breaks bootstrap on Linux/ia32:
>>
>> https://gcc.gnu.org/ml/gcc-regression/2015-06/msg00649.html
>>
>> ../../src-trunk/gcc/gentarget-def.c: In function âvoid
>> def_target_insn(const char*, const char*)â:
>> ../../src-trunk/gcc/gentarget-def.c:88:34: error: comparison between
>> signed and unsigned integer expressions [-Werror=sign-compare]
>>   if (strtol (p + 1, &endptr, 10) != opno
>>
>
> There are
>
>   unsigned int opno = 0;
>   for (const char *p = prototype; *p; ++p)
>     if (*p == 'x' && ISDIGIT (p[1]))
>       {
>         /* This should be a parameter name of the form "x<OPNO>".
>            That doesn't contribute to the suffix, so skip ahead and
>            process the following character.  */
>         char *endptr;
>         if (strtol (p + 1, &endptr, 10) != opno
>             || (*endptr != ',' && *endptr != ')'))
>
> strtol returns long int.  Somehow, there is no warning on x86-64.

Because on x86_64 (and all LP64 targets), the comparison gets promoted
to long (which is 64bit) so the conversion from unsigned int to long
does not lose precision.

Thanks,
Andrew

>
> --
> H.J.

Re: Add .def file for public target instructions

[H.J. Lu]

On Thu, Jun 25, 2015 at 4:00 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Thu, Jun 25, 2015 at 3:57 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Thu, Jun 25, 2015 at 1:09 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Tue, Jun 23, 2015 at 11:41 AM, Richard Sandiford
>>> <rdsandiford@googlemail.com> wrote:
>>>> [A fair bit later than promised, sorry...]
>>>>
>>>> Mikhail posted a patch to make genflags generate the default HAVE_foo
>>>> and gen_foo definitions that have recently been added to defaults.h:
>>>>
>>>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00723.html
>>>>
>>>> I agree it'd be a good idea to generate this kind of thing automatically,
>>>> but I think we should take the opportunity to move the interface to the
>>>> target structure.  I.e.:
>>>>
>>>>   HAVE_foo -> targetm.have_foo ()
>>>>   gen_foo -> targetm.gen_foo ()
>>>>
>>>> This should move us closer to the pipedream goal of supporting multiple
>>>> targets at once.  It should also mean that only the target code depends
>>>> on insn-flags.h.
>>>>
>>>> The patch just moves return and simple_return as an example.  I have more
>>>> locally (in order to test other code paths), but they're just an obvious
>>>> extension of this one.
>>>>
>>>> The patch relies on the hashing changes in:
>>>>
>>>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01066.html
>>>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01564.html
>>>>
>>>> and on this trivial patch:
>>>>
>>>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01604.html
>>>>
>>>> It seems a bit heavyweight when you just look at these two instructions,
>>>> but I think it'll be a saving in the end.
>>>>
>>>> Bootstrapped & regression-tested on x86_64-linux-gnu.  Also tested
>>>> via config-list.mk.  OK to install?
>>>>
>>>> Thanks,
>>>> Richard
>>>>
>>>>
>>>> gcc/
>>>>         * Makefile.in (TARGET_DEF): Add target-insns.def.
>>>>         (.PRECIOUS, simple_rtl_generated_h): Add insn-target-def.h.
>>>>         (build/gentarget-def.o): New rule.
>>>>         (genprogrtl): Add target-def.
>>>>         * target-insns.def, gentarget-def.c: New files.
>>>>         * target.def: Add targetm.have_* and targetm.gen_* hooks,
>>>>         based on the contents of target-insns.def.
>>>>         * defaults.h (HAVE_simple_return, gen_simple_return): Delete.
>>>>         (HAVE_return, gen_return): Delete.
>>>>         * target-def.h: Include insn-target-def.h.
>>>>         * cfgrtl.c (force_nonfallthru_and_redirect): Use targetm interface
>>>>         instead of direct calls.  Rely on them to do the appropriate assertions.
>>>>         * function.c (gen_return_pattern): Likewise.  Return an rtx_insn *.
>>>>         (convert_jumps_to_returns): Use targetm interface instead of
>>>>         direct calls.
>>>>         (thread_prologue_and_epilogue_insns): Likewise.
>>>>         * reorg.c (find_end_label, dbr_schedule): Likewise.
>>>>         * shrink-wrap.h (SHRINK_WRAPPING_ENABLED): Likewise.
>>>>         * shrink-wrap.c (convert_to_simple_return): Likewise.
>>>>         (try_shrink_wrapping): Use SHRINK_WRAPPING_ENABLED.
>>>>
>>>
>>> This breaks bootstrap on Linux/ia32:
>>>
>>> https://gcc.gnu.org/ml/gcc-regression/2015-06/msg00649.html
>>>
>>> ../../src-trunk/gcc/gentarget-def.c: In function âvoid
>>> def_target_insn(const char*, const char*)â:
>>> ../../src-trunk/gcc/gentarget-def.c:88:34: error: comparison between
>>> signed and unsigned integer expressions [-Werror=sign-compare]
>>>   if (strtol (p + 1, &endptr, 10) != opno
>>>
>>
>> There are
>>
>>   unsigned int opno = 0;
>>   for (const char *p = prototype; *p; ++p)
>>     if (*p == 'x' && ISDIGIT (p[1]))
>>       {
>>         /* This should be a parameter name of the form "x<OPNO>".
>>            That doesn't contribute to the suffix, so skip ahead and
>>            process the following character.  */
>>         char *endptr;
>>         if (strtol (p + 1, &endptr, 10) != opno
>>             || (*endptr != ',' && *endptr != ')'))
>>
>> strtol returns long int.  Somehow, there is no warning on x86-64.
>
> Because on x86_64 (and all LP64 targets), the comparison gets promoted
> to long (which is 64bit) so the conversion from unsigned int to long
> does not lose precision.
>

I am testing this.


-- 
H.J.
---
diff --git a/gcc/gentarget-def.c b/gcc/gentarget-def.c
index d4839e8..3ca9cfd 100644
--- a/gcc/gentarget-def.c
+++ b/gcc/gentarget-def.c
@@ -77,7 +77,7 @@ def_target_insn (const char *name, const char *prototype)
      together to get a suffix.  */
   char *suffix = XALLOCAVEC (char, strlen (prototype) + 1);
   i = 0;
-  unsigned int opno = 0;
+  long opno = 0;
   for (const char *p = prototype; *p; ++p)
     if (*p == 'x' && ISDIGIT (p[1]))
       {

Re: Add .def file for public target instructions

[H.J. Lu]

On Thu, Jun 25, 2015 at 4:37 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Jun 25, 2015 at 4:00 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Thu, Jun 25, 2015 at 3:57 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Thu, Jun 25, 2015 at 1:09 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Tue, Jun 23, 2015 at 11:41 AM, Richard Sandiford
>>>> <rdsandiford@googlemail.com> wrote:
>>>>> [A fair bit later than promised, sorry...]
>>>>>
>>>>> Mikhail posted a patch to make genflags generate the default HAVE_foo
>>>>> and gen_foo definitions that have recently been added to defaults.h:
>>>>>
>>>>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00723.html
>>>>>
>>>>> I agree it'd be a good idea to generate this kind of thing automatically,
>>>>> but I think we should take the opportunity to move the interface to the
>>>>> target structure.  I.e.:
>>>>>
>>>>>   HAVE_foo -> targetm.have_foo ()
>>>>>   gen_foo -> targetm.gen_foo ()
>>>>>
>>>>> This should move us closer to the pipedream goal of supporting multiple
>>>>> targets at once.  It should also mean that only the target code depends
>>>>> on insn-flags.h.
>>>>>
>>>>> The patch just moves return and simple_return as an example.  I have more
>>>>> locally (in order to test other code paths), but they're just an obvious
>>>>> extension of this one.
>>>>>
>>>>> The patch relies on the hashing changes in:
>>>>>
>>>>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01066.html
>>>>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01564.html
>>>>>
>>>>> and on this trivial patch:
>>>>>
>>>>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01604.html
>>>>>
>>>>> It seems a bit heavyweight when you just look at these two instructions,
>>>>> but I think it'll be a saving in the end.
>>>>>
>>>>> Bootstrapped & regression-tested on x86_64-linux-gnu.  Also tested
>>>>> via config-list.mk.  OK to install?
>>>>>
>>>>> Thanks,
>>>>> Richard
>>>>>
>>>>>
>>>>> gcc/
>>>>>         * Makefile.in (TARGET_DEF): Add target-insns.def.
>>>>>         (.PRECIOUS, simple_rtl_generated_h): Add insn-target-def.h.
>>>>>         (build/gentarget-def.o): New rule.
>>>>>         (genprogrtl): Add target-def.
>>>>>         * target-insns.def, gentarget-def.c: New files.
>>>>>         * target.def: Add targetm.have_* and targetm.gen_* hooks,
>>>>>         based on the contents of target-insns.def.
>>>>>         * defaults.h (HAVE_simple_return, gen_simple_return): Delete.
>>>>>         (HAVE_return, gen_return): Delete.
>>>>>         * target-def.h: Include insn-target-def.h.
>>>>>         * cfgrtl.c (force_nonfallthru_and_redirect): Use targetm interface
>>>>>         instead of direct calls.  Rely on them to do the appropriate assertions.
>>>>>         * function.c (gen_return_pattern): Likewise.  Return an rtx_insn *.
>>>>>         (convert_jumps_to_returns): Use targetm interface instead of
>>>>>         direct calls.
>>>>>         (thread_prologue_and_epilogue_insns): Likewise.
>>>>>         * reorg.c (find_end_label, dbr_schedule): Likewise.
>>>>>         * shrink-wrap.h (SHRINK_WRAPPING_ENABLED): Likewise.
>>>>>         * shrink-wrap.c (convert_to_simple_return): Likewise.
>>>>>         (try_shrink_wrapping): Use SHRINK_WRAPPING_ENABLED.
>>>>>
>>>>
>>>> This breaks bootstrap on Linux/ia32:
>>>>
>>>> https://gcc.gnu.org/ml/gcc-regression/2015-06/msg00649.html
>>>>
>>>> ../../src-trunk/gcc/gentarget-def.c: In function âvoid
>>>> def_target_insn(const char*, const char*)â:
>>>> ../../src-trunk/gcc/gentarget-def.c:88:34: error: comparison between
>>>> signed and unsigned integer expressions [-Werror=sign-compare]
>>>>   if (strtol (p + 1, &endptr, 10) != opno
>>>>
>>>
>>> There are
>>>
>>>   unsigned int opno = 0;
>>>   for (const char *p = prototype; *p; ++p)
>>>     if (*p == 'x' && ISDIGIT (p[1]))
>>>       {
>>>         /* This should be a parameter name of the form "x<OPNO>".
>>>            That doesn't contribute to the suffix, so skip ahead and
>>>            process the following character.  */
>>>         char *endptr;
>>>         if (strtol (p + 1, &endptr, 10) != opno
>>>             || (*endptr != ',' && *endptr != ')'))
>>>
>>> strtol returns long int.  Somehow, there is no warning on x86-64.
>>
>> Because on x86_64 (and all LP64 targets), the comparison gets promoted
>> to long (which is 64bit) so the conversion from unsigned int to long
>> does not lose precision.
>>
>
> I am testing this.
>
>
> --
> H.J.
> ---
> diff --git a/gcc/gentarget-def.c b/gcc/gentarget-def.c
> index d4839e8..3ca9cfd 100644
> --- a/gcc/gentarget-def.c
> +++ b/gcc/gentarget-def.c
> @@ -77,7 +77,7 @@ def_target_insn (const char *name, const char *prototype)
>       together to get a suffix.  */
>    char *suffix = XALLOCAVEC (char, strlen (prototype) + 1);
>    i = 0;
> -  unsigned int opno = 0;
> +  long opno = 0;
>    for (const char *p = prototype; *p; ++p)
>      if (*p == 'x' && ISDIGIT (p[1]))
>        {

It doesn't work.  I checked in this patch instead.


-- 
H.J.
---
Index: ChangeLog
===================================================================
--- ChangeLog (revision 224992)
+++ ChangeLog (working copy)
@@ -1,3 +1,8 @@
+2015-06-25  H.J. Lu  <hongjiu.lu@intel.com>
+
+ * gentarget-def.c (def_target_insn): Cast return of strtol to
+ unsigned int.
+
 2015-06-25  Andrew MacLeod  <amacleod@redhat.com>

  * gimple.h (gimple_call_set_fn): Move inline function.
Index: gentarget-def.c
===================================================================
--- gentarget-def.c (revision 224992)
+++ gentarget-def.c (working copy)
@@ -85,7 +85,7 @@ def_target_insn (const char *name, const
    That doesn't contribute to the suffix, so skip ahead and
    process the following character.  */
  char *endptr;
- if (strtol (p + 1, &endptr, 10) != opno
+ if ((unsigned int) strtol (p + 1, &endptr, 10) != opno
     || (*endptr != ',' && *endptr != ')'))
   {
     error ("invalid prototype for '%s'", name);

Re: Add .def file for public target instructions

[Markus Trippelsdorf]

On 2015.06.23 at 19:41 +0100, Richard Sandiford wrote:
> 
> gcc/
> 	* Makefile.in (TARGET_DEF): Add target-insns.def.
> 	(.PRECIOUS, simple_rtl_generated_h): Add insn-target-def.h.
> 	(build/gentarget-def.o): New rule.
> 	(genprogrtl): Add target-def.
> 	* target-insns.def, gentarget-def.c: New files.
> 	* target.def: Add targetm.have_* and targetm.gen_* hooks,
> 	based on the contents of target-insns.def.
> 	* defaults.h (HAVE_simple_return, gen_simple_return): Delete.
> 	(HAVE_return, gen_return): Delete.
> 	* target-def.h: Include insn-target-def.h.
> 	* cfgrtl.c (force_nonfallthru_and_redirect): Use targetm interface
> 	instead of direct calls.  Rely on them to do the appropriate assertions.
> 	* function.c (gen_return_pattern): Likewise.  Return an rtx_insn *.
> 	(convert_jumps_to_returns): Use targetm interface instead of
> 	direct calls.
> 	(thread_prologue_and_epilogue_insns): Likewise.
> 	* reorg.c (find_end_label, dbr_schedule): Likewise.
> 	* shrink-wrap.h (SHRINK_WRAPPING_ENABLED): Likewise.
> 	* shrink-wrap.c (convert_to_simple_return): Likewise.
> 	(try_shrink_wrapping): Use SHRINK_WRAPPING_ENABLED.

The patch breaks bootstrap on ppc64le. During libgcc configuration:

conftest.c: In function 'main':
conftest.c:16:1: internal compiler error: in as_a, at is-a.h:192
 }
 ^
0x1010411b as_a<rtx_jump_insn*, rtx_insn>
        ../../gcc/gcc/is-a.h:192
0x1040ccd3 as_a<rtx_jump_insn*, rtx_insn>
        ../../gcc/gcc/emit-rtl.c:4750
0x1040ccd3 emit_jump_insn_after(rtx_def*, rtx_def*)
        ../../gcc/gcc/emit-rtl.c:4749
0x104c378f emit_return_into_block(bool, basic_block_def*)
        ../../gcc/gcc/function.c:5633
0x104c3ee7 emit_return_for_exit(edge_def*, bool)
        ../../gcc/gcc/function.c:5779
0x104c9ee3 thread_prologue_and_epilogue_insns()
        ../../gcc/gcc/function.c:5961
0x104ca39f rest_of_handle_thread_prologue_and_epilogue
        ../../gcc/gcc/function.c:6443
0x104ca39f execute
        ../../gcc/gcc/function.c:6481
Please submit a full bug report,
with preprocessed source if appropriate.



-- 
Markus

Re: Add .def file for public target instructions

[Richard Sandiford]

Markus Trippelsdorf <markus@trippelsdorf.de> writes:
> On 2015.06.23 at 19:41 +0100, Richard Sandiford wrote:
>> 
>> gcc/
>> 	* Makefile.in (TARGET_DEF): Add target-insns.def.
>> 	(.PRECIOUS, simple_rtl_generated_h): Add insn-target-def.h.
>> 	(build/gentarget-def.o): New rule.
>> 	(genprogrtl): Add target-def.
>> 	* target-insns.def, gentarget-def.c: New files.
>> 	* target.def: Add targetm.have_* and targetm.gen_* hooks,
>> 	based on the contents of target-insns.def.
>> 	* defaults.h (HAVE_simple_return, gen_simple_return): Delete.
>> 	(HAVE_return, gen_return): Delete.
>> 	* target-def.h: Include insn-target-def.h.
>> 	* cfgrtl.c (force_nonfallthru_and_redirect): Use targetm interface
>> 	instead of direct calls.  Rely on them to do the appropriate assertions.
>> 	* function.c (gen_return_pattern): Likewise.  Return an rtx_insn *.
>> 	(convert_jumps_to_returns): Use targetm interface instead of
>> 	direct calls.
>> 	(thread_prologue_and_epilogue_insns): Likewise.
>> 	* reorg.c (find_end_label, dbr_schedule): Likewise.
>> 	* shrink-wrap.h (SHRINK_WRAPPING_ENABLED): Likewise.
>> 	* shrink-wrap.c (convert_to_simple_return): Likewise.
>> 	(try_shrink_wrapping): Use SHRINK_WRAPPING_ENABLED.
>
> The patch breaks bootstrap on ppc64le. During libgcc configuration:

Yeah, sorry about that.  The problem is due to confusion about whether
the generator should emit a barrier or not.  After the final instruction
in a sequence, it should always be up to the caller to insert any
necessary barriers, just like it is for define_insns, and for define_expands
implemented in C++ with DONE.

I'm testing the following patch.

Thanks,
Richard


gcc/
	* rtl.h (emit): Add an optional boolean parameter to control
	whether barriers are emitted.
	* emit-rtl.c (emit): Likewise.
	* gensupport.c (get_emit_function): Return null rather than "emit".
	* genemit.c (gen_emit_seq): Handle the null return value.
	Don't emit barriers after the final instruction in the sequence.
	* gentarget-def.c (main): Don't emit barriers after the instruction.

Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	2015-06-26 09:09:10.128602409 +0100
+++ gcc/rtl.h	2015-06-26 09:09:10.316600233 +0100
@@ -3494,7 +3494,7 @@ extern void add_insn (rtx_insn *);
 extern void add_insn_before (rtx, rtx, basic_block);
 extern void add_insn_after (rtx, rtx, basic_block);
 extern void remove_insn (rtx);
-extern rtx_insn *emit (rtx);
+extern rtx_insn *emit (rtx, bool = true);
 extern void emit_insn_at_entry (rtx);
 extern rtx gen_lowpart_SUBREG (machine_mode, rtx);
 extern rtx gen_const_mem (machine_mode, rtx);
Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c	2015-06-26 09:09:10.124602455 +0100
+++ gcc/emit-rtl.c	2015-06-26 09:09:10.316600233 +0100
@@ -5303,11 +5303,14 @@ set_dst_reg_note (rtx insn, enum reg_not
   return NULL_RTX;
 }
 \f
-/* Emit the rtl pattern X as an appropriate kind of insn.
+/* Emit the rtl pattern X as an appropriate kind of insn.  Also emit a
+   following barrier if the instruction needs one and if ALLOW_BARRIER_P
+   is true.
+
    If X is a label, it is simply added into the insn chain.  */
 
 rtx_insn *
-emit (rtx x)
+emit (rtx x, bool allow_barrier_p)
 {
   enum rtx_code code = classify_insn (x);
 
@@ -5320,7 +5323,8 @@ emit (rtx x)
     case  JUMP_INSN:
       {
 	rtx_insn *insn = emit_jump_insn (x);
-	if (any_uncondjump_p (insn) || GET_CODE (x) == RETURN)
+	if (allow_barrier_p
+	    && (any_uncondjump_p (insn) || GET_CODE (x) == RETURN))
 	  return emit_barrier ();
 	return insn;
       }
Index: gcc/gensupport.c
===================================================================
--- gcc/gensupport.c	2015-06-26 09:09:10.124602455 +0100
+++ gcc/gensupport.c	2015-06-26 09:09:10.316600233 +0100
@@ -2983,7 +2983,9 @@ get_pattern_stats (struct pattern_stats
 				       stats->max_scratch_opno)) + 1;
 }
 
-/* Return the emit_* function that should be used for pattern X.  */
+/* Return the emit_* function that should be used for pattern X, or NULL
+   if we can't pick a particular type at compile time and should instead
+   fall back to "emit".  */
 
 const char *
 get_emit_function (rtx x)
@@ -3000,7 +3002,7 @@ get_emit_function (rtx x)
       return "emit_jump_insn";
 
     case UNKNOWN:
-      return "emit";
+      return NULL;
 
     default:
       gcc_unreachable ();
Index: gcc/genemit.c
===================================================================
--- gcc/genemit.c	2015-06-26 09:09:10.124602455 +0100
+++ gcc/genemit.c	2015-06-26 09:09:10.316600233 +0100
@@ -277,12 +277,22 @@ gen_emit_seq (rtvec vec, char *used)
 {
   for (int i = 0, len = GET_NUM_ELEM (vec); i < len; ++i)
     {
+      bool last_p = (i == len - 1);
       rtx next = RTVEC_ELT (vec, i);
-      printf ("  %s (", get_emit_function (next));
-      gen_exp (next, DEFINE_EXPAND, used);
-      printf (");\n");
-      if (needs_barrier_p (next))
-	printf ("  emit_barrier ();");
+      if (const char *name = get_emit_function (next))
+	{
+	  printf ("  %s (", name);
+	  gen_exp (next, DEFINE_EXPAND, used);
+	  printf (");\n");
+	  if (!last_p && needs_barrier_p (next))
+	    printf ("  emit_barrier ();");
+	}
+      else
+	{
+	  printf ("  emit (");
+	  gen_exp (next, DEFINE_EXPAND, used);
+	  printf (", %s);\n", last_p ? "false" : "true");
+	}
     }
 }
 \f
Index: gcc/gentarget-def.c
===================================================================
--- gcc/gentarget-def.c	2015-06-26 09:09:10.124602455 +0100
+++ gcc/gentarget-def.c	2015-06-26 09:09:10.316600233 +0100
@@ -245,7 +245,7 @@ main (int argc, char **argv)
   printf ("  if (rtx_insn *insn = dyn_cast <rtx_insn *> (x))\n");
   printf ("    return insn;\n");
   printf ("  start_sequence ();\n");
-  printf ("  emit (x);\n");
+  printf ("  emit (x, false);\n");
   printf ("  rtx_insn *res = get_insns ();\n");
   printf ("  end_sequence ();\n");
   printf ("  return res;\n");

Re: Add .def file for public target instructions

[Richard Sandiford]

"H.J. Lu" <hjl.tools@gmail.com> writes:
> On Thu, Jun 25, 2015 at 4:37 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Thu, Jun 25, 2015 at 4:00 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>> On Thu, Jun 25, 2015 at 3:57 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Thu, Jun 25, 2015 at 1:09 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> On Tue, Jun 23, 2015 at 11:41 AM, Richard Sandiford
>>>>> <rdsandiford@googlemail.com> wrote:
>>>>>> [A fair bit later than promised, sorry...]
>>>>>>
>>>>>> Mikhail posted a patch to make genflags generate the default HAVE_foo
>>>>>> and gen_foo definitions that have recently been added to defaults.h:
>>>>>>
>>>>>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00723.html
>>>>>>
>>>>>> I agree it'd be a good idea to generate this kind of thing automatically,
>>>>>> but I think we should take the opportunity to move the interface to the
>>>>>> target structure.  I.e.:
>>>>>>
>>>>>>   HAVE_foo -> targetm.have_foo ()
>>>>>>   gen_foo -> targetm.gen_foo ()
>>>>>>
>>>>>> This should move us closer to the pipedream goal of supporting multiple
>>>>>> targets at once.  It should also mean that only the target code depends
>>>>>> on insn-flags.h.
>>>>>>
>>>>>> The patch just moves return and simple_return as an example.  I have more
>>>>>> locally (in order to test other code paths), but they're just an obvious
>>>>>> extension of this one.
>>>>>>
>>>>>> The patch relies on the hashing changes in:
>>>>>>
>>>>>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01066.html
>>>>>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01564.html
>>>>>>
>>>>>> and on this trivial patch:
>>>>>>
>>>>>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01604.html
>>>>>>
>>>>>> It seems a bit heavyweight when you just look at these two instructions,
>>>>>> but I think it'll be a saving in the end.
>>>>>>
>>>>>> Bootstrapped & regression-tested on x86_64-linux-gnu.  Also tested
>>>>>> via config-list.mk.  OK to install?
>>>>>>
>>>>>> Thanks,
>>>>>> Richard
>>>>>>
>>>>>>
>>>>>> gcc/
>>>>>>         * Makefile.in (TARGET_DEF): Add target-insns.def.
>>>>>>         (.PRECIOUS, simple_rtl_generated_h): Add insn-target-def.h.
>>>>>>         (build/gentarget-def.o): New rule.
>>>>>>         (genprogrtl): Add target-def.
>>>>>>         * target-insns.def, gentarget-def.c: New files.
>>>>>>         * target.def: Add targetm.have_* and targetm.gen_* hooks,
>>>>>>         based on the contents of target-insns.def.
>>>>>>         * defaults.h (HAVE_simple_return, gen_simple_return): Delete.
>>>>>>         (HAVE_return, gen_return): Delete.
>>>>>>         * target-def.h: Include insn-target-def.h.
>>>>>>         * cfgrtl.c (force_nonfallthru_and_redirect): Use targetm interface
>>>>>>         instead of direct calls.  Rely on them to do the appropriate assertions.
>>>>>>         * function.c (gen_return_pattern): Likewise.  Return an rtx_insn *.
>>>>>>         (convert_jumps_to_returns): Use targetm interface instead of
>>>>>>         direct calls.
>>>>>>         (thread_prologue_and_epilogue_insns): Likewise.
>>>>>>         * reorg.c (find_end_label, dbr_schedule): Likewise.
>>>>>>         * shrink-wrap.h (SHRINK_WRAPPING_ENABLED): Likewise.
>>>>>>         * shrink-wrap.c (convert_to_simple_return): Likewise.
>>>>>>         (try_shrink_wrapping): Use SHRINK_WRAPPING_ENABLED.
>>>>>>
>>>>>
>>>>> This breaks bootstrap on Linux/ia32:
>>>>>
>>>>> https://gcc.gnu.org/ml/gcc-regression/2015-06/msg00649.html
>>>>>
>>>>> ../../src-trunk/gcc/gentarget-def.c: In function âvoid
>>>>> def_target_insn(const char*, const char*)â:
>>>>> ../../src-trunk/gcc/gentarget-def.c:88:34: error: comparison between
>>>>> signed and unsigned integer expressions [-Werror=sign-compare]
>>>>>   if (strtol (p + 1, &endptr, 10) != opno
>>>>>
>>>>
>>>> There are
>>>>
>>>>   unsigned int opno = 0;
>>>>   for (const char *p = prototype; *p; ++p)
>>>>     if (*p == 'x' && ISDIGIT (p[1]))
>>>>       {
>>>>         /* This should be a parameter name of the form "x<OPNO>".
>>>>            That doesn't contribute to the suffix, so skip ahead and
>>>>            process the following character.  */
>>>>         char *endptr;
>>>>         if (strtol (p + 1, &endptr, 10) != opno
>>>>             || (*endptr != ',' && *endptr != ')'))
>>>>
>>>> strtol returns long int.  Somehow, there is no warning on x86-64.
>>>
>>> Because on x86_64 (and all LP64 targets), the comparison gets promoted
>>> to long (which is 64bit) so the conversion from unsigned int to long
>>> does not lose precision.
>>>
>>
>> I am testing this.
>>
>>
>> --
>> H.J.
>> ---
>> diff --git a/gcc/gentarget-def.c b/gcc/gentarget-def.c
>> index d4839e8..3ca9cfd 100644
>> --- a/gcc/gentarget-def.c
>> +++ b/gcc/gentarget-def.c
>> @@ -77,7 +77,7 @@ def_target_insn (const char *name, const char *prototype)
>>       together to get a suffix.  */
>>    char *suffix = XALLOCAVEC (char, strlen (prototype) + 1);
>>    i = 0;
>> -  unsigned int opno = 0;
>> +  long opno = 0;
>>    for (const char *p = prototype; *p; ++p)
>>      if (*p == 'x' && ISDIGIT (p[1]))
>>        {
>
> It doesn't work.  I checked in this patch instead.

Thanks H.J., and sorry for the breakage.

Richard

Re: Add .def file for public target instructions

[Richard Sandiford]

Richard Sandiford <richard.sandiford@arm.com> writes:
> Markus Trippelsdorf <markus@trippelsdorf.de> writes:
>> On 2015.06.23 at 19:41 +0100, Richard Sandiford wrote:
>>> 
>>> gcc/
>>> 	* Makefile.in (TARGET_DEF): Add target-insns.def.
>>> 	(.PRECIOUS, simple_rtl_generated_h): Add insn-target-def.h.
>>> 	(build/gentarget-def.o): New rule.
>>> 	(genprogrtl): Add target-def.
>>> 	* target-insns.def, gentarget-def.c: New files.
>>> 	* target.def: Add targetm.have_* and targetm.gen_* hooks,
>>> 	based on the contents of target-insns.def.
>>> 	* defaults.h (HAVE_simple_return, gen_simple_return): Delete.
>>> 	(HAVE_return, gen_return): Delete.
>>> 	* target-def.h: Include insn-target-def.h.
>>> 	* cfgrtl.c (force_nonfallthru_and_redirect): Use targetm interface
>>> 	instead of direct calls.  Rely on them to do the appropriate assertions.
>>> 	* function.c (gen_return_pattern): Likewise.  Return an rtx_insn *.
>>> 	(convert_jumps_to_returns): Use targetm interface instead of
>>> 	direct calls.
>>> 	(thread_prologue_and_epilogue_insns): Likewise.
>>> 	* reorg.c (find_end_label, dbr_schedule): Likewise.
>>> 	* shrink-wrap.h (SHRINK_WRAPPING_ENABLED): Likewise.
>>> 	* shrink-wrap.c (convert_to_simple_return): Likewise.
>>> 	(try_shrink_wrapping): Use SHRINK_WRAPPING_ENABLED.
>>
>> The patch breaks bootstrap on ppc64le. During libgcc configuration:
>
> Yeah, sorry about that.  The problem is due to confusion about whether
> the generator should emit a barrier or not.  After the final instruction
> in a sequence, it should always be up to the caller to insert any
> necessary barriers, just like it is for define_insns, and for define_expands
> implemented in C++ with DONE.
>
> I'm testing the following patch.

Committed after bootstrapping & regression-testing on x86_64-linux-gnu
and testing that Anton's testcase worked on ppc64le.  (The emit-rtl.c
part seemed obvious given the generator changes.)

Sorry again for the breakage.

Thanks,
Richard

> gcc/
> 	* rtl.h (emit): Add an optional boolean parameter to control
> 	whether barriers are emitted.
> 	* emit-rtl.c (emit): Likewise.
> 	* gensupport.c (get_emit_function): Return null rather than "emit".
> 	* genemit.c (gen_emit_seq): Handle the null return value.
> 	Don't emit barriers after the final instruction in the sequence.
> 	* gentarget-def.c (main): Don't emit barriers after the instruction.
>
> Index: gcc/rtl.h
> ===================================================================
> --- gcc/rtl.h	2015-06-26 09:09:10.128602409 +0100
> +++ gcc/rtl.h	2015-06-26 09:09:10.316600233 +0100
> @@ -3494,7 +3494,7 @@ extern void add_insn (rtx_insn *);
>  extern void add_insn_before (rtx, rtx, basic_block);
>  extern void add_insn_after (rtx, rtx, basic_block);
>  extern void remove_insn (rtx);
> -extern rtx_insn *emit (rtx);
> +extern rtx_insn *emit (rtx, bool = true);
>  extern void emit_insn_at_entry (rtx);
>  extern rtx gen_lowpart_SUBREG (machine_mode, rtx);
>  extern rtx gen_const_mem (machine_mode, rtx);
> Index: gcc/emit-rtl.c
> ===================================================================
> --- gcc/emit-rtl.c	2015-06-26 09:09:10.124602455 +0100
> +++ gcc/emit-rtl.c	2015-06-26 09:09:10.316600233 +0100
> @@ -5303,11 +5303,14 @@ set_dst_reg_note (rtx insn, enum reg_not
>    return NULL_RTX;
>  }
>  \f
> -/* Emit the rtl pattern X as an appropriate kind of insn.
> +/* Emit the rtl pattern X as an appropriate kind of insn.  Also emit a
> +   following barrier if the instruction needs one and if ALLOW_BARRIER_P
> +   is true.
> +
>     If X is a label, it is simply added into the insn chain.  */
>  
>  rtx_insn *
> -emit (rtx x)
> +emit (rtx x, bool allow_barrier_p)
>  {
>    enum rtx_code code = classify_insn (x);
>  
> @@ -5320,7 +5323,8 @@ emit (rtx x)
>      case  JUMP_INSN:
>        {
>  	rtx_insn *insn = emit_jump_insn (x);
> -	if (any_uncondjump_p (insn) || GET_CODE (x) == RETURN)
> +	if (allow_barrier_p
> +	    && (any_uncondjump_p (insn) || GET_CODE (x) == RETURN))
>  	  return emit_barrier ();
>  	return insn;
>        }
> Index: gcc/gensupport.c
> ===================================================================
> --- gcc/gensupport.c	2015-06-26 09:09:10.124602455 +0100
> +++ gcc/gensupport.c	2015-06-26 09:09:10.316600233 +0100
> @@ -2983,7 +2983,9 @@ get_pattern_stats (struct pattern_stats
>  				       stats->max_scratch_opno)) + 1;
>  }
>  
> -/* Return the emit_* function that should be used for pattern X.  */
> +/* Return the emit_* function that should be used for pattern X, or NULL
> +   if we can't pick a particular type at compile time and should instead
> +   fall back to "emit".  */
>  
>  const char *
>  get_emit_function (rtx x)
> @@ -3000,7 +3002,7 @@ get_emit_function (rtx x)
>        return "emit_jump_insn";
>  
>      case UNKNOWN:
> -      return "emit";
> +      return NULL;
>  
>      default:
>        gcc_unreachable ();
> Index: gcc/genemit.c
> ===================================================================
> --- gcc/genemit.c	2015-06-26 09:09:10.124602455 +0100
> +++ gcc/genemit.c	2015-06-26 09:09:10.316600233 +0100
> @@ -277,12 +277,22 @@ gen_emit_seq (rtvec vec, char *used)
>  {
>    for (int i = 0, len = GET_NUM_ELEM (vec); i < len; ++i)
>      {
> +      bool last_p = (i == len - 1);
>        rtx next = RTVEC_ELT (vec, i);
> -      printf ("  %s (", get_emit_function (next));
> -      gen_exp (next, DEFINE_EXPAND, used);
> -      printf (");\n");
> -      if (needs_barrier_p (next))
> -	printf ("  emit_barrier ();");
> +      if (const char *name = get_emit_function (next))
> +	{
> +	  printf ("  %s (", name);
> +	  gen_exp (next, DEFINE_EXPAND, used);
> +	  printf (");\n");
> +	  if (!last_p && needs_barrier_p (next))
> +	    printf ("  emit_barrier ();");
> +	}
> +      else
> +	{
> +	  printf ("  emit (");
> +	  gen_exp (next, DEFINE_EXPAND, used);
> +	  printf (", %s);\n", last_p ? "false" : "true");
> +	}
>      }
>  }
>  \f
> Index: gcc/gentarget-def.c
> ===================================================================
> --- gcc/gentarget-def.c	2015-06-26 09:09:10.124602455 +0100
> +++ gcc/gentarget-def.c	2015-06-26 09:09:10.316600233 +0100
> @@ -245,7 +245,7 @@ main (int argc, char **argv)
>    printf ("  if (rtx_insn *insn = dyn_cast <rtx_insn *> (x))\n");
>    printf ("    return insn;\n");
>    printf ("  start_sequence ();\n");
> -  printf ("  emit (x);\n");
> +  printf ("  emit (x, false);\n");
>    printf ("  rtx_insn *res = get_insns ();\n");
>    printf ("  end_sequence ();\n");
>    printf ("  return res;\n");

Re: Add .def file for public target instructions

[Trevor Saunders]

On Tue, Jun 23, 2015 at 07:41:42PM +0100, Richard Sandiford wrote:
> [A fair bit later than promised, sorry...]
> 
> Mikhail posted a patch to make genflags generate the default HAVE_foo
> and gen_foo definitions that have recently been added to defaults.h:
> 
>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00723.html
> 
> I agree it'd be a good idea to generate this kind of thing automatically,
> but I think we should take the opportunity to move the interface to the
> target structure.  I.e.:
> 
>   HAVE_foo -> targetm.have_foo ()
>   gen_foo -> targetm.gen_foo ()
> 
> This should move us closer to the pipedream goal of supporting multiple
> targets at once.  It should also mean that only the target code depends
> on insn-flags.h.

using targetm. certainly seems like an improvement.  I wonder if it
would be faster to stick this data on a per function object.  I think
that would mean you could compute what insns are available once when the
function is created and afterwards all checks would only needed to be
reading computed values.

Trev

Re: Add .def file for public target instructions

[Richard Biener]

On Wed, Jul 1, 2015 at 11:39 AM, Trevor Saunders <tbsaunde@tbsaunde.org> wrote:
> On Tue, Jun 23, 2015 at 07:41:42PM +0100, Richard Sandiford wrote:
>> [A fair bit later than promised, sorry...]
>>
>> Mikhail posted a patch to make genflags generate the default HAVE_foo
>> and gen_foo definitions that have recently been added to defaults.h:
>>
>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00723.html
>>
>> I agree it'd be a good idea to generate this kind of thing automatically,
>> but I think we should take the opportunity to move the interface to the
>> target structure.  I.e.:
>>
>>   HAVE_foo -> targetm.have_foo ()
>>   gen_foo -> targetm.gen_foo ()
>>
>> This should move us closer to the pipedream goal of supporting multiple
>> targets at once.  It should also mean that only the target code depends
>> on insn-flags.h.
>
> using targetm. certainly seems like an improvement.  I wonder if it
> would be faster to stick this data on a per function object.  I think
> that would mean you could compute what insns are available once when the
> function is created and afterwards all checks would only needed to be
> reading computed values.

I think the memory cost of this is prohibitive.

Richard.

> Trev
>

Re: Add .def file for public target instructions

[Richard Sandiford]

Richard Biener <richard.guenther@gmail.com> writes:
> On Wed, Jul 1, 2015 at 11:39 AM, Trevor Saunders <tbsaunde@tbsaunde.org> wrote:
>> On Tue, Jun 23, 2015 at 07:41:42PM +0100, Richard Sandiford wrote:
>>> [A fair bit later than promised, sorry...]
>>>
>>> Mikhail posted a patch to make genflags generate the default HAVE_foo
>>> and gen_foo definitions that have recently been added to defaults.h:
>>>
>>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00723.html
>>>
>>> I agree it'd be a good idea to generate this kind of thing automatically,
>>> but I think we should take the opportunity to move the interface to the
>>> target structure.  I.e.:
>>>
>>>   HAVE_foo -> targetm.have_foo ()
>>>   gen_foo -> targetm.gen_foo ()
>>>
>>> This should move us closer to the pipedream goal of supporting multiple
>>> targets at once.  It should also mean that only the target code depends
>>> on insn-flags.h.
>>
>> using targetm. certainly seems like an improvement.  I wonder if it
>> would be faster to stick this data on a per function object.  I think
>> that would mean you could compute what insns are available once when the
>> function is created and afterwards all checks would only needed to be
>> reading computed values.
>
> I think the memory cost of this is prohibitive.

Yeah.  I did wonder about having optabs-like caching in some
target_globals structure, but I don't think it's really worth it.
In practice we only tend to call the have_*() functions when we're
trying to generate something (unlike optabs), and so any saving would
be dwarfed by the cost of generating whatever rtx we go on to create.

Thanks,
Richard

Re: Add .def file for public target instructions

[Richard Sandiford]

Richard Sandiford <richard.sandiford@arm.com> writes:
> Richard Biener <richard.guenther@gmail.com> writes:
>> On Wed, Jul 1, 2015 at 11:39 AM, Trevor Saunders
>> <tbsaunde@tbsaunde.org> wrote:
>>> On Tue, Jun 23, 2015 at 07:41:42PM +0100, Richard Sandiford wrote:
>>>> [A fair bit later than promised, sorry...]
>>>>
>>>> Mikhail posted a patch to make genflags generate the default HAVE_foo
>>>> and gen_foo definitions that have recently been added to defaults.h:
>>>>
>>>>   https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00723.html
>>>>
>>>> I agree it'd be a good idea to generate this kind of thing automatically,
>>>> but I think we should take the opportunity to move the interface to the
>>>> target structure.  I.e.:
>>>>
>>>>   HAVE_foo -> targetm.have_foo ()
>>>>   gen_foo -> targetm.gen_foo ()
>>>>
>>>> This should move us closer to the pipedream goal of supporting multiple
>>>> targets at once.  It should also mean that only the target code depends
>>>> on insn-flags.h.
>>>
>>> using targetm. certainly seems like an improvement.  I wonder if it
>>> would be faster to stick this data on a per function object.  I think
>>> that would mean you could compute what insns are available once when the
>>> function is created and afterwards all checks would only needed to be
>>> reading computed values.
>>
>> I think the memory cost of this is prohibitive.
>
> Yeah.  I did wonder about having optabs-like caching in some
> target_globals structure, but I don't think it's really worth it.
> In practice we only tend to call the have_*() functions when we're
> trying to generate something (unlike optabs), and so any saving would
> be dwarfed by the cost of generating whatever rtx we go on to create.

...plus (forgot the main reason): the condition for things like return
and simple_return are only available after register allocation.

Thanks,
Richard