*_ALIGN_MAX_SKIP macros

*_ALIGN_MAX_SKIP macros

[DJ Delorie]

 JUMP_ALIGN_MAX_SKIP
 LABEL_ALIGN_AFTER_BARRIER_MAX_SKIP
 LOOP_ALIGN_MAX_SKIP
 LABEL_ALIGN_MAX_SKIP

None of these macros take any parameters, but for optimal performance
on RX, it's key to match the max_skip with the size of the following
opcode - there's a penalty only if you branch to an opcode which spans
fetch groups, but it's the same penalty as running a NOP opcode, so
you don't want to add nops if you don't have to.

Can we extend those macros to take the label as a parameter?  From
that, the backend could find the next actual insn.

Re: *_ALIGN_MAX_SKIP macros

[Ian Lance Taylor]

DJ Delorie <dj@redhat.com> writes:

>  JUMP_ALIGN_MAX_SKIP
>  LABEL_ALIGN_AFTER_BARRIER_MAX_SKIP
>  LOOP_ALIGN_MAX_SKIP
>  LABEL_ALIGN_MAX_SKIP
>
> None of these macros take any parameters, but for optimal performance
> on RX, it's key to match the max_skip with the size of the following
> opcode - there's a penalty only if you branch to an opcode which spans
> fetch groups, but it's the same penalty as running a NOP opcode, so
> you don't want to add nops if you don't have to.
>
> Can we extend those macros to take the label as a parameter?  From
> that, the backend could find the next actual insn.

Seems fine to me.  Looks like none of the existing targets define them
directly anyhow.

If you do this, though, turn them into target hooks.  They are currently
fields in target_flag_state.

Ian

Re: *_ALIGN_MAX_SKIP macros

[DJ Delorie]

> DJ Delorie <dj@redhat.com> writes:
> >  JUMP_ALIGN_MAX_SKIP
> >  LABEL_ALIGN_AFTER_BARRIER_MAX_SKIP
> >  LOOP_ALIGN_MAX_SKIP
> >  LABEL_ALIGN_MAX_SKIP
> >
> > None of these macros take any parameters, but for optimal performance
> > on RX, it's key to match the max_skip with the size of the following
> > opcode - there's a penalty only if you branch to an opcode which spans
> > fetch groups, but it's the same penalty as running a NOP opcode, so
> > you don't want to add nops if you don't have to.
> >
> > Can we extend those macros to take the label as a parameter?  From
> > that, the backend could find the next actual insn.
> 
> Seems fine to me.  Looks like none of the existing targets define them
> directly anyhow.
> 
> If you do this, though, turn them into target hooks.  They are currently
> fields in target_flag_state.

How's this?

	* doc/tm.texi.in (TARGET_ASM_JUMP_ALIGN_MAX_SKIP): New.
	(TARGET_ASM_LABEL_ALIGN_AFTER_BARRIER_MAX_SKIP): Change to hook.
	(TARGET_ASM_LOOP_ALIGN_MAX_SKIP): Likewise.
	(TARGET_ASM_LABEL_ALIGN_MAX_SKIP): Likewise.

	* targhooks.h (default_label_align_after_barrier_max_skip,
	default_loop_align_max_skip, default_label_align_max_skip,
	default_jump_align_max_skip): Declare.

	* target.def (label_align_after_barrier_max_skip): New.
	(loop_align_max_skip): New.
	(label_align_max_skip): New.
	(jump_align_max_skip): New.

	* final.c (LABEL_ALIGN_MAX_SKIP): Remove.
	(LOOP_ALIGN_MAX_SKIP): Remove.
	(LABEL_ALIGN_AFTER_BARRIER_MAX_SKIP): Remove.
	(JUMP_ALIGN_MAX_SKIP): Remove.
	(default_label_align_after_barrier_max_skip): New.
	(default_loop_align_max_skip): New.
	(default_label_align_max_skip): New.
	(default_jump_align_max_skip): New.
	(compute_alignments): Use the new hooks.
	(shorten_branches): Likewise.

