Add support for use_hazard_barrier_return function attribute

Add support for use_hazard_barrier_return function attribute

[Prachi Godbole]

This patch adds support for function attribute  __attribute__ ((use_hazard_barrier_return)). The attribute will generate hazard barrier return (jr.hb) instead of a normal return instruction.

Changelog:

2017-04-25  Prachi Godbole  <prachi.godbole@imgtec.com>

gcc/
	* config/mips/mips.h (machine_function): New variable
	use_hazard_barrier_return_p.
	* config/mips/mips.md (UNSPEC_JRHB): New unspec.
	(mips_hb_return_internal): New insn pattern.
	* config/mips/mips.c (mips_attribute_table): Add attribute
	use_hazard_barrier_return.
	(mips_use_hazard_barrier_return_p): New static function.
	(mips_function_attr_inlinable_p): Likewise.
	(mips_compute_frame_info): Set use_hazard_barrier_return_p.  Emit error
	for unsupported architecture choice.
	(mips_function_ok_for_sibcall, mips_can_use_return_insn): Return false
	for use_hazard_barrier_return.
	(mips_expand_epilogue): Emit hazard barrier return.
	* doc/extend.texi: Document use_hazard_barrier_return.

gcc/testsuite/
	* gcc.target/mips/hazard-barrier-return-attribute.c: New test.


Ok for stage1?

