This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 4/4] [SPARC] Errata workaround for GRLIB-TN-0013
- From: Eric Botcazou <ebotcazou at adacore dot com>
- To: Daniel Cederman <cederman at gaisler dot com>
- Cc: gcc-patches at gcc dot gnu dot org, sebastian dot huber at embedded-brains dot de, daniel at gaisler dot com
- Date: Fri, 24 Nov 2017 12:22:34 +0100
- Subject: Re: [PATCH 4/4] [SPARC] Errata workaround for GRLIB-TN-0013
- Authentication-results: sourceware.org; auth=none
- References: <20171120125003.22670-1-cederman@gaisler.com> <20171120125003.22670-5-cederman@gaisler.com>
> 2017-11-17 Daniel Cederman <cederman@gaisler.com>
>
> * config/sparc/sparc.c (fpop_reg_depend_p): New function.
> (div_sqrt_insn_p): New function.
> (sparc_do_work_around_errata): Insert NOP instructions to
> prevent sequences that could trigger the TN-0013 errata for
> certain LEON3 processors.
> (pass_work_around_errata::gate): Also test sparc_fix_tn0013.
> (sparc_option_override): Set sparc_fix_tn0013 appropriately.
> * config/sparc/sparc.md (fix_tn0013): New attribute.
> (in_branch_delay): Prevent div and sqrt in delay slot if fix_tn0013.
> * config/sparc/sparc.opt (sparc_fix_tn0013: New variable.
OK for mainline and 7 branch modulo the following nits:
> +/* True if any of the floating-point instruction's source
> + registers is the same as the provided register. */
> +
> +static int
> fpop_reg_depend_p (rtx_insn *insn, unsigned int reg)
'bool' instead of 'int'. "True if any of INSN's source register(s) is REG".
The function's name is awkward. insn_uses_reg_p? insn_reads_reg_p?
> +/* True if the instruction is floating-point division or
> + floating-point square-root. */
> +
> +static int
> +div_sqrt_insn_p (rtx_insn *insn)
'bool' instead of 'int'. "True if INSN is a floating-point ..."
> + if ( GET_CODE (PATTERN (insn)) != SET)
> + return false;
No space before GET_CODE.
> @@ -1064,6 +1097,80 @@ sparc_do_work_around_errata (void)
> insert_nop = true;
> }
>
> + /* Look for sequences that could trigger the TN-0013 errata. */
> + if (sparc_fix_tn0013
> + && NONJUMP_INSN_P (insn)
> + && div_sqrt_insn_p (insn))
> + {
> + int i;
> + int fp_found = 0;
> + unsigned int dest_reg;
> + rtx_insn *after;
> +
> + dest_reg = REGNO (SET_DEST (single_set (insn)));
const unsigned int dest_reg = REGNO (SET_DEST (single_set (insn)));
> + next = next_active_insn (insn);
> + if (!next)
> + break;
> +
> + for (after = next, i = 0; i < 4; i++)
> + {
> + /* Count floating-point operations. */
> + if (i != 3
> + && fpop_insn_p (after))
On a single line.
> + /* If the insn uses the destination register of
> + the div/sqrt, then it cannot be problematic. */
> + if (fpop_reg_depend_p (after, dest_reg))
> + break;
> + fp_found++;
> + }
> +
> + /* Count floating-point loads. */
> + if (i != 3
> + && (set = single_set (after)) != NULL_RTX
> + && REG_P (SET_DEST (set))
> + && REGNO (SET_DEST (set)) > 31)
> + {
> + /* If the insn uses the destination register of
> + the div/sqrt, then it cannot be problematic. */
> + if (REGNO (SET_DEST (set)) == dest_reg)
> + break;
> + fp_found++;
> + }
> +
> + /* Check if this is a problematic sequence. */
> + if (i > 1
> + && fp_found >= 2
> + && div_sqrt_insn_p (after))
> + {
> + /* Add extra NOP to prevent second version of
> + problematic sequence. */
> + if (i == 2)
> + emit_insn_before (gen_nop (), next);
> + insert_nop = true;
> + break;
> + }
So you emit 2 NOPs in a row if i == 2?
> diff --git a/gcc/config/sparc/sparc.opt b/gcc/config/sparc/sparc.opt
> index 22267f5..43cd964 100644
> --- a/gcc/config/sparc/sparc.opt
> +++ b/gcc/config/sparc/sparc.opt
> @@ -253,6 +253,10 @@ Enable workarounds for the errata of the GR712RC
> processor. TargetVariable
> unsigned int sparc_fix_b2bst
>
> +;; Enable workaround for TN-0013 errata
> +TargetVariable
> +unsigned int sparc_fix_tn0013
Can we find a more evocative name for the variable?
--
Eric Botcazou