[PATCH] sparc: Char arrays are 64-bit aligned on SPARC

[PATCH] sparc: Char arrays are 64-bit aligned on SPARC

[Daniel Cederman]

pr88077 fails on SPARC since char HeaderStr[1] in pr88077_1.c and
long HeaderStr in pr88077_0.c differs in alignment.

warning: alignment 4 of normal symbol `HeaderStr' in c_lto_pr88077_0.o is
smaller than 8 used by the common definition in c_lto_pr88077_1.o

gcc/testsuite/ChangeLog:

	* gcc.dg/lto/pr88077_0.c: Change type to match alignment for SPARC
---
 gcc/testsuite/gcc.dg/lto/pr88077_0.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gcc/testsuite/gcc.dg/lto/pr88077_0.c b/gcc/testsuite/gcc.dg/lto/pr88077_0.c
index 924fe9fc3f01..9455295051fc 100644
--- a/gcc/testsuite/gcc.dg/lto/pr88077_0.c
+++ b/gcc/testsuite/gcc.dg/lto/pr88077_0.c
@@ -1,3 +1,7 @@
 /* { dg-lto-do link } */
 
+#if defined __sparc__
+long long HeaderStr;
+#else
 long HeaderStr;
+#endif
-- 
2.40.1

[PATCH 1/2] sparc: Revert membar optimization that is not suitable for LEON5

[Daniel Cederman]

From: Andreas Larsson <andreas@gaisler.com>

LEON5 has a deeper write-buffer and hence stb is not enough to flush a
write out. For compatibility, use the default V8 approach for both
LEON3 and LEON5.

This reverts commit 49cc765db35a5a21cab2aece27a44983fa70b94b,
"sync.md (*membar_storeload_leon3): New insn."

gcc/ChangeLog:

	* config/sparc/sync.md (*membar_storeload_leon3): Remove
	(*membar_storeload): Enable for LEON
---
 gcc/config/sparc/sync.md | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/gcc/config/sparc/sync.md b/gcc/config/sparc/sync.md
index 8808ed5bc14e..ac291420b8b9 100644
--- a/gcc/config/sparc/sync.md
+++ b/gcc/config/sparc/sync.md
@@ -64,19 +64,11 @@
   "stbar"
   [(set_attr "type" "multi")])
 
-;; For LEON3, STB has the effect of membar #StoreLoad.
-(define_insn "*membar_storeload_leon3"
-  [(set (match_operand:BLK 0 "" "")
-	(unspec:BLK [(match_dup 0) (const_int 2)] UNSPEC_MEMBAR))]
-  "TARGET_LEON3"
-  "stb\t%%g0, [%%sp-1]"
-  [(set_attr "type" "store")])
-
 ;; For V8, LDSTUB has the effect of membar #StoreLoad.
 (define_insn "*membar_storeload"
   [(set (match_operand:BLK 0 "" "")
 	(unspec:BLK [(match_dup 0) (const_int 2)] UNSPEC_MEMBAR))]
-  "TARGET_V8 && !TARGET_LEON3"
+  "TARGET_V8"
   "ldstub\t[%%sp-1], %%g0"
   [(set_attr "type" "multi")])
 
-- 
2.40.1

Re: [PATCH 1/2] sparc: Revert membar optimization that is not suitable for LEON5

[Eric Botcazou]

> LEON5 has a deeper write-buffer and hence stb is not enough to flush a
> write out. For compatibility, use the default V8 approach for both
> LEON3 and LEON5.
> 
> This reverts commit 49cc765db35a5a21cab2aece27a44983fa70b94b,
> "sync.md (*membar_storeload_leon3): New insn."
> 
> gcc/ChangeLog:
> 
> 	* config/sparc/sync.md (*membar_storeload_leon3): Remove
> 	(*membar_storeload): Enable for LEON

OK.

-- 
Eric Botcazou

[PATCH] sparc: Treat instructions with length 0 as empty

[Daniel Cederman]

This is to handle the membar_empty instruction that can be generated
when compiling for UT699.

gcc/ChangeLog:

	* config/sparc/sparc.cc (next_active_non_empty_insn): Length 0 treated as empty
---
 gcc/config/sparc/sparc.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/config/sparc/sparc.cc b/gcc/config/sparc/sparc.cc
index 62c57cc53159..8970674004b3 100644
--- a/gcc/config/sparc/sparc.cc
+++ b/gcc/config/sparc/sparc.cc
@@ -1119,6 +1119,7 @@ next_active_non_empty_insn (rtx_insn *insn)
   while (insn
 	 && (GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
 	     || GET_CODE (PATTERN (insn)) == ASM_INPUT
+	     || (get_attr_length (insn) == 0)
 	     || (USEFUL_INSN_P (insn)
 		 && (asm_noperands (PATTERN (insn)) >= 0)
 		 && !strcmp (decode_asm_operands (PATTERN (insn),
-- 
2.40.1

Re: [PATCH] sparc: Treat instructions with length 0 as empty

[Eric Botcazou]

> This is to handle the membar_empty instruction that can be generated
> when compiling for UT699.
> 
> gcc/ChangeLog:
> 
> 	* config/sparc/sparc.cc (next_active_non_empty_insn): Length 0 treated as empty

OK without the superfluous parentheses.


-- 
Eric Botcazou

[PATCH 2/2] sparc: Add errata workaround to membar patterns

[Daniel Cederman]

LEON now uses the standard V8 membar patterns that contains an ldstub
instruction. This instruction needs to be aligned properly when the
GR712RC errata workaround is enabled.

gcc/ChangeLog:

	* config/sparc/sparc.cc (atomic_insn_for_leon3_p): Treat membar_storeload as atomic
	* config/sparc/sync.md: Add GR712RC errata workaround
---
 gcc/config/sparc/sparc.cc |  1 +
 gcc/config/sparc/sync.md  | 23 ++++++++++++++++++-----
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/gcc/config/sparc/sparc.cc b/gcc/config/sparc/sparc.cc
index ebf1a557a49d..62c57cc53159 100644
--- a/gcc/config/sparc/sparc.cc
+++ b/gcc/config/sparc/sparc.cc
@@ -1052,6 +1052,7 @@ atomic_insn_for_leon3_p (rtx_insn *insn)
 {
   switch (INSN_CODE (insn))
     {
+    case CODE_FOR_membar_storeload:
     case CODE_FOR_swapsi:
     case CODE_FOR_ldstub:
     case CODE_FOR_atomic_compare_and_swap_leon3_1:
diff --git a/gcc/config/sparc/sync.md b/gcc/config/sparc/sync.md
index ac291420b8b9..fa249908a55f 100644
--- a/gcc/config/sparc/sync.md
+++ b/gcc/config/sparc/sync.md
@@ -65,12 +65,19 @@
   [(set_attr "type" "multi")])
 
 ;; For V8, LDSTUB has the effect of membar #StoreLoad.
-(define_insn "*membar_storeload"
+(define_insn "membar_storeload"
   [(set (match_operand:BLK 0 "" "")
 	(unspec:BLK [(match_dup 0) (const_int 2)] UNSPEC_MEMBAR))]
   "TARGET_V8"
-  "ldstub\t[%%sp-1], %%g0"
-  [(set_attr "type" "multi")])
+{
+  if (sparc_fix_gr712rc)
+    return ".align\t16\n\tldstub\t[%%sp-1], %%g0";
+  else
+    return "ldstub\t[%%sp-1], %%g0";
+}
+  [(set_attr "type" "multi")
+   (set (attr "length") (if_then_else (eq_attr "fix_gr712rc" "true")
+		      (const_int 4) (const_int 1)))])
 
 ;; Put the two together, in combination with the fact that V8 implements PSO
 ;; as its weakest memory model, means a full barrier.  Match all remaining
@@ -80,9 +87,15 @@
 	(unspec:BLK [(match_dup 0) (match_operand:SI 1 "const_int_operand")]
 		    UNSPEC_MEMBAR))]
   "TARGET_V8"
-  "stbar\n\tldstub\t[%%sp-1], %%g0"
+{
+  if (sparc_fix_gr712rc)
+    return "stbar\n.align\t16\n\tldstub\t[%%sp-1], %%g0";
+  else
+    return "stbar\n\tldstub\t[%%sp-1], %%g0";
+}
   [(set_attr "type" "multi")
-   (set_attr "length" "2")])
+   (set (attr "length") (if_then_else (eq_attr "fix_gr712rc" "true")
+		      (const_int 5) (const_int 2)))])
 
 ;; For V9, we have the full membar instruction.
 (define_insn "*membar"
-- 
2.40.1

Re: [PATCH 2/2] sparc: Add errata workaround to membar patterns

[Eric Botcazou]

> LEON now uses the standard V8 membar patterns that contains an ldstub
> instruction. This instruction needs to be aligned properly when the
> GR712RC errata workaround is enabled.
> 
> gcc/ChangeLog:
> 
> 	* config/sparc/sparc.cc (atomic_insn_for_leon3_p): Treat 
membar_storeload as atomic
> 	* config/sparc/sync.md: Add GR712RC errata workaround

The second ChangeLog entry should be more detailed:

	* config/sparc/sync.md (membar_storeload): Turn into named insn
	and add GR712RC errata workaround.
	(membar_v8): Add GR712RC errata workaround.

OK with this change.

-- 
Eric Botcazou

Re: [PATCH] sparc: Char arrays are 64-bit aligned on SPARC

[Eric Botcazou]

> pr88077 fails on SPARC since char HeaderStr[1] in pr88077_1.c and
> long HeaderStr in pr88077_0.c differs in alignment.
> 
> warning: alignment 4 of normal symbol `HeaderStr' in c_lto_pr88077_0.o is
> smaller than 8 used by the common definition in c_lto_pr88077_1.o