Regards,
Prachi


Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 246899)
+++ gcc/doc/extend.texi	(working copy)
@@ -4496,6 +4496,12 @@ On MIPS targets, you can use the @code{nocompressi
 to locally turn off MIPS16 and microMIPS code generation.  This attribute
 overrides the @option{-mips16} and @option{-mmicromips} options on the
 command line (@pxref{MIPS Options}).
+
+@item use_hazard_barrier_return
+@cindex @code{use_hazard_barrier_return} function attribute, MIPS
+This function attribute instructs the compiler to generate hazard barrier return
+that clears all execution and instruction hazards while returning, instead of
+generating a normal return instruction.
 @end table
 
 @node MSP430 Function Attributes
Index: gcc/config/mips/mips.md
===================================================================
--- gcc/config/mips/mips.md	(revision 246899)
+++ gcc/config/mips/mips.md	(working copy)
@@ -156,6 +156,7 @@
 
   ;; The `.insn' pseudo-op.
   UNSPEC_INSN_PSEUDO
+  UNSPEC_JRHB
 ])
 
 (define_constants
@@ -6578,6 +6579,20 @@
   [(set_attr "type"	"jump")
    (set_attr "mode"	"none")])
 
+;; Insn to clear execution and instruction hazards while returning.
+;; However, it doesn't clear hazards created by the insn in its delay slot.
+;; Thus, explicitly place a nop in its delay slot.
+
+(define_insn "mips_hb_return_internal"
+  [(return)
+   (unspec_volatile [(match_operand 0 "pmode_register_operand" "")]
+		    UNSPEC_JRHB)]
+  ""
+  {
+    return "%(jr.hb\t$31%/%)";
+  }
+  [(set_attr "insn_count" "2")])
+
 ;; Normal return.
 
 (define_insn "<optab>_internal"
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	(revision 246899)
+++ gcc/config/mips/mips.c	(working copy)
@@ -615,6 +615,7 @@ static const struct attribute_spec mips_attribute_
     mips_handle_use_shadow_register_set_attr, false },
   { "keep_interrupts_masked",	0, 0, false, true,  true, NULL, false },
   { "use_debug_exception_return", 0, 0, false, true,  true, NULL, false },
+  { "use_hazard_barrier_return", 0, 0, true, false, false, NULL, false },
   { NULL,	   0, 0, false, false, false, NULL, false }
 };
 

@@ -1275,6 +1276,16 @@ mips_use_debug_exception_return_p (tree type)
 			   TYPE_ATTRIBUTES (type)) != NULL;
 }
 
+/* Check if the attribute to use hazard barrier return is set for
+   the function declaration DECL.  */
+
+static bool
+mips_use_hazard_barrier_return_p (tree decl)
+{
+  return lookup_attribute ("use_hazard_barrier_return",
+			    DECL_ATTRIBUTES (decl)) != NULL;
+}
+
 /* Return the set of compression modes that are explicitly required
    by the attributes in ATTRIBUTES.  */
 
@@ -1460,6 +1471,21 @@ mips_can_inline_p (tree caller, tree callee)
   return default_target_can_inline_p (caller, callee);
 }
 
+/* Implement TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P.
+
+   A function reqeuesting clearing of all instruction and execution hazards
+   before returning cannot be inlined - thereby not clearing any hazards.
+   All our other function attributes are related to how out-of-line copies
+   should be compiled or called.  They don't in themselves prevent inlining.  */
+
+static bool
+mips_function_attr_inlinable_p (const_tree decl)
+{
+  if (mips_use_hazard_barrier_return_p (const_cast<tree>(decl)))
+    return false;
+  return hook_bool_const_tree_true (decl);
+}
+
 /* Handle an "interrupt" attribute with an optional argument.  */
 
 static tree
@@ -7863,6 +7889,11 @@ mips_function_ok_for_sibcall (tree decl, tree exp
       && !targetm.binds_local_p (decl))
     return false;
 
+  /* Can't generate sibling calls if returning from current function using
+     hazard barrier return.  */
+  if (mips_use_hazard_barrier_return_p (current_function_decl))
+    return false;
+
   /* Otherwise OK.  */
   return true;
 }
@@ -10943,6 +10974,17 @@ mips_compute_frame_info (void)
 	}
     }
 
+  /* Determine whether to use hazard barrier return or not.  */
+  if (mips_use_hazard_barrier_return_p (current_function_decl))
+    {
+      if (mips_isa_rev < 2)
+	error ("hazard barrier return requires MIPS ISA to be R2 or greater");
+      else if (TARGET_MIPS16)
+	error ("hazard barrier return cannot be used with MIPS16 functions");
+      else
+	cfun->machine->use_hazard_barrier_return_p = true;
+    }
+
   frame = &cfun->machine->frame;
   memset (frame, 0, sizeof (*frame));
   size = get_frame_size ();
@@ -12606,7 +12648,8 @@ mips_expand_epilogue (bool sibcall_p)
 	       && !crtl->calls_eh_return
 	       && !sibcall_p
 	       && step2 > 0
-	       && mips_unsigned_immediate_p (step2, 5, 2))
+	       && mips_unsigned_immediate_p (step2, 5, 2)
+	       && !cfun->machine->use_hazard_barrier_return_p)
 	use_jraddiusp_p = true;
       else
 	/* Deallocate the final bit of the frame.  */
@@ -12647,6 +12690,11 @@ mips_expand_epilogue (bool sibcall_p)
 	  else
 	    emit_jump_insn (gen_mips_eret ());
 	}
+      else if (cfun->machine->use_hazard_barrier_return_p)
+	{
+	  rtx reg = gen_rtx_REG (Pmode, RETURN_ADDR_REGNUM);
+	  emit_jump_insn (gen_mips_hb_return_internal (reg));
+	}
       else
 	{
 	  rtx pat;
@@ -12705,6 +12753,11 @@ mips_can_use_return_insn (void)
   if (cfun->machine->interrupt_handler_p)
     return false;
 
+  /* Even if the function has a null epilogue, generating hazard barrier return
+     in epilogue handler is a lot cleaner and more manageable.  */
+  if (cfun->machine->use_hazard_barrier_return_p)
+    return false;
+
   if (!reload_completed)
     return false;
 
@@ -22476,10 +22529,9 @@ mips_promote_function_mode (const_tree type ATTRIB
 
 #undef TARGET_ATTRIBUTE_TABLE
 #define TARGET_ATTRIBUTE_TABLE mips_attribute_table
-/* All our function attributes are related to how out-of-line copies should
-   be compiled or called.  They don't in themselves prevent inlining.  */
+
 #undef TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P
-#define TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P hook_bool_const_tree_true
+#define TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P mips_function_attr_inlinable_p
 
 #undef TARGET_EXTRA_LIVE_ON_ENTRY
 #define TARGET_EXTRA_LIVE_ON_ENTRY mips_extra_live_on_entry
Index: gcc/config/mips/mips.h
===================================================================
--- gcc/config/mips/mips.h	(revision 246899)
+++ gcc/config/mips/mips.h	(working copy)
@@ -3405,6 +3405,9 @@ struct GTY(())  machine_function {
 
   /* True if GCC stored callee saved registers in the frame header.  */
   bool use_frame_header_for_callee_saved_regs;
+
+  /* True if the function should generate hazard barrier return.  */
+  bool use_hazard_barrier_return_p;
 };
 #endif
 
Index: gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
===================================================================
--- gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c	(revision 0)
+++ gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c	(revision 0)
@@ -0,0 +1,13 @@
+/* Test attribute for clearing hazards while returning.  */
+/* { dg-do compile } */
+/* { dg-options "isa_rev>=2 -mno-mips16" } */
+
+extern int bar ();
+
+int __attribute__ ((use_hazard_barrier_return))
+foo ()
+{
+  return bar ();
+}
+/* { dg-final { scan-assembler "\tjr.hb\t$31\n" } } */
+/* { dg-final { scan-assembler "\tnop\n" } } */

RE: Add support for use_hazard_barrier_return function attribute

[Matthew Fortune]

Prachi Godbole <Prachi.Godbole@imgtec.com> writes:
> Changelog:
> 
> 2017-04-25  Prachi Godbole  <prachi.godbole@imgtec.com>
> 
> gcc/
> 	* config/mips/mips.h (machine_function): New variable
> 	use_hazard_barrier_return_p.
> 	* config/mips/mips.md (UNSPEC_JRHB): New unspec.
> 	(mips_hb_return_internal): New insn pattern.
> 	* config/mips/mips.c (mips_attribute_table): Add attribute
> 	use_hazard_barrier_return.
> 	(mips_use_hazard_barrier_return_p): New static function.
> 	(mips_function_attr_inlinable_p): Likewise.
> 	(mips_compute_frame_info): Set use_hazard_barrier_return_p.  Emit
> error
> 	for unsupported architecture choice.
> 	(mips_function_ok_for_sibcall, mips_can_use_return_insn): Return
> false
> 	for use_hazard_barrier_return.
> 	(mips_expand_epilogue): Emit hazard barrier return.
> 	* doc/extend.texi: Document use_hazard_barrier_return.
> 
> gcc/testsuite/
> 	* gcc.target/mips/hazard-barrier-return-attribute.c: New test.

Sorry for not getting back to this after stage1 opened.

This looks like a great idea.  I'm slightly concerned about what will
happen if code uses this attribute and is built by older tools.  The
problem being that it will just get a warning and that may or may not
be strong enough for a user to notice they did not get a jr.hb and
then go on to get weird runtime failures.

That said we don't have this kind of feature detection for all the new
attributes relating to interrupts so perhaps we just leave this to
-Werror and/or configure time detection of the feature.

No major issues with this just a little cleanup...

> Index: gcc/doc/extend.texi
> ===================================================================
> --- gcc/doc/extend.texi	(revision 246899)
> +++ gcc/doc/extend.texi	(working copy)
> @@ -4496,6 +4496,12 @@ On MIPS targets, you can use the
> @code{nocompressi
>  to locally turn off MIPS16 and microMIPS code generation.  This attribute
>  overrides the @option{-mips16} and @option{-mmicromips} options on the
>  command line (@pxref{MIPS Options}).
> +
> +@item use_hazard_barrier_return
> +@cindex @code{use_hazard_barrier_return} function attribute, MIPS
> +This function attribute instructs the compiler to generate hazard barrier return

Missing an 'a':

... generate a hazard barrier return...

Documentation normally wraps at 74 columns which makes these lines a
bit long.

> +that clears all execution and instruction hazards while returning, instead of
> +generating a normal return instruction.
>  @end table
> 
>  @node MSP430 Function Attributes
> Index: gcc/config/mips/mips.md
> ===================================================================
> --- gcc/config/mips/mips.md	(revision 246899)
> +++ gcc/config/mips/mips.md	(working copy)
> @@ -156,6 +156,7 @@
> 
>    ;; The `.insn' pseudo-op.
>    UNSPEC_INSN_PSEUDO
> +  UNSPEC_JRHB

Add a comment on what the unspec is.

>  ])
> 
>  (define_constants
> @@ -6578,6 +6579,20 @@
>    [(set_attr "type"	"jump")
>     (set_attr "mode"	"none")])
> 
> +;; Insn to clear execution and instruction hazards while returning.
> +;; However, it doesn't clear hazards created by the insn in its delay slot.
> +;; Thus, explicitly place a nop in its delay slot.
> +
> +(define_insn "mips_hb_return_internal"
> +  [(return)
> +   (unspec_volatile [(match_operand 0 "pmode_register_operand" "")]
> +		    UNSPEC_JRHB)]
> +  ""
> +  {
> +    return "%(jr.hb\t$31%/%)";
> +  }
> +  [(set_attr "insn_count" "2")])
> +
>  ;; Normal return.
> 
>  (define_insn "<optab>_internal"
> Index: gcc/config/mips/mips.c
> ===================================================================
> --- gcc/config/mips/mips.c	(revision 246899)
> +++ gcc/config/mips/mips.c	(working copy)
> @@ -615,6 +615,7 @@ static const struct attribute_spec mips_attribute_
>      mips_handle_use_shadow_register_set_attr, false },
>    { "keep_interrupts_masked",	0, 0, false, true,  true, NULL, false },
>    { "use_debug_exception_return", 0, 0, false, true,  true, NULL, false },
> +  { "use_hazard_barrier_return", 0, 0, true, false, false, NULL, false },
>    { NULL,	   0, 0, false, false, false, NULL, false }
>  };
> 
> 
> @@ -1275,6 +1276,16 @@ mips_use_debug_exception_return_p (tree type)
>  			   TYPE_ATTRIBUTES (type)) != NULL;
>  }
> 
> +/* Check if the attribute to use hazard barrier return is set for
> +   the function declaration DECL.  */
> +
> +static bool
> +mips_use_hazard_barrier_return_p (tree decl)

