patch: RTL loop optimizer - new target hook in doloop_valid_p

Adrian Straetling straetling@de.ibm.com
Wed Jun 8 17:44:00 GMT 2005


On Tue, Jun 07, 2005 at 12:52:24PM -0700, Richard Henderson wrote:
> On Tue, Jun 07, 2005 at 06:42:20PM +0200, Adrian Straetling wrote:
> > Sorry for overlooking that. I guess that would be nice to have.
> > With this patch doloop_valid_p also passes 'dump_file' to the target
> > hook.  
> 
> I think I'd prefer that the hook return a "const char *",
> rename the hook "invalid_within_doloop", and so NULL is a
> natural result for valid.

Here is an updated patch that does so and also includes the necessary changes
for the affected targets (rs6000, s390).  It survived a bootstrap on s390x and
showed no regressions. I also checked the hook behaving correctly when
defined similar to rs6000.  Ok for mainline?

I left the behaviour of default_invalid_within_doloop unchanged. A following-up
patch could probably remove the PPC specific part though I don't know if any
other target is relying on it?

Bye, 
Adrian


2005-06-08  Adrian Straetling  <straetling@de.ibm.com>

	* target.h: (insn_valid_within_doloop): Rename into
	  "invalid_within_doloop". Change return type to "const char *". Update
	  Comment.
	* targhooks.h: (default_insn_valid_within_doloop): Rename into
	  "default_invalid_within_doloop". 
	* targhooks.c: Likewise. Update Comment.
	* target-def.h: (TARGET_INSN_VALID_WITHIN_DOLOOP): Rename target hook
	  into "TARGET_INVALID_WITHIN_DOLOOP". Default it to
	  "default_invalid_within_doloop".
	* gcc/hooks.c: (hook_constcharptr_rtx_null): New function.
	  (hook_bool_rtx_true): Remove.
	* gcc/hooks.h: (hook_constcharptr_rtx_null): Declare.
	  (hook_bool_rtx_true): Remove.
	* loop-doloop.c: (doloop_valid_p): Temporarily store return value of
	  "invalid_within_doloop" and print error message if non-null.
	  Update Comment.
	* doc/tm.texi: Update documentation.
	* config/s390/s390.c: Adjust to new hook name and new default hook.
	* config/rs6000/rs6000.c: (rs6000_insn_valid_within_doloop): Rename
	  into "rs6000_invalid_within_doloop".
	  (s6000_invalid_within_doloop): Change return type to "static const
	  char *" and replace return values. Update Comment.


Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c.orig	2005-06-08 11:07:23.000000000 +0200
+++ gcc/config/rs6000/rs6000.c	2005-06-08 18:41:07.000000000 +0200
@@ -552,7 +552,7 @@ struct processor_costs power4_cost = {
 
 
 static bool rs6000_function_ok_for_sibcall (tree, tree);
-static bool rs6000_insn_valid_within_doloop (rtx);
+static bool rs6000_invalid_within_doloop (rtx);
 static rtx rs6000_generate_compare (enum rtx_code);
 static void rs6000_maybe_dead (rtx);
 static void rs6000_emit_stack_tie (void);
@@ -910,7 +910,7 @@ static const char alt_reg_names[][8] =
 #define TARGET_FUNCTION_OK_FOR_SIBCALL rs6000_function_ok_for_sibcall
 
 #undef TARGET_INSN_VALID_WITHIN_DOLOOP
-#define TARGET_INSN_VALID_WITHIN_DOLOOP rs6000_insn_valid_within_doloop
+#define TARGET_INSN_VALID_WITHIN_DOLOOP rs6000_invalid_within_doloop
 
 #undef TARGET_RTX_COSTS
 #define TARGET_RTX_COSTS rs6000_rtx_costs
@@ -12532,21 +12532,22 @@ rs6000_function_ok_for_sibcall (tree dec
   return false;
 }
 
-/* TRUE if INSN insn is valid within a low-overhead loop.
+/* NULL if INSN insn is valid within a low-overhead loop.
+   Otherwise return why doloop cannot be applied.
    PowerPC uses the COUNT register for branch on table instructions.  */
 
-static bool
-rs6000_insn_valid_within_doloop (rtx insn)
+static const char *
+rs6000_invalid_within_doloop (rtx insn)
 {
   if (CALL_P (insn))
-    return false;
+    return "Function call in the loop.";
 
   if (JUMP_P (insn)
       && (GET_CODE (PATTERN (insn)) == ADDR_DIFF_VEC
 	  || GET_CODE (PATTERN (insn)) == ADDR_VEC))
-    return false;
+    return "Computed branch in the loop.";
 
-  return true;
+  return NULL;
 }
 
 static int
Index: gcc/config/s390/s390.c
===================================================================
--- gcc/config/s390/s390.c.orig	2005-06-08 11:07:23.000000000 +0200
+++ gcc/config/s390/s390.c	2005-06-08 12:27:30.000000000 +0200
@@ -8284,8 +8284,8 @@ s390_reorg (void)
 #undef TARGET_CC_MODES_COMPATIBLE
 #define TARGET_CC_MODES_COMPATIBLE s390_cc_modes_compatible
 
-#undef TARGET_INSN_VALID_WITHIN_DOLOOP
-#define TARGET_INSN_VALID_WITHIN_DOLOOP hook_bool_rtx_true
+#undef TARGET_INVALID_WITHIN_DOLOOP
+#define TARGET_INVALID_WITHIN_DOLOOP hook_constcharptr_rtx_null
 
 #ifdef HAVE_AS_TLS
 #undef TARGET_ASM_OUTPUT_DWARF_DTPREL
Index: gcc/doc/tm.texi
===================================================================
--- gcc/doc/tm.texi.orig	2005-06-08 11:07:25.000000000 +0200
+++ gcc/doc/tm.texi	2005-06-08 18:34:58.000000000 +0200
@@ -9377,13 +9377,14 @@ simplified expression for the call's res
 the value will be ignored.
 @end deftypefn
 
-@deftypefn {Target Hook} bool TARGET_INSN_VALID_WITHIN_DOLOOP (rtx @var{insn})
+@deftypefn {Target Hook} const char * TARGET_INVALID_WITHIN_DOLOOP (rtx @var{insn})
 
-Take an instruction in @var{insn} and return true if it is valid within a
-low-overhead loop.
+Take an instruction in @var{insn} and return NULL if it is valid within a
+low-overhead loop, otherwise return a string why doloop could not be applied.
 
-Many targets use special registers for low-overhead looping. This function
-should return false for any instruction that clobbers these. 
+Many targets use special registers for low-overhead looping. For any
+instruction that clobbers these this function should return a string indicating
+the reason why the doloop could not be applied. 
 By default, the RTL loop optimizer does not use a present doloop pattern for
 loops containing function calls or branch on table instructions.  
 @end deftypefn
Index: gcc/hooks.c
===================================================================
--- gcc/hooks.c.orig	2005-06-08 11:03:32.000000000 +0200
+++ gcc/hooks.c	2005-06-08 18:58:49.035026268 +0200
@@ -192,12 +192,6 @@ hook_bool_rtx_false (rtx a ATTRIBUTE_UNU
 }
 
 bool
-hook_bool_rtx_true (rtx a ATTRIBUTE_UNUSED)
-{
-  return true;
-}
-
-bool
 hook_bool_uintp_uintp_false (unsigned int *a ATTRIBUTE_UNUSED,
 			     unsigned int *b ATTRIBUTE_UNUSED)
 {
@@ -255,3 +249,10 @@ hook_tree_tree_tree_bool_null (tree t0 A
 {
   return NULL;
 }
+
+/* Generic hook that takes a rtx and returns a NULL string.  */
+const char *
+hook_constcharptr_rtx_null (rtx r ATTRIBUTE_UNUSED)
+{
+  return NULL;
+}
Index: gcc/hooks.h
===================================================================
--- gcc/hooks.h.orig	2005-06-08 11:03:32.000000000 +0200
+++ gcc/hooks.h	2005-06-08 18:58:40.955026268 +0200
@@ -35,7 +35,6 @@ extern bool hook_bool_tree_hwi_hwi_tree_
 extern bool hook_bool_tree_hwi_hwi_tree_true (tree, HOST_WIDE_INT, HOST_WIDE_INT,
 				       tree);
 extern bool hook_bool_rtx_false (rtx);
-extern bool hook_bool_rtx_true (rtx);
 extern bool hook_bool_uintp_uintp_false (unsigned int *, unsigned int *);
 extern bool hook_bool_rtx_int_int_intp_false (rtx, int, int, int *);
 extern bool hook_bool_constcharptr_size_t_false (const char *, size_t);
@@ -66,4 +65,5 @@ extern rtx hook_rtx_tree_int_null (tree,
 extern tree hook_tree_tree_tree_tree_3rd_identity (tree, tree, tree);
 extern const char *hook_constcharptr_tree_null (tree);
 extern tree hook_tree_tree_tree_bool_null (tree, tree, bool);
+extern const char *hook_constcharptr_rtx_null (rtx);
 #endif
Index: gcc/loop-doloop.c
===================================================================
--- gcc/loop-doloop.c.orig	2005-06-08 11:03:32.000000000 +0200
+++ gcc/loop-doloop.c	2005-06-08 18:43:21.000000000 +0200
@@ -203,12 +203,16 @@ doloop_valid_p (struct loop *loop, struc
 	{
 	  /* Different targets have different necessities for low-overhead
 	     looping.  Call the back end for each instruction within the loop
-	     to let it decide whether the insn is valid.  */
-	  if (!targetm.insn_valid_within_doloop (insn))
-	  {
+	     to let it decide whether the insn prohibits a low-overhead loop.
+	     It will then return the cause for it to emit to the dump file.  */
+	  const char * invalid = targetm.invalid_within_doloop (insn);
+	  if (invalid)
+	    {
+	      if (dump_file)
+		fprintf (dump_file, "Doloop: %s\n", invalid);
 	      result = false;
 	      goto cleanup;
-	  }
+	    }
 	}
     }
   result = true;
Index: gcc/target-def.h
===================================================================
--- gcc/target-def.h.orig	2005-06-08 10:51:23.000000000 +0200
+++ gcc/target-def.h	2005-06-08 18:34:58.000000000 +0200
@@ -137,8 +137,8 @@ Foundation, 59 Temple Place - Suite 330,
 #define TARGET_HAVE_NAMED_SECTIONS false
 #endif
 
-#ifndef TARGET_INSN_VALID_WITHIN_DOLOOP
-#define TARGET_INSN_VALID_WITHIN_DOLOOP default_insn_valid_within_doloop
+#ifndef TARGET_INVALID_WITHIN_DOLOOP
+#define TARGET_INVALID_WITHIN_DOLOOP default_invalid_within_doloop
 #endif
 
 #ifndef TARGET_HAVE_TLS
@@ -564,7 +564,7 @@ Foundation, 59 Temple Place - Suite 330,
   TARGET_DWARF_CALLING_CONVENTION,              \
   TARGET_DWARF_HANDLE_FRAME_UNSPEC,		\
   TARGET_STDARG_OPTIMIZE_HOOK,			\
-  TARGET_INSN_VALID_WITHIN_DOLOOP,		\
+  TARGET_INVALID_WITHIN_DOLOOP,			\
   TARGET_CALLS,					\
   TARGET_CXX,					\
   TARGET_HAVE_NAMED_SECTIONS,			\
Index: gcc/target.h
===================================================================
--- gcc/target.h.orig	2005-06-08 11:07:14.000000000 +0200
+++ gcc/target.h	2005-06-08 11:13:39.000000000 +0200
@@ -528,8 +528,9 @@ struct gcc_target
      to be checked for va_list references.  */
   bool (*stdarg_optimize_hook) (struct stdarg_info *ai, tree lhs, tree rhs);
 
-  /* Returns true if target supports the insn within a doloop block.  */
-  bool (*insn_valid_within_doloop) (rtx);
+  /* Returns NULL if target supports the insn within a doloop block,
+     otherwise it returns an error message.  */
+  const char * (*invalid_within_doloop) (rtx);
     
   /* Functions relating to calls - argument passing, returns, etc.  */
   struct calls {
Index: gcc/targhooks.c
===================================================================
--- gcc/targhooks.c.orig	2005-06-08 11:03:32.000000000 +0200
+++ gcc/targhooks.c	2005-06-08 11:25:46.000000000 +0200
@@ -262,34 +262,28 @@ default_scalar_mode_supported_p (enum ma
     }
 }
 
-/* TRUE if INSN insn is valid within a low-overhead loop.
+/* NULL if INSN insn is valid within a low-overhead loop, otherwise returns
+   an error message.
   
    This function checks whether a given INSN is valid within a low-overhead
-   loop.  A called function may clobber any special registers required for
-   low-overhead looping. Additionally, some targets (eg, PPC) use the count
+   loop.  If INSN is invalid it returns the reason for that, otherwise it
+   returns NULL. A called function may clobber any special registers required
+   for low-overhead looping. Additionally, some targets (eg, PPC) use the count
    register for branch on table instructions. We reject the doloop pattern in
    these cases.  */
 
-bool 
-default_insn_valid_within_doloop (rtx insn)
+const char *
+default_invalid_within_doloop (rtx insn)
 {
   if (CALL_P (insn))
-    {
-      if (dump_file)
-	fprintf (dump_file, "Doloop: Function call in loop.\n");
-	return false;
-    }
+    return "Function call in loop.";
   
   if (JUMP_P (insn)
       && (GET_CODE (PATTERN (insn)) == ADDR_DIFF_VEC
 	  || GET_CODE (PATTERN (insn)) == ADDR_VEC))
-    {
-      if (dump_file)
-	fprintf (dump_file, "Doloop: Computed branch in the loop.\n");
-      return false;
-    }
+    return "Computed branch in the loop.";
   
-  return true;
+  return NULL;
 }
 
 bool
Index: gcc/targhooks.h
===================================================================
--- gcc/targhooks.h.orig	2005-06-08 11:03:32.000000000 +0200
+++ gcc/targhooks.h	2005-06-08 11:09:35.000000000 +0200
@@ -46,7 +46,7 @@ extern void default_unwind_emit (FILE *,
 
 extern bool default_scalar_mode_supported_p (enum machine_mode);
 
-extern bool default_insn_valid_within_doloop (rtx);
+extern const char * default_invalid_within_doloop (rtx);
 
 /* These are here, and not in hooks.[ch], because not all users of
    hooks.h include tm.h, and thus we don't have CUMULATIVE_ARGS.  */



More information about the Gcc-patches mailing list