I have never seen it though.  Is that really a warning issued by GCC?

-- 
Eric Botcazou

Re: [PATCH] sparc: Char arrays are 64-bit aligned on SPARC

[Daniel Cederman]

On 2024-01-08 10:20, Eric Botcazou wrote:
>> pr88077 fails on SPARC since char HeaderStr[1] in pr88077_1.c and
>> long HeaderStr in pr88077_0.c differs in alignment.
>>
>> warning: alignment 4 of normal symbol `HeaderStr' in c_lto_pr88077_0.o is
>> smaller than 8 used by the common definition in c_lto_pr88077_1.o
> 
> I have never seen it though.  Is that really a warning issued by GCC?
> 

Hello Eric! Thank you for reviewing the patches!

No, this warning is not from GCC, it is from binutils ld. I forgot to 
mention that in the message. I get a similar warning from older versions 
of ld, so I do not think it is a new warning. It is also there with GCC 10.

For the OK:ed patches (with your changes), can I push them to 
release/gcc-13 in addition to master?

/Daniel

Re: [PATCH] sparc: Char arrays are 64-bit aligned on SPARC

[Eric Botcazou]

> Hello Eric! Thank you for reviewing the patches!

You're welcome.

> No, this warning is not from GCC, it is from binutils ld. I forgot to
> mention that in the message. I get a similar warning from older versions
> of ld, so I do not think it is a new warning. It is also there with GCC 10.

I see, thanks for the explanation, the patch is OK then.

> For the OK:ed patches (with your changes), can I push them to
> release/gcc-13 in addition to master?

Sure.

-- 
Eric Botcazou