Make this a const_tree so you don't have to const_cast.

> +{
> +  return lookup_attribute ("use_hazard_barrier_return",
> +			    DECL_ATTRIBUTES (decl)) != NULL;
> +}
> +

DECL_ATTRIBUTES is indented 1 space too many.

>  /* Return the set of compression modes that are explicitly required
>     by the attributes in ATTRIBUTES.  */
> 
> @@ -1460,6 +1471,21 @@ mips_can_inline_p (tree caller, tree callee)
>    return default_target_can_inline_p (caller, callee);
>  }
> 
> +/* Implement TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P.
> +
> +   A function reqeuesting clearing of all instruction and execution hazards
> +   before returning cannot be inlined - thereby not clearing any hazards.
> +   All our other function attributes are related to how out-of-line copies
> +   should be compiled or called.  They don't in themselves prevent inlining.  */
> +
> +static bool
> +mips_function_attr_inlinable_p (const_tree decl)
> +{
> +  if (mips_use_hazard_barrier_return_p (const_cast<tree>(decl)))

Remove the const_cast as above.

> +    return false;
> +  return hook_bool_const_tree_true (decl);
> +}
> +

Just return true, the various true/false hooks are only there to satisfy the
prototype required for the hook.

>  /* Handle an "interrupt" attribute with an optional argument.  */
> 
>  static tree
> @@ -7863,6 +7889,11 @@ mips_function_ok_for_sibcall (tree decl, tree exp
>        && !targetm.binds_local_p (decl))
>      return false;
> 
> +  /* Can't generate sibling calls if returning from current function using
> +     hazard barrier return.  */
> +  if (mips_use_hazard_barrier_return_p (current_function_decl))
> +    return false;
> +

I'd reword because this is not quite true:

Functions that need to return with a hazard barrier cannot sibcall
because:

1) Hazard barriers are not possible for direct jumps

2) Despite an indirect jump with hazard barrier being possible we do
   not use it so that the logic for generating a hazard barrier jump
   can be contained within the epilogue handling.

