> -----Original Message-----
> From: Andrew Pinski [mailto:pins...@gmail.com]
> Sent: 16 August 2021 21:17
> To: Dragan Mladjenovic <ot_dragan.mladjeno...@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-
> patc...@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.godb...@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