Index: doc/tm.texi.in
===================================================================
--- doc/tm.texi.in	(revision 164701)
+++ doc/tm.texi.in	(working copy)
@@ -8893,26 +8893,32 @@ define the macro.
 Unless it's necessary to inspect the @var{label} parameter, it is better
 to set the variable @var{align_jumps} in the target's
 @code{TARGET_OPTION_OVERRIDE}.  Otherwise, you should try to honor the user's
 selection in @var{align_jumps} in a @code{JUMP_ALIGN} implementation.
 @end defmac
 
+@hook TARGET_ASM_JUMP_ALIGN_MAX_SKIP
+The maximum number of bytes to skip before @var{label} when applying
+@code{JUMP_ALIGN}.  This works only if
+@code{ASM_OUTPUT_MAX_SKIP_ALIGN} is defined.
+@end deftypefn
+
 @defmac LABEL_ALIGN_AFTER_BARRIER (@var{label})
 The alignment (log base 2) to put in front of @var{label}, which follows
 a @code{BARRIER}.
 
 This macro need not be defined if you don't want any special alignment
 to be done at such a time.  Most machine descriptions do not currently
 define the macro.
 @end defmac
 
-@defmac LABEL_ALIGN_AFTER_BARRIER_MAX_SKIP
-The maximum number of bytes to skip when applying
+@hook TARGET_ASM_LABEL_ALIGN_AFTER_BARRIER_MAX_SKIP
+The maximum number of bytes to skip before @var{label} when applying
 @code{LABEL_ALIGN_AFTER_BARRIER}.  This works only if
 @code{ASM_OUTPUT_MAX_SKIP_ALIGN} is defined.
-@end defmac
+@end deftypefn
 
 @defmac LOOP_ALIGN (@var{label})
 The alignment (log base 2) to put in front of @var{label}, which follows
 a @code{NOTE_INSN_LOOP_BEG} note.
 
 This macro need not be defined if you don't want any special alignment
@@ -8922,32 +8928,34 @@ define the macro.
 Unless it's necessary to inspect the @var{label} parameter, it is better
 to set the variable @code{align_loops} in the target's
 @code{TARGET_OPTION_OVERRIDE}.  Otherwise, you should try to honor the user's
 selection in @code{align_loops} in a @code{LOOP_ALIGN} implementation.
 @end defmac
 
-@defmac LOOP_ALIGN_MAX_SKIP
-The maximum number of bytes to skip when applying @code{LOOP_ALIGN}.
-This works only if @code{ASM_OUTPUT_MAX_SKIP_ALIGN} is defined.
-@end defmac
+@hook TARGET_ASM_LOOP_ALIGN_MAX_SKIP
+The maximum number of bytes to skip when applying @code{LOOP_ALIGN} to
+@var{label}.  This works only if @code{ASM_OUTPUT_MAX_SKIP_ALIGN} is
+defined.
+@end deftypefn
 
 @defmac LABEL_ALIGN (@var{label})
 The alignment (log base 2) to put in front of @var{label}.
 If @code{LABEL_ALIGN_AFTER_BARRIER} / @code{LOOP_ALIGN} specify a different alignment,
 the maximum of the specified values is used.
 
 Unless it's necessary to inspect the @var{label} parameter, it is better
 to set the variable @code{align_labels} in the target's
 @code{TARGET_OPTION_OVERRIDE}.  Otherwise, you should try to honor the user's
 selection in @code{align_labels} in a @code{LABEL_ALIGN} implementation.
 @end defmac
 
-@defmac LABEL_ALIGN_MAX_SKIP
-The maximum number of bytes to skip when applying @code{LABEL_ALIGN}.
-This works only if @code{ASM_OUTPUT_MAX_SKIP_ALIGN} is defined.
-@end defmac
+@hook TARGET_ASM_LABEL_ALIGN_MAX_SKIP
+The maximum number of bytes to skip when applying @code{LABEL_ALIGN}
+to @var{label}.  This works only if @code{ASM_OUTPUT_MAX_SKIP_ALIGN}
+is defined.
+@end deftypefn
 
 @defmac ASM_OUTPUT_SKIP (@var{stream}, @var{nbytes})
 A C statement to output to the stdio stream @var{stream} an assembler
 instruction to advance the location counter by @var{nbytes} bytes.
 Those bytes should be zero when loaded.  @var{nbytes} will be a C
 expression of type @code{unsigned HOST_WIDE_INT}.