>    /* Otherwise OK.  */
>    return true;
>  }
> @@ -10943,6 +10974,17 @@ mips_compute_frame_info (void)
>  	}
>      }
> 
> +  /* Determine whether to use hazard barrier return or not.  */
> +  if (mips_use_hazard_barrier_return_p (current_function_decl))
> +    {
> +      if (mips_isa_rev < 2)
> +	error ("hazard barrier return requires MIPS ISA to be R2 or greater");

"hazard barrier returns require a MIPS32r2 processor or greater"

> +      else if (TARGET_MIPS16)
> +	error ("hazard barrier return cannot be used with MIPS16 functions");

"hazard barrier returns are not supported for MIPS16 functions"

> +      else
> +	cfun->machine->use_hazard_barrier_return_p = true;
> +    }
> +
>    frame = &cfun->machine->frame;
>    memset (frame, 0, sizeof (*frame));
>    size = get_frame_size ();
> @@ -12606,7 +12648,8 @@ mips_expand_epilogue (bool sibcall_p)
>  	       && !crtl->calls_eh_return
>  	       && !sibcall_p
>  	       && step2 > 0
> -	       && mips_unsigned_immediate_p (step2, 5, 2))
> +	       && mips_unsigned_immediate_p (step2, 5, 2)
> +	       && !cfun->machine->use_hazard_barrier_return_p)
>  	use_jraddiusp_p = true;
>        else
>  	/* Deallocate the final bit of the frame.  */
> @@ -12647,6 +12690,11 @@ mips_expand_epilogue (bool sibcall_p)
>  	  else
>  	    emit_jump_insn (gen_mips_eret ());
>  	}
> +      else if (cfun->machine->use_hazard_barrier_return_p)
> +	{
> +	  rtx reg = gen_rtx_REG (Pmode, RETURN_ADDR_REGNUM);
> +	  emit_jump_insn (gen_mips_hb_return_internal (reg));
> +	}
>        else
>  	{
>  	  rtx pat;
> @@ -12705,6 +12753,11 @@ mips_can_use_return_insn (void)
>    if (cfun->machine->interrupt_handler_p)
>      return false;
> 
> +  /* Even if the function has a null epilogue, generating hazard barrier return
> +     in epilogue handler is a lot cleaner and more manageable.  */

Even if the function has a null epilogue we choose to only create
hazard barrier returns in the epilogue expansion for simplicity.

> +  if (cfun->machine->use_hazard_barrier_return_p)
> +    return false;
> +
>    if (!reload_completed)
>      return false;
> 
> @@ -22476,10 +22529,9 @@ mips_promote_function_mode (const_tree type
> ATTRIB
> 
>  #undef TARGET_ATTRIBUTE_TABLE
>  #define TARGET_ATTRIBUTE_TABLE mips_attribute_table
> -/* All our function attributes are related to how out-of-line copies should
> -   be compiled or called.  They don't in themselves prevent inlining.
> */
> +
>  #undef TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P
> -#define TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P hook_bool_const_tree_true
> +#define TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P
> mips_function_attr_inlinable_p
> 
>  #undef TARGET_EXTRA_LIVE_ON_ENTRY
>  #define TARGET_EXTRA_LIVE_ON_ENTRY mips_extra_live_on_entry
> Index: gcc/config/mips/mips.h
> ===================================================================
> --- gcc/config/mips/mips.h	(revision 246899)
> +++ gcc/config/mips/mips.h	(working copy)
> @@ -3405,6 +3405,9 @@ struct GTY(())  machine_function {
> 
>    /* True if GCC stored callee saved registers in the frame header.  */
>    bool use_frame_header_for_callee_saved_regs;
> +
> +  /* True if the function should generate hazard barrier return.  */

... generate a hazard barrier...

> +  bool use_hazard_barrier_return_p;
>  };
>  #endif
> 
> Index: gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
> ===================================================================
> --- gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
> 	(revision 0)
> +++ gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
> 	(revision 0)
> @@ -0,0 +1,13 @@
> +/* Test attribute for clearing hazards while returning.  */
> +/* { dg-do compile } */
> +/* { dg-options "isa_rev>=2 -mno-mips16" } */
> +
> +extern int bar ();
> +
> +int __attribute__ ((use_hazard_barrier_return))
> +foo ()
> +{
> +  return bar ();
> +}
> +/* { dg-final { scan-assembler "\tjr.hb\t$31\n" } } */
> +/* { dg-final { scan-assembler "\tnop\n" } } */

The separate tests here don't guarantee the nop is in the delay slot, it
needs to check for both in one scan-assembler to do that. It will be
something like this (not tested):

> +/* { dg-final { scan-assembler "\tjr.hb\t$31\n\tnop\n" } } */

Can you extend this test to also cover the inlining restriction? I think
the test already covers sibcalls as it would normally have been a sibcall.
The extra bit of test will be something like:

int
another ()
{
  return foo ();
}
/* { dg-final { scan-assembler "(j|jal|bc|balc)\tfoo" } } */

Please post the updated patch but I'm happy for you to commit if you
understand all my comments. 

Thanks,
Matthew

RE: Add support for use_hazard_barrier_return function attribute

[Prachi Godbole]

Please find the updated patch below. I hope I've covered everything.
I've added the test for inline restriction, could you check if I got all the options correct?

Changelog:

2017-06-23  Prachi Godbole  <prachi.godbole@imgtec.com>

gcc/
	* config/mips/mips.h (machine_function): New variable
	use_hazard_barrier_return_p.
	* config/mips/mips.md (UNSPEC_JRHB): New unspec.
	(mips_hb_return_internal): New insn pattern.
	* config/mips/mips.c (mips_attribute_table): Add attribute
	use_hazard_barrier_return.
	(mips_use_hazard_barrier_return_p): New static function.
	(mips_function_attr_inlinable_p): Likewise.
	(mips_compute_frame_info): Set use_hazard_barrier_return_p.  Emit error
	for unsupported architecture choice.
	(mips_function_ok_for_sibcall, mips_can_use_return_insn): Return false
	for use_hazard_barrier_return.
	(mips_expand_epilogue): Emit hazard barrier return.
	* doc/extend.texi: Document use_hazard_barrier_return.

gcc/testsuite/
	* gcc.target/mips/hazard-barrier-return-attribute.c: New tests.

Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 246899)
+++ gcc/doc/extend.texi	(working copy)
@@ -4496,6 +4496,12 @@ On MIPS targets, you can use the @code{nocompressi
 to locally turn off MIPS16 and microMIPS code generation.  This attribute
 overrides the @option{-mips16} and @option{-mmicromips} options on the
 command line (@pxref{MIPS Options}).
+
+@item use_hazard_barrier_return
+@cindex @code{use_hazard_barrier_return} function attribute, MIPS
+This function attribute instructs the compiler to generate a hazard
+barrier return that clears all execution and instruction hazards while
+returning, instead of generating a normal return instruction.
 @end table
 
 @node MSP430 Function Attributes
Index: gcc/config/mips/mips.md
===================================================================
--- gcc/config/mips/mips.md	(revision 246899)
+++ gcc/config/mips/mips.md	(working copy)
@@ -156,6 +156,9 @@
 
   ;; The `.insn' pseudo-op.
   UNSPEC_INSN_PSEUDO
+
+  ;; Hazard barrier return.
+  UNSPEC_JRHB
 ])
 
 (define_constants
@@ -6578,6 +6581,20 @@
   [(set_attr "type"	"jump")
    (set_attr "mode"	"none")])
 
+;; Insn to clear execution and instruction hazards while returning.
+;; However, it doesn't clear hazards created by the insn in its delay slot.
+;; Thus, explicitly place a nop in its delay slot.
+
+(define_insn "mips_hb_return_internal"
+  [(return)
+   (unspec_volatile [(match_operand 0 "pmode_register_operand" "")]
+		    UNSPEC_JRHB)]
+  ""
+  {
+    return "%(jr.hb\t$31%/%)";
+  }
+  [(set_attr "insn_count" "2")])
+
 ;; Normal return.
 
 (define_insn "<optab>_internal"
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	(revision 246899)
+++ gcc/config/mips/mips.c	(working copy)
@@ -615,6 +615,7 @@ static const struct attribute_spec mips_attribute_
     mips_handle_use_shadow_register_set_attr, false },
   { "keep_interrupts_masked",	0, 0, false, true,  true, NULL, false },
   { "use_debug_exception_return", 0, 0, false, true,  true, NULL, false },
+  { "use_hazard_barrier_return", 0, 0, true, false, false, NULL, false },
   { NULL,	   0, 0, false, false, false, NULL, false }
 };
 

@@ -1275,6 +1276,16 @@ mips_use_debug_exception_return_p (tree type)
 			   TYPE_ATTRIBUTES (type)) != NULL;
 }
 
+/* Check if the attribute to use hazard barrier return is set for
+   the function declaration DECL.  */
+
+static bool
+mips_use_hazard_barrier_return_p (const_tree decl)
+{
+  return lookup_attribute ("use_hazard_barrier_return",
+			   DECL_ATTRIBUTES (decl)) != NULL;
+}
+
 /* Return the set of compression modes that are explicitly required
    by the attributes in ATTRIBUTES.  */
 
@@ -1460,6 +1471,21 @@ mips_can_inline_p (tree caller, tree callee)
   return default_target_can_inline_p (caller, callee);
 }
 
+/* Implement TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P.
+
+   A function reqeuesting clearing of all instruction and execution hazards
+   before returning cannot be inlined - thereby not clearing any hazards.
+   All our other function attributes are related to how out-of-line copies
+   should be compiled or called.  They don't in themselves prevent inlining.  */
+
+static bool
+mips_function_attr_inlinable_p (const_tree decl)
+{
+  if (mips_use_hazard_barrier_return_p (decl))
+    return false;
+  return true;
+}
+
 /* Handle an "interrupt" attribute with an optional argument.  */
 
 static tree
@@ -7863,6 +7889,17 @@ mips_function_ok_for_sibcall (tree decl, tree exp
       && !targetm.binds_local_p (decl))
     return false;
 
+  /* Functions that need to return with a hazard barrier cannot sibcall because:
+
+     1) Hazard barriers are not possible for direct jumps
+
+     2) Despite an indirect jump with hazard barrier being possible we do
+	not use it so that the logic for generating a hazard barrier jump
+	can be contained within the epilogue handling.  */
+
+  if (mips_use_hazard_barrier_return_p (current_function_decl))
+    return false;
+
   /* Otherwise OK.  */
   return true;
 }
@@ -10943,6 +10980,17 @@ mips_compute_frame_info (void)
 	}
     }
 
+  /* Determine whether to use hazard barrier return or not.  */
+  if (mips_use_hazard_barrier_return_p (current_function_decl))
+    {
+      if (mips_isa_rev < 2)
+	error ("hazard barrier returns require a MIPS32r2 processor or greater");
+      else if (TARGET_MIPS16)
+	error ("hazard barrier returns are not supported for MIPS16 functions");
+      else
+	cfun->machine->use_hazard_barrier_return_p = true;
+    }
+
   frame = &cfun->machine->frame;
   memset (frame, 0, sizeof (*frame));
   size = get_frame_size ();
@@ -12606,7 +12654,8 @@ mips_expand_epilogue (bool sibcall_p)
 	       && !crtl->calls_eh_return
 	       && !sibcall_p
 	       && step2 > 0
-	       && mips_unsigned_immediate_p (step2, 5, 2))
+	       && mips_unsigned_immediate_p (step2, 5, 2)
+	       && !cfun->machine->use_hazard_barrier_return_p)
 	use_jraddiusp_p = true;
       else
 	/* Deallocate the final bit of the frame.  */
@@ -12647,6 +12696,11 @@ mips_expand_epilogue (bool sibcall_p)
 	  else
 	    emit_jump_insn (gen_mips_eret ());
 	}
+      else if (cfun->machine->use_hazard_barrier_return_p)
+	{
+	  rtx reg = gen_rtx_REG (Pmode, RETURN_ADDR_REGNUM);
+	  emit_jump_insn (gen_mips_hb_return_internal (reg));
+	}
       else
 	{
 	  rtx pat;
@@ -12705,6 +12759,11 @@ mips_can_use_return_insn (void)
   if (cfun->machine->interrupt_handler_p)
     return false;
 
+  /* Even if the function has a null epilogue we choose to only create hazard
+     barrier returns in the epilogue expansion for simplicity.  */
+  if (cfun->machine->use_hazard_barrier_return_p)
+    return false;
+
   if (!reload_completed)
     return false;
 
@@ -22476,10 +22535,9 @@ mips_promote_function_mode (const_tree type ATTRIB
 
 #undef TARGET_ATTRIBUTE_TABLE
 #define TARGET_ATTRIBUTE_TABLE mips_attribute_table
-/* All our function attributes are related to how out-of-line copies should
-   be compiled or called.  They don't in themselves prevent inlining.  */
+
 #undef TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P
-#define TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P hook_bool_const_tree_true
+#define TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P mips_function_attr_inlinable_p
 
 #undef TARGET_EXTRA_LIVE_ON_ENTRY
 #define TARGET_EXTRA_LIVE_ON_ENTRY mips_extra_live_on_entry
Index: gcc/config/mips/mips.h
===================================================================
--- gcc/config/mips/mips.h	(revision 246899)
+++ gcc/config/mips/mips.h	(working copy)
@@ -3405,6 +3405,9 @@ struct GTY(())  machine_function {
 
   /* True if GCC stored callee saved registers in the frame header.  */
   bool use_frame_header_for_callee_saved_regs;
+
+  /* True if the function should generate a hazard barrier return.  */
+  bool use_hazard_barrier_return_p;
 };
 #endif
 
Index: gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
===================================================================
--- gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c	(revision 0)
+++ gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c	(revision 0)
@@ -0,0 +1,20 @@
+/* Test attribute for clearing hazards while returning.  */
+/* { dg-do compile } */
+/* { dg-options "isa_rev>=2 -mno-mips16" } */
+
+extern int bar ();
+
+int __attribute__ ((use_hazard_barrier_return))
+foo ()
+{
+  return bar ();
+}
+/* { dg-final { scan-assembler "\tjr.hb\t\\\$31\n\tnop\n" } } */
+
+/* Test to confirm that foo() is not inlined. */
+int
+foo1 ()
+{
+  return foo ();
+}
+/* { dg-final { scan-assembler "((j|jal|bc|balc)\tfoo)|((jr|jalr|jrc|jalrc)\t\\\$25)" } } */

-----Original Message-----
From: Matthew Fortune 
Sent: Wednesday, June 14, 2017 9:48 PM
To: Prachi Godbole; gcc-patches@gcc.gnu.org
Subject: RE: Add support for use_hazard_barrier_return function attribute

Prachi Godbole <Prachi.Godbole@imgtec.com> writes:
> Changelog:
> 
> 2017-04-25  Prachi Godbole  <prachi.godbole@imgtec.com>
> 
> gcc/
> 	* config/mips/mips.h (machine_function): New variable
> 	use_hazard_barrier_return_p.
> 	* config/mips/mips.md (UNSPEC_JRHB): New unspec.
> 	(mips_hb_return_internal): New insn pattern.
> 	* config/mips/mips.c (mips_attribute_table): Add attribute
> 	use_hazard_barrier_return.
> 	(mips_use_hazard_barrier_return_p): New static function.
> 	(mips_function_attr_inlinable_p): Likewise.
> 	(mips_compute_frame_info): Set use_hazard_barrier_return_p.  Emit 
> error
> 	for unsupported architecture choice.
> 	(mips_function_ok_for_sibcall, mips_can_use_return_insn): Return 
> false
> 	for use_hazard_barrier_return.
> 	(mips_expand_epilogue): Emit hazard barrier return.
> 	* doc/extend.texi: Document use_hazard_barrier_return.
> 
> gcc/testsuite/
> 	* gcc.target/mips/hazard-barrier-return-attribute.c: New test.

Sorry for not getting back to this after stage1 opened.

This looks like a great idea.  I'm slightly concerned about what will happen if code uses this attribute and is built by older tools.  The problem being that it will just get a warning and that may or may not be strong enough for a user to notice they did not get a jr.hb and then go on to get weird runtime failures.

That said we don't have this kind of feature detection for all the new attributes relating to interrupts so perhaps we just leave this to -Werror and/or configure time detection of the feature.

No major issues with this just a little cleanup...

> Index: gcc/doc/extend.texi
> ===================================================================
> --- gcc/doc/extend.texi	(revision 246899)
> +++ gcc/doc/extend.texi	(working copy)
> @@ -4496,6 +4496,12 @@ On MIPS targets, you can use the 
> @code{nocompressi  to locally turn off MIPS16 and microMIPS code 
> generation.  This attribute  overrides the @option{-mips16} and 
> @option{-mmicromips} options on the  command line (@pxref{MIPS 
> Options}).
> +
> +@item use_hazard_barrier_return
> +@cindex @code{use_hazard_barrier_return} function attribute, MIPS 
> +This function attribute instructs the compiler to generate hazard 
> +barrier return

Missing an 'a':

... generate a hazard barrier return...

Documentation normally wraps at 74 columns which makes these lines a bit long.

> +that clears all execution and instruction hazards while returning, 
> +instead of generating a normal return instruction.
>  @end table
> 
>  @node MSP430 Function Attributes
> Index: gcc/config/mips/mips.md
> ===================================================================
> --- gcc/config/mips/mips.md	(revision 246899)
> +++ gcc/config/mips/mips.md	(working copy)
> @@ -156,6 +156,7 @@
> 
>    ;; The `.insn' pseudo-op.
>    UNSPEC_INSN_PSEUDO
> +  UNSPEC_JRHB

Add a comment on what the unspec is.

>  ])
> 
>  (define_constants
> @@ -6578,6 +6579,20 @@
>    [(set_attr "type"	"jump")
>     (set_attr "mode"	"none")])
> 
> +;; Insn to clear execution and instruction hazards while returning.
> +;; However, it doesn't clear hazards created by the insn in its delay slot.
> +;; Thus, explicitly place a nop in its delay slot.
> +
> +(define_insn "mips_hb_return_internal"
> +  [(return)
> +   (unspec_volatile [(match_operand 0 "pmode_register_operand" "")]
> +		    UNSPEC_JRHB)]
> +  ""
> +  {
> +    return "%(jr.hb\t$31%/%)";
> +  }
> +  [(set_attr "insn_count" "2")])
> +
>  ;; Normal return.
> 
>  (define_insn "<optab>_internal"
> Index: gcc/config/mips/mips.c
> ===================================================================
> --- gcc/config/mips/mips.c	(revision 246899)
> +++ gcc/config/mips/mips.c	(working copy)
> @@ -615,6 +615,7 @@ static const struct attribute_spec mips_attribute_
>      mips_handle_use_shadow_register_set_attr, false },
>    { "keep_interrupts_masked",	0, 0, false, true,  true, NULL, false },
>    { "use_debug_exception_return", 0, 0, false, true,  true, NULL, 
> false },
> +  { "use_hazard_barrier_return", 0, 0, true, false, false, NULL, 
> + false },
>    { NULL,	   0, 0, false, false, false, NULL, false }
>  };
> 
> 
> @@ -1275,6 +1276,16 @@ mips_use_debug_exception_return_p (tree type)
>  			   TYPE_ATTRIBUTES (type)) != NULL;  }
> 
> +/* Check if the attribute to use hazard barrier return is set for
> +   the function declaration DECL.  */
> +
> +static bool
> +mips_use_hazard_barrier_return_p (tree decl)

