diff mbox

[SPARC] Add a workaround for the LEON3FT store-store errata

Message ID 20170614082640.31509-1-cederman@gaisler.com
State New
Headers show

Commit Message

Daniel Cederman June 14, 2017, 8:26 a.m. UTC
Hello all,

I'm resending this patch with an update that fixes an issue when using
it together with the -mflat flag.

-

This patch adds a workaround to the Sparc backend for the LEON3FT
store-store errata. It is enabled using the -mfix-b2bst flag.

The workaround inserts NOP instructions to prevent the following two
instruction sequences from being generated:

std -> stb/sth/st/std
stb/sth/st -> any single non-store/load instruction -> stb/sth/st/std

The __FIX_B2BST define can be used to only enable workarounds in assembly
code when the flag is used.

See GRLIB-TN-0009, "LEON3FT Stale Cache Entry After Store with Data Tag
Parity Error", for more information.

gcc/ChangeLog:

2017-01-18  Daniel Cederman  <cederman@gaisler.com>

	* config/sparc/sparc.c (sparc_do_work_around_errata): Insert NOP
	instructions to prevent sequences that can trigger the store-store
	errata for certain LEON3FT processors. Enable with -mfix-b2bst.
	(sparc_option_override): -mfix-ut699 implies -mfix-b2bst.
	* config/sparc/sparc-c.c (sparc_target_macros): Define __FIX_B2BST.
	* config/sparc/sparc.md: Prevent stores in delay slot.
	* config/sparc/sparc.opt: Add -mfix-b2bst flag.
	* doc/invoke.texi: Document -mfix-b2bst flag.
---
 gcc/config/sparc/sparc-c.c |   3 ++
 gcc/config/sparc/sparc.c   | 107 ++++++++++++++++++++++++++++++++++++++++++++-
 gcc/config/sparc/sparc.md  |  10 ++++-
 gcc/config/sparc/sparc.opt |   4 ++
 gcc/doc/invoke.texi        |   7 ++-
 5 files changed, 126 insertions(+), 5 deletions(-)

Comments

Sebastian Huber June 20, 2017, 5:55 a.m. UTC | #1
Hello,

would someone mind reviewing this patch please. It was already sent for 
review on January this year and got no attention. Now we are in a 
different development stage.

https://gcc.gnu.org/ml/gcc-patches/2017-01/msg01354.html
David Miller June 20, 2017, 3:44 p.m. UTC | #2
From: Sebastian Huber <sebastian.huber@embedded-brains.de>
Date: Tue, 20 Jun 2017 07:55:33 +0200

> would someone mind reviewing this patch please. It was already sent
> for review on January this year and got no attention. Now we are in a
> different development stage.
> 
> https://gcc.gnu.org/ml/gcc-patches/2017-01/msg01354.html

I'm fine with this change.
Eric Botcazou June 20, 2017, 7:19 p.m. UTC | #3
> I'm fine with this change.

I disagree, the existing policy is to avoid switches like -mfix-b2bst and use 
-mfix-xxxx where xxxx is a CPU (here xxxx could be ut699e or ut700).
David Miller June 20, 2017, 7:54 p.m. UTC | #4
From: Eric Botcazou <ebotcazou@adacore.com>
Date: Tue, 20 Jun 2017 21:19:37 +0200

>> I'm fine with this change.
> 
> I disagree, the existing policy is to avoid switches like -mfix-b2bst and use 
> -mfix-xxxx where xxxx is a CPU (here xxxx could be ut699e or ut700).

Ok, I was not aware of that policy.  But this should be easy for the
submitter to fix.
Eric Botcazou June 21, 2017, 6:35 a.m. UTC | #5
> Ok, I was not aware of that policy.

The reason is that experience showed that you may have several issues for the 
same class of processors (e.g. for the original UT699) and you don't want to 
have to pass a list of -mfix-xxxx switches to fix them all.  Moreover, the 
workarounds may interact with each other (again e.g. the original UT699) so 
you don't want to have to test all the combinations of -mfix-xxxx switches.
diff mbox

Patch

diff --git a/gcc/config/sparc/sparc-c.c b/gcc/config/sparc/sparc-c.c
index 9603173..6979f9c 100644
--- a/gcc/config/sparc/sparc-c.c
+++ b/gcc/config/sparc/sparc-c.c
@@ -60,4 +60,7 @@  sparc_target_macros (void)
       cpp_define (parse_in, "__VIS__=0x100");
       cpp_define (parse_in, "__VIS=0x100");
     }
+
+  if (sparc_fix_b2bst)
+    builtin_define_std ("__FIX_B2BST");
 }
diff --git a/gcc/config/sparc/sparc.c b/gcc/config/sparc/sparc.c
index 95a64a4..d5225e0 100644
--- a/gcc/config/sparc/sparc.c
+++ b/gcc/config/sparc/sparc.c
@@ -896,6 +896,12 @@  mem_ref (rtx x)
    to properly detect the various hazards.  Therefore, this machine specific
    pass runs as late as possible.  */
 