Index: targhooks.h
===================================================================
--- targhooks.h	(revision 164701)
+++ targhooks.h	(working copy)
@@ -156,6 +156,11 @@ extern bool default_profile_before_prolo
 extern bool default_class_likely_spilled_p (reg_class_t);
 
 extern enum unwind_info_type default_debug_unwind_info (void);
 extern enum unwind_info_type default_except_unwind_info (void);
 extern enum unwind_info_type dwarf2_except_unwind_info (void);
 extern enum unwind_info_type sjlj_except_unwind_info (void);
+
+extern int default_label_align_after_barrier_max_skip (rtx);
+extern int default_loop_align_max_skip (rtx);
+extern int default_label_align_max_skip (rtx);
+extern int default_jump_align_max_skip (rtx);
Index: target.def
===================================================================
--- target.def	(revision 164701)
+++ target.def	(working copy)
@@ -98,12 +98,44 @@ DEFHOOKPOD
 (byte_op,
  "",
  const char *, "\t.byte\t")
 DEFHOOKPOD (aligned_op, "*", struct asm_int_op, TARGET_ASM_ALIGNED_INT_OP)
 DEFHOOKPOD (unaligned_op, "*", struct asm_int_op, TARGET_ASM_UNALIGNED_INT_OP)
 
+/* The maximum number of bytes to skip when applying
+   LABEL_ALIGN_AFTER_BARRIER.  */
+DEFHOOK
+(label_align_after_barrier_max_skip,
+ "",
+ int, (rtx label),
+ default_label_align_after_barrier_max_skip)
+
+/* The maximum number of bytes to skip when applying
+   LOOP_ALIGN.  */
+DEFHOOK
+(loop_align_max_skip,
+ "",
+ int, (rtx label),
+ default_loop_align_max_skip)
+
+/* The maximum number of bytes to skip when applying
+   LABEL_ALIGN.  */
+DEFHOOK
+(label_align_max_skip,
+ "",
+ int, (rtx label),
+ default_label_align_max_skip)
+
+/* The maximum number of bytes to skip when applying
+   JUMP_ALIGN.  */
+DEFHOOK
+(jump_align_max_skip,
+ "",
+ int, (rtx label),
+ default_jump_align_max_skip)
+
 /* Try to output the assembler code for an integer object whose
    value is given by X.  SIZE is the size of the object in bytes and
    ALIGNED_P indicates whether it is aligned.  Return true if
    successful.  Only handles cases for which BYTE_OP, ALIGNED_OP
    and UNALIGNED_OP are NULL.  */
 DEFHOOK
Index: final.c
===================================================================
--- final.c	(revision 164701)
+++ final.c	(working copy)
@@ -65,12 +65,13 @@ along with GCC; see the file COPYING3.  
 #include "rtl-error.h"
 #include "toplev.h" /* exact_log2, floor_log2 */
 #include "reload.h"
 #include "intl.h"
 #include "basic-block.h"
 #include "target.h"
+#include "targhooks.h"
 #include "debug.h"
 #include "expr.h"
 #include "cfglayout.h"
 #include "tree-pass.h"
 #include "tree-flow.h"
 #include "timevar.h"
@@ -494,39 +495,47 @@ get_attr_min_length (rtx insn)
    address mod X to one mod Y, which is Y - X.  */
 
 #ifndef LABEL_ALIGN
 #define LABEL_ALIGN(LABEL) align_labels_log
 #endif
 
-#ifndef LABEL_ALIGN_MAX_SKIP
-#define LABEL_ALIGN_MAX_SKIP align_labels_max_skip
-#endif
-
 #ifndef LOOP_ALIGN
 #define LOOP_ALIGN(LABEL) align_loops_log
 #endif
 