Make this a const_tree so you don't have to const_cast.

> +{
> +  return lookup_attribute ("use_hazard_barrier_return",
> +			    DECL_ATTRIBUTES (decl)) != NULL; }
> +

DECL_ATTRIBUTES is indented 1 space too many.

>  /* Return the set of compression modes that are explicitly required
>     by the attributes in ATTRIBUTES.  */
> 
> @@ -1460,6 +1471,21 @@ mips_can_inline_p (tree caller, tree callee)
>    return default_target_can_inline_p (caller, callee);  }
> 
> +/* Implement TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P.
> +
> +   A function reqeuesting clearing of all instruction and execution hazards
> +   before returning cannot be inlined - thereby not clearing any hazards.
> +   All our other function attributes are related to how out-of-line copies
> +   should be compiled or called.  They don't in themselves prevent 
> + inlining.  */
> +
> +static bool
> +mips_function_attr_inlinable_p (const_tree decl) {
> +  if (mips_use_hazard_barrier_return_p (const_cast<tree>(decl)))

Remove the const_cast as above.

> +    return false;
> +  return hook_bool_const_tree_true (decl); }
> +

Just return true, the various true/false hooks are only there to satisfy the prototype required for the hook.

>  /* Handle an "interrupt" attribute with an optional argument.  */
> 
>  static tree
> @@ -7863,6 +7889,11 @@ mips_function_ok_for_sibcall (tree decl, tree exp
>        && !targetm.binds_local_p (decl))
>      return false;
> 
> +  /* Can't generate sibling calls if returning from current function using
> +     hazard barrier return.  */
> +  if (mips_use_hazard_barrier_return_p (current_function_decl))
> +    return false;
> +