+/* True if INSN is a md pattern or asm statement.  */
+#define USEFUL_INSN_P(INSN)						\
+  (NONDEBUG_INSN_P (INSN)						\
+   && GET_CODE (PATTERN (INSN)) != USE					\
+   && GET_CODE (PATTERN (INSN)) != CLOBBER)
+
 static unsigned int
 sparc_do_work_around_errata (void)
 {
@@ -915,6 +921,98 @@  sparc_do_work_around_errata (void)
 	if (rtx_sequence *seq = dyn_cast <rtx_sequence *> (PATTERN (insn)))
 	  insn = seq->insn (1);
 
+      /* Look for a double-word store.  */
+      if (sparc_fix_b2bst
+	  && NONJUMP_INSN_P (insn)
+	  && (set = single_set (insn)) != NULL_RTX
+	  && GET_MODE_SIZE (GET_MODE (SET_DEST (set))) == 8
+	  && MEM_P (SET_DEST (set)))
+	{
+	  next = next_active_insn (insn);
+	  if (!next)
+	    break;
+
+	  /* Skip empty assembly statements.  */
+	  if ((GET_CODE(PATTERN(next)) == UNSPEC_VOLATILE) ||
+	      (USEFUL_INSN_P (next)
+	      && (asm_noperands (PATTERN (next))>=0)
+	      && !strcmp (decode_asm_operands (PATTERN (next),
+					       NULL, NULL, NULL,
+					       NULL, NULL), "")))
+	    next = next_active_insn (next);
+	  if (!next)
+	    break;
+
+	  /* If the insn is a branch, then it cannot be problematic.  */
+	  if (!NONJUMP_INSN_P (next) || GET_CODE (PATTERN (next)) == SEQUENCE)
+	    continue;
+
+	  if ((set = single_set (next)) == NULL_RTX)
+	    continue;
+
+	  /* Add NOP if double-word store is followed by any type of store.  */
+	  if (MEM_P (SET_DEST (set)))
+	    insert_nop = true;
+	}
+      else
+      /* Look for single-word, half-word, or byte store.  */
+      if (sparc_fix_b2bst
+	  && NONJUMP_INSN_P (insn)
+	  && (set = single_set (insn)) != NULL_RTX
+	  && GET_MODE_SIZE (GET_MODE (SET_DEST (set))) <= 4
+	  && MEM_P (SET_DEST (set)))
+	{
+	  rtx_insn *after;
+
+	  next = next_active_insn (insn);
+	  if (!next)
+	    break;
+
+	  /* Skip empty assembly statements.  */
+	  if ((GET_CODE(PATTERN(next)) == UNSPEC_VOLATILE) ||
+	      (USEFUL_INSN_P (next)
+	      && (asm_noperands (PATTERN (next))>=0)
+	      && !strcmp (decode_asm_operands (PATTERN (next),
+					       NULL, NULL, NULL,
+					       NULL, NULL), "")))
+	    next = next_active_insn (next);
+	  if (!next)
+	    break;
+
+	  /* If the insn is a branch, then it cannot be problematic.  */
+	  if (!NONJUMP_INSN_P (next) || GET_CODE (PATTERN (next)) == SEQUENCE)
+	    continue;
+
+	  /* If the insn is a load or store, then it cannot be problematic.  */
+	  if ((set = single_set (next)) != NULL_RTX
+	      && (MEM_P (SET_DEST (set)) || MEM_P (SET_SRC (set))))
+	    continue;
+
+	  after = next_active_insn (next);
+	  if (!after)
+	    continue;
+
+	  /* Skip empty assembly statements.  */
+	  if ((GET_CODE(PATTERN(after)) == UNSPEC_VOLATILE) ||
+	      (USEFUL_INSN_P (after)
+	      && (asm_noperands (PATTERN (after))>=0)
+	      && !strcmp (decode_asm_operands (PATTERN (after),
+					       NULL, NULL, NULL,
+					       NULL, NULL), "")))
+	    after = next_active_insn (after);
+	  if (!after)
+	    break;
+
+	  /* If the insn is a branch, then it cannot be problematic.  */
+	  if (!NONJUMP_INSN_P (after) || GET_CODE (PATTERN (after)) == SEQUENCE)
+	    continue;
+
+	  /* Add NOP if third instruction is a store.  */
+	  if (((set = single_set (after)) != NULL_RTX)
+	      && MEM_P (SET_DEST (set)))
+	    insert_nop = true;
+	}
+      else
       /* Look for a single-word load into an odd-numbered FP register.  */
       if (sparc_fix_at697f
 	  && NONJUMP_INSN_P (insn)
@@ -1167,8 +1265,9 @@  public:
   /* opt_pass methods: */
   virtual bool gate (function *)
     {
-      /* The only errata we handle are those of the AT697F and UT699.  */
-      return sparc_fix_at697f != 0 || sparc_fix_ut699 != 0;
+      /* The only errata we handle are those of the AT697F,
+	 UT699, and certain LEON3FT.  */
+      return sparc_fix_at697f || sparc_fix_ut699 || sparc_fix_b2bst;
     }
 
   virtual unsigned int execute (function *)
@@ -1527,6 +1626,10 @@  sparc_option_override (void)
   if (!(target_flags_explicit & MASK_LRA))
     target_flags |= MASK_LRA;
 
+  /* -mfix-ut699 implies -mfix-b2bst.  */
+  if (sparc_fix_ut699)
+    sparc_fix_b2bst = 1;
+
   /* Supply a default value for align_functions.  */
   if (align_functions == 0)
     {
diff --git a/gcc/config/sparc/sparc.md b/gcc/config/sparc/sparc.md
index 5c5096b..6c1b1e3 100644
--- a/gcc/config/sparc/sparc.md
+++ b/gcc/config/sparc/sparc.md
@@ -329,6 +329,10 @@ 
    (symbol_ref "(sparc_fix_ut699 != 0
 		 ? FIX_UT699_TRUE : FIX_UT699_FALSE)"))
 
+(define_attr "fix_b2bst" "false,true"
+   (symbol_ref "(sparc_fix_b2bst != 0
+		 ? FIX_B2BST_TRUE : FIX_B2BST_FALSE)"))
+
 ;; Length (in # of insns).
 ;; Beware that setting a length greater or equal to 3 for conditional branches
 ;; has a side-effect (see output_cbranch and output_v9branch).
@@ -476,6 +480,8 @@ 
 (define_attr "in_branch_delay" "false,true"
   (cond [(eq_attr "type" "uncond_branch,branch,cbcond,uncond_cbcond,call,sibcall,call_no_delay_slot,multi")
 	   (const_string "false")
+	 (and (eq_attr "fix_b2bst" "true") (eq_attr "type" "store,fpstore"))
+	   (const_string "false")
 	 (and (eq_attr "fix_ut699" "true") (eq_attr "type" "load,sload"))
 	   (const_string "false")
 	 (and (eq_attr "fix_ut699" "true")
@@ -6061,7 +6067,7 @@ 
 	(div:DF (match_operand:DF 1 "register_operand" "e")
 		(match_operand:DF 2 "register_operand" "e")))]
   "TARGET_FPU && sparc_fix_ut699"
-  "fdivd\t%1, %2, %0\n\tstd\t%0, [%%sp-8]"
+  "fdivd\t%1, %2, %0\n\tnop\n\tstd\t%0, [%%sp-8]\n\tnop"
   [(set_attr "type" "fpdivd")
    (set_attr "fptype" "double")
    (set_attr "length" "2")])
@@ -6313,7 +6319,7 @@ 
   [(set (match_operand:DF 0 "register_operand" "=e")
 	(sqrt:DF (match_operand:DF 1 "register_operand" "e")))]
   "TARGET_FPU && sparc_fix_ut699"
-  "fsqrtd\t%1, %0\n\tstd\t%0, [%%sp-8]"
+  "fsqrtd\t%1, %0\n\tnop\n\tstd\t%0, [%%sp-8]\n\tnop"
   [(set_attr "type" "fpsqrtd")
    (set_attr "fptype" "double")
    (set_attr "length" "2")])
diff --git a/gcc/config/sparc/sparc.opt b/gcc/config/sparc/sparc.opt
index 86f85d9..02b52be 100644
--- a/gcc/config/sparc/sparc.opt
+++ b/gcc/config/sparc/sparc.opt
@@ -230,6 +230,10 @@  mfix-ut699
 Target Report RejectNegative Var(sparc_fix_ut699)
 Enable workarounds for the errata of the UT699 processor.
 
+mfix-b2bst
+Target Report RejectNegative Var(sparc_fix_b2bst)
+Enable workarounds for LEON3FT store-store errata
+
 Mask(LONG_DOUBLE_128)
 ;; Use 128-bit long double
 
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 653bc07..d4682dc 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -1126,7 +1126,7 @@  See RS/6000 and PowerPC Options.
 -mvis2  -mno-vis2  -mvis3  -mno-vis3 @gol
 -mcbcond  -mno-cbcond  -mfmaf  -mno-fmaf  @gol
 -mpopc  -mno-popc  -msubxc  -mno-subxc@gol
--mfix-at697f  -mfix-ut699 @gol
+-mfix-at697f  -mfix-ut699 -mfix-b2bst @gol
 -mlra  -mno-lra}
 
 @emph{SPU Options}
@@ -23696,6 +23696,11 @@  processor (which corresponds to erratum #13 of the AT697E processor).
 @opindex mfix-ut699
 Enable the documented workarounds for the floating-point errata and the data
 cache nullify errata of the UT699 processor.
+
+@item -mfix-b2bst
+@opindex mfix-b2bst
+Enable the documented workaround for the back-to-back store errata of
+certain LEON3FT processors.
 @end table
 
 These @samp{-m} options are supported in addition to the above