-#ifndef LOOP_ALIGN_MAX_SKIP
-#define LOOP_ALIGN_MAX_SKIP align_loops_max_skip
-#endif
-
 #ifndef LABEL_ALIGN_AFTER_BARRIER
 #define LABEL_ALIGN_AFTER_BARRIER(LABEL) 0
 #endif
 
-#ifndef LABEL_ALIGN_AFTER_BARRIER_MAX_SKIP
-#define LABEL_ALIGN_AFTER_BARRIER_MAX_SKIP 0
-#endif
-
 #ifndef JUMP_ALIGN
 #define JUMP_ALIGN(LABEL) align_jumps_log
 #endif
 
-#ifndef JUMP_ALIGN_MAX_SKIP
-#define JUMP_ALIGN_MAX_SKIP align_jumps_max_skip
-#endif
+int
+default_label_align_after_barrier_max_skip (rtx insn ATTRIBUTE_UNUSED)
+{
+  return 0;
+}
+
+int
+default_loop_align_max_skip (rtx insn ATTRIBUTE_UNUSED)
+{
+  return align_loops_max_skip;
+}
+
+int
+default_label_align_max_skip (rtx insn ATTRIBUTE_UNUSED)
+{
+  return align_labels_max_skip;
+}
+
+int
+default_jump_align_max_skip (rtx insn ATTRIBUTE_UNUSED)
+{
+  return align_jumps_max_skip;
+}
 
 #ifndef ADDR_VEC_ALIGN
 static int
 final_addr_vec_align (rtx addr_vec)
 {
   int align = GET_MODE_SIZE (GET_MODE (PATTERN (addr_vec)));
@@ -728,13 +737,13 @@ compute_alignments (void)
 	  if (dump_file)
 	    fprintf(dump_file, "BB %4i freq %4i loop %2i loop_depth %2i skipped.\n",
 		    bb->index, bb->frequency, bb->loop_father->num, bb->loop_depth);
 	  continue;
 	}
       max_log = LABEL_ALIGN (label);
-      max_skip = LABEL_ALIGN_MAX_SKIP;
+      max_skip = targetm.asm_out.label_align_max_skip (label);
 
       FOR_EACH_EDGE (e, ei, bb->preds)
 	{
 	  if (e->flags & EDGE_FALLTHRU)
 	    has_fallthru = 1, fallthru_frequency += EDGE_FREQUENCY (e);
 	  else
@@ -772,13 +781,13 @@ compute_alignments (void)
 	  log = JUMP_ALIGN (label);
 	  if (dump_file)
 	    fprintf(dump_file, "  jump alignment added.\n");
 	  if (max_log < log)
 	    {
 	      max_log = log;
-	      max_skip = JUMP_ALIGN_MAX_SKIP;
+	      max_skip = targetm.asm_out.jump_align_max_skip (label);
 	    }
 	}
       /* In case block is frequent and reached mostly by non-fallthru edge,
 	 align it.  It is most likely a first block of loop.  */
       if (has_fallthru
 	  && optimize_bb_for_speed_p (bb)
@@ -789,13 +798,13 @@ compute_alignments (void)
 	  log = LOOP_ALIGN (label);
 	  if (dump_file)
 	    fprintf(dump_file, "  internal loop alignment added.\n");
 	  if (max_log < log)
 	    {
 	      max_log = log;
-	      max_skip = LOOP_ALIGN_MAX_SKIP;
+	      max_skip = targetm.asm_out.loop_align_max_skip (label);
 	    }
 	}
       LABEL_TO_ALIGNMENT (label) = max_log;
       LABEL_TO_MAX_SKIP (label) = max_skip;
     }
 
