* [PATCH] [MIPS] Hazard barrier return support
@ 2021-08-16 14:42 Dragan Mladjenovic
2021-08-16 19:16 ` Andrew Pinski
0 siblings, 1 reply; 6+ messages in thread
From: Dragan Mladjenovic @ 2021-08-16 14:42 UTC (permalink / raw)
To: gcc-patches
This patch allows a function to request clearing of all instruction and execution
hazards upon normal return via __attribute__ ((use_hazard_barrier_return)).
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.
---
Rehash of original patch posted by Prachi with minimal changes. Tested against
mips-mti-elf with mips32r2/-EB and mips32r2/-EB/-micromips.
gcc/config/mips/mips.c | 58 +++++++++++++++++--
gcc/config/mips/mips.h | 3 +
gcc/config/mips/mips.md | 15 +++++
gcc/doc/extend.texi | 6 ++
.../mips/hazard-barrier-return-attribute.c | 20 +++++++
5 files changed, 98 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 89d1be6cea6..6ce12fce52e 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -630,6 +630,7 @@ static const struct attribute_spec mips_attribute_table[] = {
mips_handle_use_shadow_register_set_attr, NULL },
{ "keep_interrupts_masked", 0, 0, false, true, true, false, NULL, NULL },
{ "use_debug_exception_return", 0, 0, false, true, true, false, NULL, NULL },
+ { "use_hazard_barrier_return", 0, 0, true, false, false, false, NULL, NULL },
{ NULL, 0, 0, false, false, false, false, NULL, NULL }
};
\f
@@ -1309,6 +1310,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. */
@@ -1494,6 +1505,19 @@ mips_can_inline_p (tree caller, tree callee)
return default_target_can_inline_p (caller, callee);
}
+/* Implement TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P.
+
+ A function requesting 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)
+{
+ return !mips_use_hazard_barrier_return_p (decl);
+}
+
/* Handle an "interrupt" attribute with an optional argument. */
static tree
@@ -7921,6 +7945,11 @@ mips_function_ok_for_sibcall (tree decl, tree exp ATTRIBUTE_UNUSED)
&& !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;
}
@@ -11008,6 +11037,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 ();
@@ -12671,7 +12711,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. */
@@ -12712,6 +12753,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;
@@ -12770,6 +12816,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;
@@ -22777,10 +22828,9 @@ mips_asm_file_end (void)
#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
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 47aac9d3d61..bae9ffe9b3c 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -3376,6 +3376,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
diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
index 455b9b802f6..dee71dc1fb0 100644
--- a/gcc/config/mips/mips.md
+++ b/gcc/config/mips/mips.md
@@ -159,6 +159,7 @@
;; The `.insn' pseudo-op.
UNSPEC_INSN_PSEUDO
+ UNSPEC_JRHB
])
(define_constants
@@ -6660,6 +6661,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"
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 49df8e6dc38..8d2a0a23af6 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -5540,6 +5540,12 @@ On MIPS targets, you can use the @code{nocompression} function attribute
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
diff --git a/gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c b/gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
new file mode 100644
index 00000000000..3575af44dcd
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
@@ -0,0 +1,20 @@
+/* Test attribute for clearing hazards while returning. */
+/* { dg-do compile } */
+/* { dg-options "isa_rev>=2 -mno-mips16" } */
+
+extern int bar ();
+
+static int __attribute__ ((use_hazard_barrier_return))
+foo0 ()
+{
+ return bar ();
+}
+
+int
+foo1 ()
+{
+ return foo0 ();
+}
+
+/* { dg-final { scan-assembler "foo0:" } } */
+/* { dg-final { scan-assembler-times "\tjr.hb\t\\\$31\n\tnop\\n" 1 } } */
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [MIPS] Hazard barrier return support
2021-08-16 14:42 [PATCH] [MIPS] Hazard barrier return support Dragan Mladjenovic
@ 2021-08-16 19:16 ` Andrew Pinski
2021-08-16 20:40 ` Dragan Mladjenovic
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Andrew Pinski @ 2021-08-16 19:16 UTC (permalink / raw)
To: Dragan Mladjenovic; +Cc: gcc-patches
On Mon, Aug 16, 2021 at 7:43 AM Dragan Mladjenovic via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> This patch allows a function to request clearing of all instruction and execution
> hazards upon normal return via __attribute__ ((use_hazard_barrier_return)).
>
> 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.
> ---
> Rehash of original patch posted by Prachi with minimal changes. Tested against
> mips-mti-elf with mips32r2/-EB and mips32r2/-EB/-micromips.
>
> gcc/config/mips/mips.c | 58 +++++++++++++++++--
> gcc/config/mips/mips.h | 3 +
> gcc/config/mips/mips.md | 15 +++++
> gcc/doc/extend.texi | 6 ++
> .../mips/hazard-barrier-return-attribute.c | 20 +++++++
> 5 files changed, 98 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
>
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> index 89d1be6cea6..6ce12fce52e 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -630,6 +630,7 @@ static const struct attribute_spec mips_attribute_table[] = {
> mips_handle_use_shadow_register_set_attr, NULL },
> { "keep_interrupts_masked", 0, 0, false, true, true, false, NULL, NULL },
> { "use_debug_exception_return", 0, 0, false, true, true, false, NULL, NULL },
> + { "use_hazard_barrier_return", 0, 0, true, false, false, false, NULL, NULL },
> { NULL, 0, 0, false, false, false, false, NULL, NULL }
> };
>
> @@ -1309,6 +1310,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. */
>
> @@ -1494,6 +1505,19 @@ mips_can_inline_p (tree caller, tree callee)
> return default_target_can_inline_p (caller, callee);
> }
>
> +/* Implement TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P.
> +
> + A function requesting 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)
> +{
> + return !mips_use_hazard_barrier_return_p (decl);
> +}
> +
> /* Handle an "interrupt" attribute with an optional argument. */
>
> static tree
> @@ -7921,6 +7945,11 @@ mips_function_ok_for_sibcall (tree decl, tree exp ATTRIBUTE_UNUSED)
> && !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;
> }
> @@ -11008,6 +11037,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");
Just a small nit, is MIPS64r2 ok too? Also did you did you test it
for MIPS64 too? I still partly care about MIPS64.
Thanks,
Andrew
> + 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 ();
> @@ -12671,7 +12711,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. */
> @@ -12712,6 +12753,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;
> @@ -12770,6 +12816,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;
>
> @@ -22777,10 +22828,9 @@ mips_asm_file_end (void)
>
> #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
> diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
> index 47aac9d3d61..bae9ffe9b3c 100644
> --- a/gcc/config/mips/mips.h
> +++ b/gcc/config/mips/mips.h
> @@ -3376,6 +3376,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
>
> diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
> index 455b9b802f6..dee71dc1fb0 100644
> --- a/gcc/config/mips/mips.md
> +++ b/gcc/config/mips/mips.md
> @@ -159,6 +159,7 @@
>
> ;; The `.insn' pseudo-op.
> UNSPEC_INSN_PSEUDO
> + UNSPEC_JRHB
> ])
>
> (define_constants
> @@ -6660,6 +6661,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"
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 49df8e6dc38..8d2a0a23af6 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -5540,6 +5540,12 @@ On MIPS targets, you can use the @code{nocompression} function attribute
> 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
> diff --git a/gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c b/gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
> new file mode 100644
> index 00000000000..3575af44dcd
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
> @@ -0,0 +1,20 @@
> +/* Test attribute for clearing hazards while returning. */
> +/* { dg-do compile } */
> +/* { dg-options "isa_rev>=2 -mno-mips16" } */
> +
> +extern int bar ();
> +
> +static int __attribute__ ((use_hazard_barrier_return))
> +foo0 ()
> +{
> + return bar ();
> +}
> +
> +int
> +foo1 ()
> +{
> + return foo0 ();
> +}
> +
> +/* { dg-final { scan-assembler "foo0:" } } */
> +/* { dg-final { scan-assembler-times "\tjr.hb\t\\\$31\n\tnop\\n" 1 } } */
> --
> 2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] [MIPS] Hazard barrier return support
2021-08-16 19:16 ` Andrew Pinski
@ 2021-08-16 20:40 ` Dragan Mladjenovic
2021-10-07 23:43 ` Maciej W. Rozycki
2021-08-17 15:58 ` Dragan Mladjenovic
2021-08-30 11:55 ` Dragan Mladjenovic
2 siblings, 1 reply; 6+ messages in thread
From: Dragan Mladjenovic @ 2021-08-16 20:40 UTC (permalink / raw)
To: Andrew Pinski; +Cc: gcc-patches
> -----Original Message-----
> From: Andrew Pinski [mailto:pinskia@gmail.com]
> Sent: 16 August 2021 21:17
> To: Dragan Mladjenovic <OT_Dragan.Mladjenovic@mediatek.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] [MIPS] Hazard barrier return support
>
> On Mon, Aug 16, 2021 at 7:43 AM Dragan Mladjenovic via Gcc-patches <gcc-
> patches@gcc.gnu.org> wrote:
> >
> > This patch allows a function to request clearing of all instruction
> > and execution hazards upon normal return via __attribute__
> ((use_hazard_barrier_return)).
> >
> > 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.
> > ---
> > Rehash of original patch posted by Prachi with minimal changes. Tested
> > against mips-mti-elf with mips32r2/-EB and mips32r2/-EB/-micromips.
> >
> > gcc/config/mips/mips.c | 58 +++++++++++++++++--
> > gcc/config/mips/mips.h | 3 +
> > gcc/config/mips/mips.md | 15 +++++
> > gcc/doc/extend.texi | 6 ++
> > .../mips/hazard-barrier-return-attribute.c | 20 +++++++
> > 5 files changed, 98 insertions(+), 4 deletions(-) create mode 100644
> > gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
> >
> > diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index
> > 89d1be6cea6..6ce12fce52e 100644
> > --- a/gcc/config/mips/mips.c
> > +++ b/gcc/config/mips/mips.c
> > @@ -630,6 +630,7 @@ static const struct attribute_spec
> mips_attribute_table[] = {
> > mips_handle_use_shadow_register_set_attr, NULL },
> > { "keep_interrupts_masked", 0, 0, false, true, true, false, NULL, NULL },
> > { "use_debug_exception_return", 0, 0, false, true, true, false,
> > NULL, NULL },
> > + { "use_hazard_barrier_return", 0, 0, true, false, false, false,
> > + NULL, NULL },
> > { NULL, 0, 0, false, false, false, false, NULL, NULL }
> > };
> >
> > @@ -1309,6 +1310,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. */
> >
> > @@ -1494,6 +1505,19 @@ mips_can_inline_p (tree caller, tree callee)
> > return default_target_can_inline_p (caller, callee); }
> >
> > +/* Implement TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P.
> > +
> > + A function requesting 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) {
> > + return !mips_use_hazard_barrier_return_p (decl); }
> > +
> > /* Handle an "interrupt" attribute with an optional argument. */
> >
> > static tree
> > @@ -7921,6 +7945,11 @@ mips_function_ok_for_sibcall (tree decl, tree
> exp ATTRIBUTE_UNUSED)
> > && !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;
> > }
> > @@ -11008,6 +11037,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");
>
> Just a small nit, is MIPS64r2 ok too? Also did you did you test it for MIPS64
> too? I still partly care about MIPS64.
I'll respin the tests for mips64r2 and mips64 just in case.
This check would fail for -mips64. GAS accepts jr.hb for both '.set mips64' and
'.set mips64r2' and '.set mips32' for that matter. mips-opc.c has the following
comment:
/* jr.hb is officially MIPS{32,64}R2, but it works on R1 as jr with
the same hazard barrier effect. */
Regards,
Dragan
>
> Thanks,
> Andrew
>
> > + 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 ();
> > @@ -12671,7 +12711,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. */ @@ -12712,6
> > +12753,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;
> > @@ -12770,6 +12816,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;
> >
> > @@ -22777,10 +22828,9 @@ mips_asm_file_end (void)
> >
> > #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 diff
> > --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index
> > 47aac9d3d61..bae9ffe9b3c 100644
> > --- a/gcc/config/mips/mips.h
> > +++ b/gcc/config/mips/mips.h
> > @@ -3376,6 +3376,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
> >
> > diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md index
> > 455b9b802f6..dee71dc1fb0 100644
> > --- a/gcc/config/mips/mips.md
> > +++ b/gcc/config/mips/mips.md
> > @@ -159,6 +159,7 @@
> >
> > ;; The `.insn' pseudo-op.
> > UNSPEC_INSN_PSEUDO
> > + UNSPEC_JRHB
> > ])
> >
> > (define_constants
> > @@ -6660,6 +6661,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"
> > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index
> > 49df8e6dc38..8d2a0a23af6 100644
> > --- a/gcc/doc/extend.texi
> > +++ b/gcc/doc/extend.texi
> > @@ -5540,6 +5540,12 @@ On MIPS targets, you can use the
> > @code{nocompression} function attribute 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
> > diff --git
> > a/gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
> > b/gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
> > new file mode 100644
> > index 00000000000..3575af44dcd
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
> > @@ -0,0 +1,20 @@
> > +/* Test attribute for clearing hazards while returning. */
> > +/* { dg-do compile } */
> > +/* { dg-options "isa_rev>=2 -mno-mips16" } */
> > +
> > +extern int bar ();
> > +
> > +static int __attribute__ ((use_hazard_barrier_return))
> > +foo0 ()
> > +{
> > + return bar ();
> > +}
> > +
> > +int
> > +foo1 ()
> > +{
> > + return foo0 ();
> > +}
> > +
> > +/* { dg-final { scan-assembler "foo0:" } } */
> > +/* { dg-final { scan-assembler-times "\tjr.hb\t\\\$31\n\tnop\\n" 1 }
> > +} */
> > --
> > 2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] [MIPS] Hazard barrier return support
2021-08-16 19:16 ` Andrew Pinski
2021-08-16 20:40 ` Dragan Mladjenovic
@ 2021-08-17 15:58 ` Dragan Mladjenovic
2021-08-30 11:55 ` Dragan Mladjenovic
2 siblings, 0 replies; 6+ messages in thread
From: Dragan Mladjenovic @ 2021-08-17 15:58 UTC (permalink / raw)
To: Andrew Pinski; +Cc: gcc-patches
> -----Original Message-----
> From: Dragan Mladjenovic
> Sent: 16 August 2021 22:40
> To: 'Andrew Pinski' <pinskia@gmail.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH] [MIPS] Hazard barrier return support
>
>
>
> > -----Original Message-----
> > From: Andrew Pinski [mailto:pinskia@gmail.com]
> > Sent: 16 August 2021 21:17
> > To: Dragan Mladjenovic <OT_Dragan.Mladjenovic@mediatek.com>
> > Cc: gcc-patches@gcc.gnu.org
> > Subject: Re: [PATCH] [MIPS] Hazard barrier return support
> >
> > On Mon, Aug 16, 2021 at 7:43 AM Dragan Mladjenovic via Gcc-patches
> > <gcc- patches@gcc.gnu.org> wrote:
> > >
> > > This patch allows a function to request clearing of all instruction
> > > and execution hazards upon normal return via __attribute__
> > ((use_hazard_barrier_return)).
> > >
> > > 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.
> > > ---
> > > Rehash of original patch posted by Prachi with minimal changes.
> > > Tested against mips-mti-elf with mips32r2/-EB and mips32r2/-EB/-
> micromips.
> > >
> > > gcc/config/mips/mips.c | 58 +++++++++++++++++--
> > > gcc/config/mips/mips.h | 3 +
> > > gcc/config/mips/mips.md | 15 +++++
> > > gcc/doc/extend.texi | 6 ++
> > > .../mips/hazard-barrier-return-attribute.c | 20 +++++++
> > > 5 files changed, 98 insertions(+), 4 deletions(-) create mode
> > > 100644
> > > gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
> > >
> > > diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index
> > > 89d1be6cea6..6ce12fce52e 100644
> > > --- a/gcc/config/mips/mips.c
> > > +++ b/gcc/config/mips/mips.c
> > > @@ -630,6 +630,7 @@ static const struct attribute_spec
> > mips_attribute_table[] = {
> > > mips_handle_use_shadow_register_set_attr, NULL },
> > > { "keep_interrupts_masked", 0, 0, false, true, true, false, NULL, NULL },
> > > { "use_debug_exception_return", 0, 0, false, true, true, false,
> > > NULL, NULL },
> > > + { "use_hazard_barrier_return", 0, 0, true, false, false, false,
> > > + NULL, NULL },
> > > { NULL, 0, 0, false, false, false, false, NULL, NULL }
> > > };
> > >
> > > @@ -1309,6 +1310,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. */
> > >
> > > @@ -1494,6 +1505,19 @@ mips_can_inline_p (tree caller, tree callee)
> > > return default_target_can_inline_p (caller, callee); }
> > >
> > > +/* Implement TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P.
> > > +
> > > + A function requesting 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) {
> > > + return !mips_use_hazard_barrier_return_p (decl); }
> > > +
> > > /* Handle an "interrupt" attribute with an optional argument. */
> > >
> > > static tree
> > > @@ -7921,6 +7945,11 @@ mips_function_ok_for_sibcall (tree decl, tree
> > exp ATTRIBUTE_UNUSED)
> > > && !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;
> > > }
> > > @@ -11008,6 +11037,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");
> >
> > Just a small nit, is MIPS64r2 ok too? Also did you did you test it
> > for MIPS64 too? I still partly care about MIPS64.
> I'll respin the tests for mips64r2 and mips64 just in case.
>
> This check would fail for -mips64. GAS accepts jr.hb for both '.set mips64' and
> '.set mips64r2' and '.set mips32' for that matter. mips-opc.c has the following
> comment:
> /* jr.hb is officially MIPS{32,64}R2, but it works on R1 as jr with
> the same hazard barrier effect. */
>
> Regards,
> Dragan
>
FYI the change introduces no new regression for mips-mti-elf mips64r2/mabi=64/EB.
Best regards,
Dragan
> >
> > Thanks,
> > Andrew
> >
> > > + 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 ();
> > > @@ -12671,7 +12711,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. */ @@ -12712,6
> > > +12753,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;
> > > @@ -12770,6 +12816,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;
> > >
> > > @@ -22777,10 +22828,9 @@ mips_asm_file_end (void)
> > >
> > > #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 diff
> > > --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index
> > > 47aac9d3d61..bae9ffe9b3c 100644
> > > --- a/gcc/config/mips/mips.h
> > > +++ b/gcc/config/mips/mips.h
> > > @@ -3376,6 +3376,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
> > >
> > > diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md index
> > > 455b9b802f6..dee71dc1fb0 100644
> > > --- a/gcc/config/mips/mips.md
> > > +++ b/gcc/config/mips/mips.md
> > > @@ -159,6 +159,7 @@
> > >
> > > ;; The `.insn' pseudo-op.
> > > UNSPEC_INSN_PSEUDO
> > > + UNSPEC_JRHB
> > > ])
> > >
> > > (define_constants
> > > @@ -6660,6 +6661,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"
> > > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index
> > > 49df8e6dc38..8d2a0a23af6 100644
> > > --- a/gcc/doc/extend.texi
> > > +++ b/gcc/doc/extend.texi
> > > @@ -5540,6 +5540,12 @@ On MIPS targets, you can use the
> > > @code{nocompression} function attribute 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
> > > diff --git
> > > a/gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
> > > b/gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
> > > new file mode 100644
> > > index 00000000000..3575af44dcd
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.
> > > +++ c
> > > @@ -0,0 +1,20 @@
> > > +/* Test attribute for clearing hazards while returning. */
> > > +/* { dg-do compile } */
> > > +/* { dg-options "isa_rev>=2 -mno-mips16" } */
> > > +
> > > +extern int bar ();
> > > +
> > > +static int __attribute__ ((use_hazard_barrier_return))
> > > +foo0 ()
> > > +{
> > > + return bar ();
> > > +}
> > > +
> > > +int
> > > +foo1 ()
> > > +{
> > > + return foo0 ();
> > > +}
> > > +
> > > +/* { dg-final { scan-assembler "foo0:" } } */
> > > +/* { dg-final { scan-assembler-times "\tjr.hb\t\\\$31\n\tnop\\n" 1
> > > +} } */
> > > --
> > > 2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] [MIPS] Hazard barrier return support
2021-08-16 19:16 ` Andrew Pinski
2021-08-16 20:40 ` Dragan Mladjenovic
2021-08-17 15:58 ` Dragan Mladjenovic
@ 2021-08-30 11:55 ` Dragan Mladjenovic
2 siblings, 0 replies; 6+ messages in thread
From: Dragan Mladjenovic @ 2021-08-30 11:55 UTC (permalink / raw)
To: Andrew Pinski, Jeff Law; +Cc: gcc-patches
> -----Original Message-----
> From: Dragan Mladjenovic
> Sent: 17 August 2021 17:59
> To: 'Andrew Pinski' <pinskia@gmail.com>
> Cc: 'gcc-patches@gcc.gnu.org' <gcc-patches@gcc.gnu.org>
> Subject: RE: [PATCH] [MIPS] Hazard barrier return support
>
>
>
> > -----Original Message-----
> > From: Dragan Mladjenovic
> > Sent: 16 August 2021 22:40
> > To: 'Andrew Pinski' <pinskia@gmail.com>
> > Cc: gcc-patches@gcc.gnu.org
> > Subject: RE: [PATCH] [MIPS] Hazard barrier return support
> >
> >
> >
> > > -----Original Message-----
> > > From: Andrew Pinski [mailto:pinskia@gmail.com]
> > > Sent: 16 August 2021 21:17
> > > To: Dragan Mladjenovic <OT_Dragan.Mladjenovic@mediatek.com>
> > > Cc: gcc-patches@gcc.gnu.org
> > > Subject: Re: [PATCH] [MIPS] Hazard barrier return support
> > >
> > > On Mon, Aug 16, 2021 at 7:43 AM Dragan Mladjenovic via Gcc-patches
> > > <gcc- patches@gcc.gnu.org> wrote:
> > > >
> > > > This patch allows a function to request clearing of all
> > > > instruction and execution hazards upon normal return via
> > > > __attribute__
> > > ((use_hazard_barrier_return)).
> > > >
> > > > 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.
> > > > ---
> > > > Rehash of original patch posted by Prachi with minimal changes.
> > > > Tested against mips-mti-elf with mips32r2/-EB and mips32r2/-EB/-
> > micromips.
> > > >
> > > > gcc/config/mips/mips.c | 58 +++++++++++++++++--
> > > > gcc/config/mips/mips.h | 3 +
> > > > gcc/config/mips/mips.md | 15 +++++
> > > > gcc/doc/extend.texi | 6 ++
> > > > .../mips/hazard-barrier-return-attribute.c | 20 +++++++
> > > > 5 files changed, 98 insertions(+), 4 deletions(-) create mode
> > > > 100644
> > > > gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
> > > >
> > > > diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index
> > > > 89d1be6cea6..6ce12fce52e 100644
> > > > --- a/gcc/config/mips/mips.c
> > > > +++ b/gcc/config/mips/mips.c
> > > > @@ -630,6 +630,7 @@ static const struct attribute_spec
> > > mips_attribute_table[] = {
> > > > mips_handle_use_shadow_register_set_attr, NULL },
> > > > { "keep_interrupts_masked", 0, 0, false, true, true, false, NULL,
> NULL },
> > > > { "use_debug_exception_return", 0, 0, false, true, true, false,
> > > > NULL, NULL },
> > > > + { "use_hazard_barrier_return", 0, 0, true, false, false, false,
> > > > + NULL, NULL },
> > > > { NULL, 0, 0, false, false, false, false, NULL, NULL }
> > > > };
> > > >
> > > > @@ -1309,6 +1310,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. */
> > > >
> > > > @@ -1494,6 +1505,19 @@ mips_can_inline_p (tree caller, tree callee)
> > > > return default_target_can_inline_p (caller, callee); }
> > > >
> > > > +/* Implement TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P.
> > > > +
> > > > + A function requesting 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) {
> > > > + return !mips_use_hazard_barrier_return_p (decl); }
> > > > +
> > > > /* Handle an "interrupt" attribute with an optional argument. */
> > > >
> > > > static tree
> > > > @@ -7921,6 +7945,11 @@ mips_function_ok_for_sibcall (tree decl,
> > > > tree
> > > exp ATTRIBUTE_UNUSED)
> > > > && !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;
> > > > }
> > > > @@ -11008,6 +11037,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");
> > >
> > > Just a small nit, is MIPS64r2 ok too? Also did you did you test it
> > > for MIPS64 too? I still partly care about MIPS64.
> > I'll respin the tests for mips64r2 and mips64 just in case.
> >
> > This check would fail for -mips64. GAS accepts jr.hb for both '.set
> > mips64' and '.set mips64r2' and '.set mips32' for that matter.
> > mips-opc.c has the following
> > comment:
> > /* jr.hb is officially MIPS{32,64}R2, but it works on R1 as jr with
> > the same hazard barrier effect. */
> >
> > Regards,
> > Dragan
> >
>
> FYI the change introduces no new regression for mips-mti-elf
> mips64r2/mabi=64/EB.
>
> Best regards,
>
> Dragan
>
> > >
> > > Thanks,
> > > Andrew
> > >
> > > > + 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 ();
> > > > @@ -12671,7 +12711,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. */ @@ -12712,6
> > > > +12753,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;
> > > > @@ -12770,6 +12816,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;
> > > >
> > > > @@ -22777,10 +22828,9 @@ mips_asm_file_end (void)
> > > >
> > > > #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 diff --git
> > > > a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index
> > > > 47aac9d3d61..bae9ffe9b3c 100644
> > > > --- a/gcc/config/mips/mips.h
> > > > +++ b/gcc/config/mips/mips.h
> > > > @@ -3376,6 +3376,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
> > > >
> > > > diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
> > > > index
> > > > 455b9b802f6..dee71dc1fb0 100644
> > > > --- a/gcc/config/mips/mips.md
> > > > +++ b/gcc/config/mips/mips.md
> > > > @@ -159,6 +159,7 @@
> > > >
> > > > ;; The `.insn' pseudo-op.
> > > > UNSPEC_INSN_PSEUDO
> > > > + UNSPEC_JRHB
> > > > ])
> > > >
> > > > (define_constants
> > > > @@ -6660,6 +6661,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"
> > > > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index
> > > > 49df8e6dc38..8d2a0a23af6 100644
> > > > --- a/gcc/doc/extend.texi
> > > > +++ b/gcc/doc/extend.texi
> > > > @@ -5540,6 +5540,12 @@ On MIPS targets, you can use the
> > > > @code{nocompression} function attribute 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
> > > > diff --git
> > > > a/gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
> > > > b/gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.c
> > > > new file mode 100644
> > > > index 00000000000..3575af44dcd
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/gcc.target/mips/hazard-barrier-return-attribute.
> > > > +++ c
> > > > @@ -0,0 +1,20 @@
> > > > +/* Test attribute for clearing hazards while returning. */
> > > > +/* { dg-do compile } */
> > > > +/* { dg-options "isa_rev>=2 -mno-mips16" } */
> > > > +
> > > > +extern int bar ();
> > > > +
> > > > +static int __attribute__ ((use_hazard_barrier_return))
> > > > +foo0 ()
> > > > +{
> > > > + return bar ();
> > > > +}
> > > > +
> > > > +int
> > > > +foo1 ()
> > > > +{
> > > > + return foo0 ();
> > > > +}
> > > > +
> > > > +/* { dg-final { scan-assembler "foo0:" } } */
> > > > +/* { dg-final { scan-assembler-times "\tjr.hb\t\\\$31\n\tnop\\n"
> > > > +1 } } */
> > > > --
> > > > 2.17.1
Ping.
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] [MIPS] Hazard barrier return support
2021-08-16 20:40 ` Dragan Mladjenovic
@ 2021-10-07 23:43 ` Maciej W. Rozycki
0 siblings, 0 replies; 6+ messages in thread
From: Maciej W. Rozycki @ 2021-10-07 23:43 UTC (permalink / raw)
To: Dragan Mladjenovic; +Cc: Andrew Pinski, gcc-patches
On Mon, 16 Aug 2021, Dragan Mladjenovic via Gcc-patches wrote:
> I'll respin the tests for mips64r2 and mips64 just in case.
>
> This check would fail for -mips64. GAS accepts jr.hb for both '.set mips64' and
> '.set mips64r2' and '.set mips32' for that matter. mips-opc.c has the following
> comment:
> /* jr.hb is officially MIPS{32,64}R2, but it works on R1 as jr with
> the same hazard barrier effect. */
That it works on R1 is however not universally true, because as I recall
at least the original 4Kc CPU does trap with RI on non-zero hint values.
It may have qualified as a processor erratum though. I just did a quick
run-time check with a 5Kc CPU and that one does execute the JR.HB encoding
just fine. I don't have a specimen of 4Kc that I could readily use, even
though I do believe I have a 4Kc module along with an Algorithmics board
that could use it and which could possibly run NetBSD (I haven't checked).
NB the semantics claim is even less true, because there is no hazard
barrier effect with the R1 JR or JALR instructions ever, and for example
the 4Kc does require to have any hazards resolved by placing a suitable
number of intervening instructions (which do not have to be SSNOP however
as the 4Kc is not superscalar). So a typical instruction hazard such as a
change of the ASID is requires 1 spacing instruction and 3 such for a data
access and an instruction fetch respectively affected by a change of the
ASID[1].
I don't know where the note in mips-opc.c originally came from and at
this point it cannot be verified. As it was typical at the time, the
change was posted with just a vague note referring a "mixed bag of opcode
table changes" as it was applied[2], so it does not help.
References:
[1] "MIPS32 4K Processor Core Family Software User's Manual", MIPS
Technologies, Inc., Document Number: MD00016, Revision 01.17,
September 25, 2002, Table 2-6 "Instruction Hazards", p. 28
[2] "MIPS opcode table update",
<https://sourceware.org/ml/binutils/2006-05/msg00056.html>
Maciej
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-07 23:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-16 14:42 [PATCH] [MIPS] Hazard barrier return support Dragan Mladjenovic
2021-08-16 19:16 ` Andrew Pinski
2021-08-16 20:40 ` Dragan Mladjenovic
2021-10-07 23:43 ` Maciej W. Rozycki
2021-08-17 15:58 ` Dragan Mladjenovic
2021-08-30 11:55 ` Dragan Mladjenovic
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).