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

[Daniel Cederman]

On 2017-06-26 09:21, Eric Botcazou wrote:


Yes, modulo the config/sparc/sparc-c.c hunk, what is it used for?



It defines __FIX_B2BST when the back-to-back store fix is enabled. This 
makes it easy to only add the workaround NOPs to inline assembly (and 
assembly files that uses the GCC preprocessor) when needed.



But the implementation looks a bit strange, can't we merge the essentially
identical blocks of code into a single block, as for the other fixes?



I will submit a new version of the patch in which I have tried to remove 
the code duplication.


--
Daniel Cederman
Cobham Gaisler

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

[Eric Botcazou]

> Eric, does Daniel's patch meet your requirements now?

Yes, modulo the config/sparc/sparc-c.c hunk, what is it used for?

But the implementation looks a bit strange, can't we merge the essentially 
identical blocks of code into a single block, as for the other fixes?

-- 
Eric Botcazo

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

[David Miller]

From: Daniel Cederman 
Date: Wed, 21 Jun 2017 15:27:51 +0200

> I have modified the patch so that the workaround is enabled by using
> either mfix-ut699, -mfix-ut700, or -mfix-gr712rc.

Eric, does Daniel's patch meet your requirements now?

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

[Daniel Cederman]

Hello all,

I have modified the patch so that the workaround is enabled by using
either mfix-ut699, -mfix-ut700, or -mfix-gr712rc.

Daniel

-

This patch adds a workaround to the Sparc backend for the LEON3FT
store-store errata. It is enabled when using the -mfix-ut699,
-mfix-ut700, or -mfix-gr712rc 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-06-21  Daniel Cederman  

* 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.
(sparc_option_override): -mfix-ut699, -mfix-ut700, and
-mfix-gr712rc enables the errata workaround.
* config/sparc/sparc-c.c (sparc_target_macros): Define __FIX_B2BST
when errata workaround is enabled.
* config/sparc/sparc.md: Prevent stores in delay slot.
* config/sparc/sparc.opt: Add -mfix-ut700 and -mfix-gr712rc flag.
* doc/invoke.texi: Document -mfix-ut700 and -mfix-gr712rc flag.
---
 gcc/config/sparc/sparc-c.c |   3 ++
 gcc/config/sparc/sparc.c   | 115 -
 gcc/config/sparc/sparc.md  |  10 +++-
 gcc/config/sparc/sparc.opt |  12 +
 gcc/doc/invoke.texi|  14 +-
 5 files changed, 148 insertions(+), 6 deletions(-)

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 790a036..6d6c941 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  (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), 

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

[Eric Botcazou]

> 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- 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- switches.

-- 
Eric Botcazou

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

[David Miller]

From: Eric Botcazou 
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- where  is a CPU (here  could be ut699e or ut700).

Ok, I was not aware of that policy.  But this should be easy for the
submitter to fix.

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

[Eric Botcazou]

> I'm fine with this change.

I disagree, the existing policy is to avoid switches like -mfix-b2bst and use 
-mfix- where  is a CPU (here  could be ut699e or ut700).

-- 
Eric Botcazou

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

[David Miller]

From: Sebastian Huber 
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.

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

[Sebastian Huber]

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

--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

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

[Daniel Cederman]

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  

* 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(-)

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  (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.  */
+