> -----Original Message-----
> From: Dragan Mladjenovic
> Sent: 16 August 2021 22:40
> To: 'Andrew Pinski' <pins...@gmail.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH] [MIPS] Hazard barrier return support
>
>
>
> > -----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
>
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