I'd reword because this is not quite true:

Functions that need to return with a hazard barrier cannot sibcall
because:

1) Hazard barriers are not possible for direct jumps

2) Despite an indirect jump with hazard barrier being possible we do
   not use it so that the logic for generating a hazard barrier jump
   can be contained within the epilogue handling.

>    /* Otherwise OK.  */
>    return true;
>  }
> @@ -10943,6 +10974,17 @@ mips_compute_frame_info (void)
>  	}
>      }
> 
> +  /* Determine whether to use hazard barrier return or not.  */
> +  if (mips_use_hazard_barrier_return_p (current_function_decl))
> +    {
> +      if (mips_isa_rev < 2)
> +	error ("hazard barrier return requires MIPS ISA to be R2 or 
> +greater");

"hazard barrier returns require a MIPS32r2 processor or greater"

> +      else if (TARGET_MIPS16)
> +	error ("hazard barrier return cannot be used with MIPS16 
> +functions");

"hazard barrier returns are not supported for MIPS16 functions"

> +      else
> +	cfun->machine->use_hazard_barrier_return_p = true;
> +    }
> +
>    frame = &cfun->machine->frame;
>    memset (frame, 0, sizeof (*frame));
>    size = get_frame_size ();
> @@ -12606,7 +12648,8 @@ mips_expand_epilogue (bool sibcall_p)
>  	       && !crtl->calls_eh_return
>  	       && !sibcall_p
>  	       && step2 > 0
> -	       && mips_unsigned_immediate_p (step2, 5, 2))
> +	       && mips_unsigned_immediate_p (step2, 5, 2)
> +	       && !cfun->machine->use_hazard_barrier_return_p)
>  	use_jraddiusp_p = true;
>        else
>  	/* Deallocate the final bit of the frame.  */ @@ -12647,6 +12690,11 
> @@ mips_expand_epilogue (bool sibcall_p)
>  	  else
>  	    emit_jump_insn (gen_mips_eret ());
>  	}
> +      else if (cfun->machine->use_hazard_barrier_return_p)
> +	{
> +	  rtx reg = gen_rtx_REG (Pmode, RETURN_ADDR_REGNUM);
> +	  emit_jump_insn (gen_mips_hb_return_internal (reg));
> +	}
>        else
>  	{
>  	  rtx pat;
> @@ -12705,6 +12753,11 @@ mips_can_use_return_insn (void)
>    if (cfun->machine->interrupt_handler_p)
>      return false;
> 
> +  /* Even if the function has a null epilogue, generating hazard barrier return
> +     in epilogue handler is a lot cleaner and more manageable.  */

Even if the function has a null epilogue we choose to only create hazard barrier returns in the epilogue expansion for simplicity.

> +  if (cfun->machine->use_hazard_barrier_return_p)
> +    return false;
> +
>    if (!reload_completed)
>      return false;
> 
> @@ -22476,10 +22529,9 @@ mips_promote_function_mode (const_tree type 
> ATTRIB
> 
>  #undef TARGET_ATTRIBUTE_TABLE
>  #define TARGET_ATTRIBUTE_TABLE mips_attribute_table
> -/* All our function attributes are related to how out-of-line copies should
> -   be compiled or called.  They don't in themselves prevent inlining.
> */
> +
>  #undef TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P
> -#define TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P 
> hook_bool_const_tree_true
> +#define TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P
> mips_function_attr_inlinable_p
> 
>  #undef TARGET_EXTRA_LIVE_ON_ENTRY
>  #define TARGET_EXTRA_LIVE_ON_ENTRY mips_extra_live_on_entry
> Index: gcc/config/mips/mips.h
> ===================================================================
> --- gcc/config/mips/mips.h	(revision 246899)
> +++ gcc/config/mips/mips.h	(working copy)
> @@ -3405,6 +3405,9 @@ struct GTY(())  machine_function {
> 
>    /* True if GCC stored callee saved registers in the frame header.  */
>    bool use_frame_header_for_callee_saved_regs;
> +
> +  /* True if the function should generate hazard barrier return.  */

... generate a hazard barrier...

> +  bool use_hazard_barrier_return_p;
>  };
>  #endif
> 
> Index: gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
> ===================================================================
> --- gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
> 	(revision 0)
> +++ gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
> 	(revision 0)
> @@ -0,0 +1,13 @@
> +/* Test attribute for clearing hazards while returning.  */
> +/* { dg-do compile } */
> +/* { dg-options "isa_rev>=2 -mno-mips16" } */
> +
> +extern int bar ();
> +
> +int __attribute__ ((use_hazard_barrier_return)) foo () {
> +  return bar ();
> +}
> +/* { dg-final { scan-assembler "\tjr.hb\t$31\n" } } */
> +/* { dg-final { scan-assembler "\tnop\n" } } */

The separate tests here don't guarantee the nop is in the delay slot, it needs to check for both in one scan-assembler to do that. It will be something like this (not tested):

> +/* { dg-final { scan-assembler "\tjr.hb\t$31\n\tnop\n" } } */

Can you extend this test to also cover the inlining restriction? I think the test already covers sibcalls as it would normally have been a sibcall.
The extra bit of test will be something like:

int
another ()
{
  return foo ();
}
/* { dg-final { scan-assembler "(j|jal|bc|balc)\tfoo" } } */

Please post the updated patch but I'm happy for you to commit if you understand all my comments. 

Thanks,
Matthew

RE: Add support for use_hazard_barrier_return function attribute

[Maciej W. Rozycki]

On Fri, 23 Jun 2017, Prachi Godbole wrote:

> Index: gcc/config/mips/mips.md
> ===================================================================
> --- gcc/config/mips/mips.md	(revision 246899)
> +++ gcc/config/mips/mips.md	(working copy)
> @@ -6578,6 +6581,20 @@
>    [(set_attr "type"	"jump")
>     (set_attr "mode"	"none")])
>  
> +;; Insn to clear execution and instruction hazards while returning.
> +;; However, it doesn't clear hazards created by the insn in its delay slot.
> +;; Thus, explicitly place a nop in its delay slot.
> +
> +(define_insn "mips_hb_return_internal"
> +  [(return)
> +   (unspec_volatile [(match_operand 0 "pmode_register_operand" "")]
> +		    UNSPEC_JRHB)]
> +  ""
> +  {
> +    return "%(jr.hb\t$31%/%)";
> +  }
> +  [(set_attr "insn_count" "2")])
> +
>  ;; Normal return.
>  
>  (define_insn "<optab>_internal"

 Nothing wrong with your proposed change, but overall I wonder if (as a 
follow-up change) we could find a nonintrusive way to have this pattern 
(and `clear_hazard_<mode>' as well) produce JRS.HB rather than JR.HB in 
microMIPS compilations, as using the 32-bit delay-slot NOP encoding where 
the 16-bit one would do is obviously a tiny, but completely unnecessary 
code space loss (and we do care about code space losses in microMIPS 
compilations; conserving space is the very purpose of the microMIPS ISA 
after all).

 Of course it wouldn't do if we rewrote the instruction pattern as 
"%(jr%!.hb\t$31%/%)" here, because the NOP that follows would have to come 
from an RTL instruction for `%!' to have any effect.  But perhaps we could 
emit RTL instead somehow rather than hardcoding the NOP with `%/'?

  Maciej

RE: Add support for use_hazard_barrier_return function attribute

[Matthew Fortune]

Prachi Godbole <Prachi.Godbole@imgtec.com> writes:
> Please find the updated patch below. I hope I've covered everything.
> I've added the test for inline restriction, could you check if I got all
> the options correct?

I think the test is probably good enough. It is a little too forgiving due
to handling the indirect call case to foo which could just detect an
indirect call from foo to bar (the placement of a scan-assembler in the
.c file has no impact on where in the generated output it will match in
the corresponding .s). Given that the test would fail appropriately on
a bare metal configuration (which is where this is likely to be most
useful) then I think that is sufficient.

Watch out for the long lines in comments. There is one that is hitting
80cols noted below to tweak before committing.

> Changelog:
> 
> 2017-06-23  Prachi Godbole  <prachi.godbole@imgtec.com>
> 
> gcc/
> 	* config/mips/mips.h (machine_function): New variable
> 	use_hazard_barrier_return_p.
> 	* config/mips/mips.md (UNSPEC_JRHB): New unspec.
> 	(mips_hb_return_internal): New insn pattern.
> 	* config/mips/mips.c (mips_attribute_table): Add attribute
> 	use_hazard_barrier_return.
> 	(mips_use_hazard_barrier_return_p): New static function.
> 	(mips_function_attr_inlinable_p): Likewise.
> 	(mips_compute_frame_info): Set use_hazard_barrier_return_p.  Emit error
> 	for unsupported architecture choice.
> 	(mips_function_ok_for_sibcall, mips_can_use_return_insn): Return false
> 	for use_hazard_barrier_return.
> 	(mips_expand_epilogue): Emit hazard barrier return.
> 	* doc/extend.texi: Document use_hazard_barrier_return.
> 
> gcc/testsuite/
> 	* gcc.target/mips/hazard-barrier-return-attribute.c: New tests.

OK to commit.

> ===================================================================
> --- gcc/config/mips/mips.c	(revision 246899)
> +++ gcc/config/mips/mips.c	(working copy)
> @@ -7863,6 +7889,17 @@ mips_function_ok_for_sibcall (tree decl, tree exp
>        && !targetm.binds_local_p (decl))
>      return false;
> +  /* Functions that need to return with a hazard barrier cannot sibcall because:

Long line for a comment above.

> +
> +     1) Hazard barriers are not possible for direct jumps
> +
> +     2) Despite an indirect jump with hazard barrier being possible we do
> +	not use it so that the logic for generating a hazard barrier jump
> +	can be contained within the epilogue handling.  */
> +
> +  if (mips_use_hazard_barrier_return_p (current_function_decl))
> +    return false;
> +
>    /* Otherwise OK.  */
>    return true;
>  }

Thanks for the new feature!

Matthew

RE: Add support for use_hazard_barrier_return function attribute

[Matthew Fortune]

Maciej Rozycki <Maciej.Rozycki@imgtec.com> writes:
> On Fri, 23 Jun 2017, Prachi Godbole wrote:
> 
> > Index: gcc/config/mips/mips.md
> > ===================================================================
> > --- gcc/config/mips/mips.md	(revision 246899)
> > +++ gcc/config/mips/mips.md	(working copy)
> > @@ -6578,6 +6581,20 @@
> >    [(set_attr "type"	"jump")
> >     (set_attr "mode"	"none")])
> >
> > +;; Insn to clear execution and instruction hazards while returning.
> > +;; However, it doesn't clear hazards created by the insn in its delay
> slot.
> > +;; Thus, explicitly place a nop in its delay slot.
> > +
> > +(define_insn "mips_hb_return_internal"
> > +  [(return)
> > +   (unspec_volatile [(match_operand 0 "pmode_register_operand" "")]
> > +		    UNSPEC_JRHB)]
> > +  ""
> > +  {
> > +    return "%(jr.hb\t$31%/%)";
> > +  }
> > +  [(set_attr "insn_count" "2")])
> > +
> >  ;; Normal return.
> >
> >  (define_insn "<optab>_internal"
> 
>  Nothing wrong with your proposed change, but overall I wonder if (as a
> follow-up change) we could find a nonintrusive way to have this pattern
> (and `clear_hazard_<mode>' as well) produce JRS.HB rather than JR.HB in
> microMIPS compilations, as using the 32-bit delay-slot NOP encoding
> where the 16-bit one would do is obviously a tiny, but completely
> unnecessary code space loss (and we do care about code space losses in
> microMIPS compilations; conserving space is the very purpose of the
> microMIPS ISA after all).
> 
>  Of course it wouldn't do if we rewrote the instruction pattern as
> "%(jr%!.hb\t$31%/%)" here, because the NOP that follows would have to
> come from an RTL instruction for `%!' to have any effect.  But perhaps
> we could emit RTL instead somehow rather than hardcoding the NOP with
> `%/'?

I think this case is so specialist we can safely just switch to writing
out the NOP directly in the output pattern just keeping the %(%) for
noreorder. This code will have to be reworked with microMIPSR6 when
submitted so it can be handled then; good spot to use jrs.hb.

Thanks,
Matthew

RE: Add support for use_hazard_barrier_return function attribute

[Maciej W. Rozycki]

On Thu, 6 Jul 2017, Matthew Fortune wrote:

> >  Nothing wrong with your proposed change, but overall I wonder if (as a
> > follow-up change) we could find a nonintrusive way to have this pattern
> > (and `clear_hazard_<mode>' as well) produce JRS.HB rather than JR.HB in
> > microMIPS compilations, as using the 32-bit delay-slot NOP encoding
> > where the 16-bit one would do is obviously a tiny, but completely
> > unnecessary code space loss (and we do care about code space losses in
> > microMIPS compilations; conserving space is the very purpose of the
> > microMIPS ISA after all).
> > 
> >  Of course it wouldn't do if we rewrote the instruction pattern as
> > "%(jr%!.hb\t$31%/%)" here, because the NOP that follows would have to
> > come from an RTL instruction for `%!' to have any effect.  But perhaps
> > we could emit RTL instead somehow rather than hardcoding the NOP with
> > `%/'?
> 
> I think this case is so specialist we can safely just switch to writing
> out the NOP directly in the output pattern just keeping the %(%) for
> noreorder. This code will have to be reworked with microMIPSR6 when
> submitted so it can be handled then; good spot to use jrs.hb.

 It does not matter for `%!' whether you use `%/' or spell out `nop' 
literally.  I was more concerned about getting the instruction count 
correctly, which would be 1.5 for the JRS.HB case, however I think you can 
just set the `length' attribute directly, to 6.

 Still the issue of having separate almost identical patterns remains, as 
barring the use of `%!' I think you'll need to qualify them with 
TARGET_MICROMIPS and !TARGET_MICROMIPS respectively, to have different 
instruction mnemonics.  In this case I think you could write (untested):

(define_insn "mips_hb_return_internal"
  [(return)
   (unspec_volatile [(match_operand 0 "pmode_register_operand" ",")]
               UNSPEC_JRHB)]
  ""
  "@
   %(jrs.hb\t$31%/%)
   %(jr.hb\t$31%/%)"
  [(set_attr "compression" "micromips,*")
   (set_attr "length" "6,8")])

however the equivalent for `clear_hazard_<mode>' would be rather horrible 
(OTOH eventually it should use ADDIUPC in its SImode microMIPS variant, so 
perhaps this is acceptable as we'll have multiple different sequences 
anyway).

 For microMIPSr6 we'll then just have another variant with no delay slot.

  Maciej