@@ -922,26 +931,26 @@ shorten_branches (rtx first ATTRIBUTE_UN
 	  if (!next_is_jumptable)
 	    {
 	      log = LABEL_ALIGN (insn);
 	      if (max_log < log)
 		{
 		  max_log = log;
-		  max_skip = LABEL_ALIGN_MAX_SKIP;
+		  max_skip = targetm.asm_out.label_align_max_skip (insn);
 		}
 	    }
 	  /* ADDR_VECs only take room if read-only data goes into the text
 	     section.  */
 	  if ((JUMP_TABLES_IN_TEXT_SECTION
 	       || readonly_data_section == text_section)
 	      && next_is_jumptable)
 	    {
 	      log = ADDR_VEC_ALIGN (next);
 	      if (max_log < log)
 		{
 		  max_log = log;
-		  max_skip = LABEL_ALIGN_MAX_SKIP;
+		  max_skip = targetm.asm_out.label_align_max_skip (insn);
 		}
 	    }
 	  LABEL_TO_ALIGNMENT (insn) = max_log;
 	  LABEL_TO_MAX_SKIP (insn) = max_skip;
 	  max_log = 0;
 	  max_skip = 0;
@@ -955,13 +964,13 @@ shorten_branches (rtx first ATTRIBUTE_UN
 	    if (LABEL_P (label))
 	      {
 		log = LABEL_ALIGN_AFTER_BARRIER (insn);
 		if (max_log < log)
 		  {
 		    max_log = log;
-		    max_skip = LABEL_ALIGN_AFTER_BARRIER_MAX_SKIP;
+		    max_skip = targetm.asm_out.label_align_after_barrier_max_skip (label);
 		  }
 		break;
 	      }
 	}
     }
 #ifdef HAVE_ATTR_length

Re: *_ALIGN_MAX_SKIP macros

[Ian Lance Taylor]

DJ Delorie <dj@redhat.com> writes:

> 	* doc/tm.texi.in (TARGET_ASM_JUMP_ALIGN_MAX_SKIP): New.
> 	(TARGET_ASM_LABEL_ALIGN_AFTER_BARRIER_MAX_SKIP): Change to hook.
> 	(TARGET_ASM_LOOP_ALIGN_MAX_SKIP): Likewise.
> 	(TARGET_ASM_LABEL_ALIGN_MAX_SKIP): Likewise.
>
> 	* targhooks.h (default_label_align_after_barrier_max_skip,
> 	default_loop_align_max_skip, default_label_align_max_skip,
> 	default_jump_align_max_skip): Declare.
>
> 	* target.def (label_align_after_barrier_max_skip): New.
> 	(loop_align_max_skip): New.
> 	(label_align_max_skip): New.
> 	(jump_align_max_skip): New.
>
> 	* final.c (LABEL_ALIGN_MAX_SKIP): Remove.
> 	(LOOP_ALIGN_MAX_SKIP): Remove.
> 	(LABEL_ALIGN_AFTER_BARRIER_MAX_SKIP): Remove.
> 	(JUMP_ALIGN_MAX_SKIP): Remove.
> 	(default_label_align_after_barrier_max_skip): New.
> 	(default_loop_align_max_skip): New.
> 	(default_label_align_max_skip): New.
> 	(default_jump_align_max_skip): New.
> 	(compute_alignments): Use the new hooks.
> 	(shorten_branches): Likewise.

This is OK if you add LABEL_ALIGN_MAX_SKIP, LOOP_ALIGN_MAX_SKIP,
LABEL_ALIGN_AFTER_BARRIER_MAX_SKIP, and JUMP_ALIGN_MAX_SKIP to the

/* Old target macros that have moved to the target hooks structure.  */

#pragma GCC poison list in system.h.

Thanks.

Ian

Re: *_ALIGN_MAX_SKIP macros

[DJ Delorie]

> This is OK if you add LABEL_ALIGN_MAX_SKIP, LOOP_ALIGN_MAX_SKIP,
> LABEL_ALIGN_AFTER_BARRIER_MAX_SKIP, and JUMP_ALIGN_MAX_SKIP to the
> 
> /* Old target macros that have moved to the target hooks structure.  */
> 
> #pragma GCC poison list in system.h.

Thanks, committed with that change.