Re: [PATCH] LoongArch: Fix atomic_exchange make comparison and may jump out

2022-11-16 Thread Xi Ruoyao via Gcc-patches
On Thu, 2022-11-17 at 11:46 +0800, Jinyang He wrote:
> > So we do need an additional dbar for compare-and-exchange, but do
> > not
> > need it for a bare atomic exchange?
> Yes.

Ok, I just noticed we also don't use dbar in atomic_add etc.

I've adjusted the patch a little (in attachment): rewritten the
ChangeLog and test, and renamed "value_exchange_and" to just
"value_exchange" because we are not doing anything beyond the exchange.

If it looks OK you can just send it as v2.

-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University
From 8b64115dfb289777300c4e84e278ab3d8ec4fe86 Mon Sep 17 00:00:00 2001
From: Jinyang He 
Date: Tue, 15 Nov 2022 21:03:28 +0800
Subject: [PATCH v2] LoongArch: Fix atomic_exchange expanding [PR107713]

We used to expand atomic_exchange_n(ptr, new, mem_order) for subword types
into something like:

{
  __typeof__(*ptr) t = atomic_load_n(ptr, mem_order);
  atomic_compare_exchange_n(ptr, , new, true, mem_order, mem_order);
  return t;
}

It's incorrect because another thread may store a different value into *ptr
after atomic_load_n.  Then atomic_compare_exchange_n will not store into
*ptr, but atomic_exchange_n should always perform the store.

gcc/ChangeLog:

	PR target/107713
	* config/loongarch/sync.md
	(atomic_cas_value_exchange_7_): New define_insn.
	(atomic_exchange): Use atomic_cas_value_exchange_7_si instead of
	atomic_cas_value_cmp_and_7_si.

gcc/testsuite/ChangeLog:

	PR target/107713
	* gcc.target/loongarch/pr107713-1.c: New test.
	* gcc.target/loongarch/pr107713-2.c: New test.
---
 gcc/config/loongarch/sync.md  | 27 +-
 .../gcc.target/loongarch/pr107713-1.c | 50 +++
 .../gcc.target/loongarch/pr107713-2.c |  9 
 3 files changed, 84 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/loongarch/pr107713-1.c
 create mode 100644 gcc/testsuite/gcc.target/loongarch/pr107713-2.c

diff --git a/gcc/config/loongarch/sync.md b/gcc/config/loongarch/sync.md
index 0c4f1983e88..45be1442439 100644
--- a/gcc/config/loongarch/sync.md
+++ b/gcc/config/loongarch/sync.md
@@ -448,6 +448,29 @@
 }
   [(set (attr "length") (const_int 32))])
 
+(define_insn "atomic_cas_value_exchange_7_"
+  [(set (match_operand:GPR 0 "register_operand" "=")
+	(match_operand:GPR 1 "memory_operand" "+ZC"))
+   (set (match_dup 1)
+	(unspec_volatile:GPR [(match_operand:GPR 2 "reg_or_0_operand" "rJ")
+			  (match_operand:GPR 3 "reg_or_0_operand" "rJ")
+			  (match_operand:GPR 4 "reg_or_0_operand" "rJ")
+			  (match_operand:GPR 5 "reg_or_0_operand"  "rJ")
+			  (match_operand:SI 6 "const_int_operand")] ;; model
+	 UNSPEC_SYNC_EXCHANGE))
+   (clobber (match_scratch:GPR 7 "="))]
+  ""
+{
+  return "%G6\\n\\t"
+	 "1:\\n\\t"
+	 "ll.\\t%0,%1\\n\\t"
+	 "and\\t%7,%0,%z3\\n\\t"
+	 "or%i5\\t%7,%7,%5\\n\\t"
+	 "sc.\\t%7,%1\\n\\t"
+	 "beqz\\t%7,1b\\n\\t";
+}
+  [(set (attr "length") (const_int 20))])
+
 (define_expand "atomic_exchange"
   [(set (match_operand:SHORT 0 "register_operand")
 	(unspec_volatile:SHORT
@@ -459,9 +482,9 @@
   ""
 {
   union loongarch_gen_fn_ptrs generator;
-  generator.fn_7 = gen_atomic_cas_value_cmp_and_7_si;
+  generator.fn_7 = gen_atomic_cas_value_exchange_7_si;
   loongarch_expand_atomic_qihi (generator, operands[0], operands[1],
-operands[1], operands[2], operands[3]);
+const0_rtx, operands[2], operands[3]);
   DONE;
 })
 
diff --git a/gcc/testsuite/gcc.target/loongarch/pr107713-1.c b/gcc/testsuite/gcc.target/loongarch/pr107713-1.c
new file mode 100644
index 000..d1536c95b27
--- /dev/null
+++ b/gcc/testsuite/gcc.target/loongarch/pr107713-1.c
@@ -0,0 +1,50 @@
+/* { dg-do run } */
+/* { dg-require-effective-target pthread } */
+/* { dg-options "-pthread" } */
+
+#include 
+
+char x, x1, x2;
+
+void *
+work1 (void *)
+{
+  for (int i = 0; i < 100; i++)
+x1 = __atomic_exchange_n (, x1, __ATOMIC_SEQ_CST);
+  return NULL;
+}
+
+void *
+work2 (void *)
+{
+  for (int i = 0; i < 100; i++)
+x2 = __atomic_exchange_n (, x2, __ATOMIC_SEQ_CST);
+  return NULL;
+}
+
+void
+test (void)
+{
+  x = 0;
+  x1 = 1;
+  x2 = 2;
+  pthread_t w1, w2;
+  if (pthread_create (, NULL, work1, NULL) != 0)
+__builtin_abort ();
+  if (pthread_create (, NULL, work2, NULL) != 0)
+__builtin_abort ();
+  if (pthread_join (w1, NULL) != 0)
+__builtin_abort ();
+  if (pthread_join (w2, NULL) != 0)
+__builtin_abort ();
+  if ((x ^ x1 ^ x2) != 3)
+__builtin_abort ();
+}
+
+int
+main ()
+{
+  int i;
+  for (i = 0; i < 1; i++)
+test ();
+}
diff --git a/gcc/testsuite/gcc.target/loongarch/pr107713-2.c b/gcc/testsuite/gcc.target/loongarch/pr107713-2.c
new file mode 100644
index 000..82d44db3d51
--- /dev/null
+++ b/gcc/testsuite/gcc.target/loongarch/pr107713-2.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-final { scan-assembler-times "beq|bne" 1 } } */
+
+char
+t (char *p, char x)
+{
+  return __atomic_exchange_n (p, 

Re: [PATCH] LoongArch: Fix atomic_exchange make comparison and may jump out

2022-11-16 Thread Jinyang He

On 2022/11/17 上午11:38, Xi Ruoyao wrote:


On Thu, 2022-11-17 at 10:55 +0800, Jinyang He wrote:

On 2022/11/17 上午9:39, Jinyang He wrote:


On 2022/11/16 下午7:46, Xi Ruoyao wrote:


On Wed, 2022-11-16 at 10:11 +0800, Jinyang He wrote:


+  return "%G6\\n\\t"
+    "1:\\n\\t"
+    "ll.\\t%0,%1\\n\\t"
+    "and\\t%7,%0,%z3\\n\\t"
+    "or%i5\\t%7,%7,%5\\n\\t"
+    "sc.\\t%7,%1\\n\\t"
+    "beqz\\t%7,1b\\n\\t";

Do we need a "dbar 0x700" after beqz?

/* snip */

That's worth discussing. Actually I don't see any dbar hint definition
like 0x700 in the manual right now.
Besides, I think what should be provided here is a relaxed version. And
whether the barrier exsit or not is depend on the specific
memory_order.

It's not related to memory order, but for a hardware issue workaround.
Jiaxun told me (via LKML):

     I had checked with Loongson guys and they confirmed that the
     workaround still needs to be applied to latest 3A4000 processors,
     including 3A4000 for MIPS and 3A5000 for LoongArch.
         Though, the reason behind the workaround varies with the
evaluation
     of their uArch, for GS464V based core, barrier is required as the
     uArch design allows regular load to be reordered after an atomic
     linked load, and that would break assumption of compiler atomic
     constraints.

That certainly seems to be needed, but before or after. It's beyond my
recognition and cc huang...@loongson.cn for help.


Pei told me the ll-sc works at present like follows,

uArch like:
    ll -> (ll.dbar ll.ld_atomic)
    sc -> (sc.dbar sc.st_atomic)

exchange:
ll.dbar
<---+
ll.ld_atomic $rd    |
...(no jmp) |
sc.dbar |
sc.st_stomic $rd    |
ld $rj -can-not-emit-at-+

The load $rj can not emit between ll.dbar and ll.ld_atomic because the
sc.dbar barrier it.


compare and exchange:
ll.dbar
<---+
ll.ld_atomic $rd    |
...(jmp) ---+--+
sc.dbar |  |
sc.st_stomic $rd    |  |
  |   <--+
ld $rj -may-emit-at-+

Jumping out ll-sc may lead loading $rj emit between ll.dbar and ll.atomic.


Thus, exchange not need dbar.





Without these dbar instructions I'd got random test failures in GCC
libgomp test suite.

Which test suite?

I mean when we didn't use dbar 0x700 for compare-and-exchange (during
the early development stage of GCC for LoongArch) I observed these
failures.

So we do need an additional dbar for compare-and-exchange, but do not
need it for a bare atomic exchange?

Yes.



Re: [PATCH] LoongArch: Fix atomic_exchange make comparison and may jump out

2022-11-16 Thread Xi Ruoyao via Gcc-patches
On Thu, 2022-11-17 at 10:55 +0800, Jinyang He wrote:
> On 2022/11/17 上午9:39, Jinyang He wrote:
> 
> > On 2022/11/16 下午7:46, Xi Ruoyao wrote:
> > 
> > > On Wed, 2022-11-16 at 10:11 +0800, Jinyang He wrote:
> > > 
> > > > > > +  return "%G6\\n\\t"
> > > > > > +    "1:\\n\\t"
> > > > > > +    "ll.\\t%0,%1\\n\\t"
> > > > > > +    "and\\t%7,%0,%z3\\n\\t"
> > > > > > +    "or%i5\\t%7,%7,%5\\n\\t"
> > > > > > +    "sc.\\t%7,%1\\n\\t"
> > > > > > +    "beqz\\t%7,1b\\n\\t";
> > > > > Do we need a "dbar 0x700" after beqz?
> > > > > 
> > > > > /* snip */
> > > > That's worth discussing. Actually I don't see any dbar hint definition
> > > > like 0x700 in the manual right now.
> > > > Besides, I think what should be provided here is a relaxed version. And
> > > > whether the barrier exsit or not is depend on the specific 
> > > > memory_order.
> > > It's not related to memory order, but for a hardware issue workaround.
> > > Jiaxun told me (via LKML):
> > > 
> > >     I had checked with Loongson guys and they confirmed that the
> > >     workaround still needs to be applied to latest 3A4000 processors,
> > >     including 3A4000 for MIPS and 3A5000 for LoongArch.
> > >         Though, the reason behind the workaround varies with the 
> > > evaluation
> > >     of their uArch, for GS464V based core, barrier is required as the
> > >     uArch design allows regular load to be reordered after an atomic
> > >     linked load, and that would break assumption of compiler atomic
> > >     constraints.
> > 
> > That certainly seems to be needed, but before or after. It's beyond my
> > recognition and cc huang...@loongson.cn for help.
> 
> 
> Pei told me the ll-sc works at present like follows,
> 
> uArch like:
>    ll -> (ll.dbar ll.ld_atomic)
>    sc -> (sc.dbar sc.st_atomic)
> 
> exchange:
> ll.dbar
> <---+
> ll.ld_atomic $rd    |
> ...(no jmp) |
> sc.dbar |
> sc.st_stomic $rd    |
> ld $rj -can-not-emit-at-+
> 
> The load $rj can not emit between ll.dbar and ll.ld_atomic because the
> sc.dbar barrier it.
> 
> 
> compare and exchange:
> ll.dbar
> <---+
> ll.ld_atomic $rd    |
> ...(jmp) ---+--+
> sc.dbar |  |
> sc.st_stomic $rd    |  |
>  |   <--+
> ld $rj -may-emit-at-+
> 
> Jumping out ll-sc may lead loading $rj emit between ll.dbar and ll.atomic.
> 
> 
> Thus, exchange not need dbar.
> 
> 
> > 
> > 
> > > 
> > > Without these dbar instructions I'd got random test failures in GCC
> > > libgomp test suite.
> 
> Which test suite?

I mean when we didn't use dbar 0x700 for compare-and-exchange (during
the early development stage of GCC for LoongArch) I observed these
failures.

So we do need an additional dbar for compare-and-exchange, but do not
need it for a bare atomic exchange?

-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University


Re: [PATCH] LoongArch: Fix atomic_exchange make comparison and may jump out

2022-11-16 Thread Jinyang He

On 2022/11/17 上午9:39, Jinyang He wrote:


On 2022/11/16 下午7:46, Xi Ruoyao wrote:


On Wed, 2022-11-16 at 10:11 +0800, Jinyang He wrote:


+  return "%G6\\n\\t"
+    "1:\\n\\t"
+    "ll.\\t%0,%1\\n\\t"
+    "and\\t%7,%0,%z3\\n\\t"
+    "or%i5\\t%7,%7,%5\\n\\t"
+    "sc.\\t%7,%1\\n\\t"
+    "beqz\\t%7,1b\\n\\t";

Do we need a "dbar 0x700" after beqz?

/* snip */

That's worth discussing. Actually I don't see any dbar hint definition
like 0x700 in the manual right now.
Besides, I think what should be provided here is a relaxed version. And
whether the barrier exsit or not is depend on the specific 
memory_order.

It's not related to memory order, but for a hardware issue workaround.
Jiaxun told me (via LKML):

    I had checked with Loongson guys and they confirmed that the
    workaround still needs to be applied to latest 3A4000 processors,
    including 3A4000 for MIPS and 3A5000 for LoongArch.
        Though, the reason behind the workaround varies with the 
evaluation

    of their uArch, for GS464V based core, barrier is required as the
    uArch design allows regular load to be reordered after an atomic
    linked load, and that would break assumption of compiler atomic
    constraints.


That certainly seems to be needed, but before or after. It's beyond my
recognition and cc huang...@loongson.cn for help.



Pei told me the ll-sc works at present like follows,

uArch like:
  ll -> (ll.dbar ll.ld_atomic)
  sc -> (sc.dbar sc.st_atomic)

exchange:
ll.dbar
<---+
ll.ld_atomic $rd    |
...(no jmp) |
sc.dbar |
sc.st_stomic $rd    |
ld $rj -can-not-emit-at-+

The load $rj can not emit between ll.dbar and ll.ld_atomic because the 
sc.dbar barrier it.



compare and exchange:
ll.dbar
<---+
ll.ld_atomic $rd    |
...(jmp) ---+--+
sc.dbar |  |
sc.st_stomic $rd    |  |
    |   <--+
ld $rj -may-emit-at-+

Jumping out ll-sc may lead loading $rj emit between ll.dbar and ll.atomic.


Thus, exchange not need dbar.







Without these dbar instructions I'd got random test failures in GCC
libgomp test suite.


Which test suite?




We use a non-zero hint here because it is treated exactly same as zero
in 3A5000, and the future LoongArch processors can fix the issue and
ignore the dbar 0x700 instruction.

Thanks, it's a nice workaround.




Re: [PATCH] LoongArch: Fix atomic_exchange make comparison and may jump out

2022-11-16 Thread Jinyang He

On 2022/11/16 下午7:46, Xi Ruoyao wrote:


On Wed, 2022-11-16 at 10:11 +0800, Jinyang He wrote:


+  return "%G6\\n\\t"
+    "1:\\n\\t"
+    "ll.\\t%0,%1\\n\\t"
+    "and\\t%7,%0,%z3\\n\\t"
+    "or%i5\\t%7,%7,%5\\n\\t"
+    "sc.\\t%7,%1\\n\\t"
+    "beqz\\t%7,1b\\n\\t";

Do we need a "dbar 0x700" after beqz?

/* snip */

That's worth discussing. Actually I don't see any dbar hint definition
like 0x700 in the manual right now.
Besides, I think what should be provided here is a relaxed version. And
whether the barrier exsit or not is depend on the specific memory_order.

It's not related to memory order, but for a hardware issue workaround.
Jiaxun told me (via LKML):

I had checked with Loongson guys and they confirmed that the
workaround still needs to be applied to latest 3A4000 processors,
including 3A4000 for MIPS and 3A5000 for LoongArch.

Though, the reason behind the workaround varies with the evaluation

of their uArch, for GS464V based core, barrier is required as the
uArch design allows regular load to be reordered after an atomic
linked load, and that would break assumption of compiler atomic
constraints.


That certainly seems to be needed, but before or after. It's beyond my
recognition and cc huang...@loongson.cn for help.




Without these dbar instructions I'd got random test failures in GCC
libgomp test suite.

We use a non-zero hint here because it is treated exactly same as zero
in 3A5000, and the future LoongArch processors can fix the issue and
ignore the dbar 0x700 instruction.

Thanks, it's a nice workaround.



Re: [PATCH] LoongArch: Fix atomic_exchange make comparison and may jump out

2022-11-16 Thread Xi Ruoyao via Gcc-patches
On Wed, 2022-11-16 at 10:11 +0800, Jinyang He wrote:

> > > +  return "%G6\\n\\t"
> > > +    "1:\\n\\t"
> > > +    "ll.\\t%0,%1\\n\\t"
> > > +    "and\\t%7,%0,%z3\\n\\t"
> > > +    "or%i5\\t%7,%7,%5\\n\\t"
> > > +    "sc.\\t%7,%1\\n\\t"
> > > +    "beqz\\t%7,1b\\n\\t";
> > Do we need a "dbar 0x700" after beqz?
> > 
> > /* snip */
> 
> That's worth discussing. Actually I don't see any dbar hint definition
> like 0x700 in the manual right now.
> Besides, I think what should be provided here is a relaxed version. And
> whether the barrier exsit or not is depend on the specific memory_order.

It's not related to memory order, but for a hardware issue workaround. 
Jiaxun told me (via LKML):

   I had checked with Loongson guys and they confirmed that the
   workaround still needs to be applied to latest 3A4000 processors,
   including 3A4000 for MIPS and 3A5000 for LoongArch.
   
   Though, the reason behind the workaround varies with the evaluation
   of their uArch, for GS464V based core, barrier is required as the
   uArch design allows regular load to be reordered after an atomic
   linked load, and that would break assumption of compiler atomic
   constraints.

Without these dbar instructions I'd got random test failures in GCC
libgomp test suite.

We use a non-zero hint here because it is treated exactly same as zero
in 3A5000, and the future LoongArch processors can fix the issue and
ignore the dbar 0x700 instruction.


-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University


Re: [PATCH] LoongArch: Fix atomic_exchange make comparison and may jump out

2022-11-15 Thread Jinyang He

On 2022/11/15 下午10:21, Xi Ruoyao wrote:


On Tue, 2022-11-15 at 21:03 +0800, Jinyang He wrote:

gcc/ChangeLog:

* config/loongarch/sync.md:
Add atomic_cas_value_exchange_and_7 and fix atomic_exchange.

nit:

* config/loongarch/sync.md (atomic_cas_value_exchange_and_7):
New define_insn.
(atomic_exchange): Use atomic_cas_value_exchange_and_7 instead
of atomic_cas_value_cmp_and.


gcc/testsuite/ChangeLog:

* gcc.target/loongarch/sync-1.c: New test.

Likewise, ChangeLog content should be indented with a tab. (Not 8
spaces: if my mail client changes my tab to 8 spaces I'm sorry).

/* snip */


OK. Thanks for the clear commit message and the explanation of format.



+  return "%G6\\n\\t"
+    "1:\\n\\t"
+    "ll.\\t%0,%1\\n\\t"
+    "and\\t%7,%0,%z3\\n\\t"
+    "or%i5\\t%7,%7,%5\\n\\t"
+    "sc.\\t%7,%1\\n\\t"
+    "beqz\\t%7,1b\\n\\t";

Do we need a "dbar 0x700" after beqz?

/* snip */


That's worth discussing. Actually I don't see any dbar hint definition
like 0x700 in the manual right now.
Besides, I think what should be provided here is a relaxed version. And
whether the barrier exsit or not is depend on the specific memory_order.

https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#_dbar





diff --git a/gcc/testsuite/gcc.target/loongarch/sync-1.c 
b/gcc/testsuite/gcc.target/loongarch/sync-1.c
new file mode 100644
index 0..cebed6a9b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/loongarch/sync-1.c
@@ -0,0 +1,104 @@
+/* Test __sync_test_and_set in atomic_exchange */
+/* { dg-do run } */
+/* { dg-options "-lpthread -std=c11" } */

This test seems not deterministic.  And the use of sched_yield is very
tricky, as the man page says:

sched_yield() is intended for use with  real-time  scheduling  policies
(i.e., SCHED_FIFO or SCHED_RR).  Use of sched_yield() with nondetermin‐
istic scheduling policies such as SCHED_OTHER is unspecified  and  very
likely means your application design is broken.


Yes, there might be something wrong. The test is just a variants from
llvm::tsan. It was presented to prove that the old implementation did
have problems.




I'd suggest to create a bug report at https://gcc.gnu.org/bugzilla


Thanks, I need to do that. It is must be I missing something at
https://gcc.gnu.org/contribute.html.



and
post this test in the PR.  Then add the PR number into the changelog,
and just add a { dg-do compile } and { dg-final { scan-assembler ... } }
test into the testsuite to ensure the correct ll/sc loop is generated.

A bug report also emphasises that this is a bug fix, which is suitable
for GCC 13 (in stage 3 now) and GCC 12 (the fix will be backported).


I will create a bug report where we all can discuss it.
Thanks for your review and help. :-)



Re: [PATCH] LoongArch: Fix atomic_exchange make comparison and may jump out

2022-11-15 Thread Xi Ruoyao via Gcc-patches
On Tue, 2022-11-15 at 21:03 +0800, Jinyang He wrote:
> gcc/ChangeLog:
> 
> * config/loongarch/sync.md:
> Add atomic_cas_value_exchange_and_7 and fix atomic_exchange.

nit:

* config/loongarch/sync.md (atomic_cas_value_exchange_and_7): 
New define_insn.
(atomic_exchange): Use atomic_cas_value_exchange_and_7 instead 
of atomic_cas_value_cmp_and.

> gcc/testsuite/ChangeLog:
>
> * gcc.target/loongarch/sync-1.c: New test.

Likewise, ChangeLog content should be indented with a tab. (Not 8
spaces: if my mail client changes my tab to 8 spaces I'm sorry).

/* snip */

> +  return "%G6\\n\\t"
> +    "1:\\n\\t"
> +    "ll.\\t%0,%1\\n\\t"
> +    "and\\t%7,%0,%z3\\n\\t"
> +    "or%i5\\t%7,%7,%5\\n\\t"
> +    "sc.\\t%7,%1\\n\\t"
> +    "beqz\\t%7,1b\\n\\t";

Do we need a "dbar 0x700" after beqz?

/* snip */

> diff --git a/gcc/testsuite/gcc.target/loongarch/sync-1.c 
> b/gcc/testsuite/gcc.target/loongarch/sync-1.c
> new file mode 100644
> index 0..cebed6a9b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/loongarch/sync-1.c
> @@ -0,0 +1,104 @@
> +/* Test __sync_test_and_set in atomic_exchange */
> +/* { dg-do run } */
> +/* { dg-options "-lpthread -std=c11" } */

This test seems not deterministic.  And the use of sched_yield is very
tricky, as the man page says:

   sched_yield() is intended for use with  real-time  scheduling  policies
   (i.e., SCHED_FIFO or SCHED_RR).  Use of sched_yield() with nondetermin‐
   istic scheduling policies such as SCHED_OTHER is unspecified  and  very
   likely means your application design is broken.

I'd suggest to create a bug report at https://gcc.gnu.org/bugzilla and
post this test in the PR.  Then add the PR number into the changelog,
and just add a { dg-do compile } and { dg-final { scan-assembler ... } }
test into the testsuite to ensure the correct ll/sc loop is generated.

A bug report also emphasises that this is a bug fix, which is suitable
for GCC 13 (in stage 3 now) and GCC 12 (the fix will be backported).

-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University


[PATCH] LoongArch: Fix atomic_exchange make comparison and may jump out

2022-11-15 Thread Jinyang He
gcc/ChangeLog:

* config/loongarch/sync.md:
Add atomic_cas_value_exchange_and_7 and fix atomic_exchange.

gcc/testsuite/ChangeLog:

* gcc.target/loongarch/sync-1.c: New test.
---
 gcc/config/loongarch/sync.md|  27 -
 gcc/testsuite/gcc.target/loongarch/sync-1.c | 104 
 2 files changed, 129 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/loongarch/sync-1.c

diff --git a/gcc/config/loongarch/sync.md b/gcc/config/loongarch/sync.md
index 0c4f1983e..8a8e6247b 100644
--- a/gcc/config/loongarch/sync.md
+++ b/gcc/config/loongarch/sync.md
@@ -448,6 +448,29 @@
 }
   [(set (attr "length") (const_int 32))])
 
+(define_insn "atomic_cas_value_exchange_and_7_"
+  [(set (match_operand:GPR 0 "register_operand" "=")
+   (match_operand:GPR 1 "memory_operand" "+ZC"))
+   (set (match_dup 1)
+   (unspec_volatile:GPR [(match_operand:GPR 2 "reg_or_0_operand" "rJ")
+ (match_operand:GPR 3 "reg_or_0_operand" "rJ")
+ (match_operand:GPR 4 "reg_or_0_operand" "rJ")
+ (match_operand:GPR 5 "reg_or_0_operand"  "rJ")
+ (match_operand:SI 6 "const_int_operand")] ;; model
+UNSPEC_SYNC_EXCHANGE))
+   (clobber (match_scratch:GPR 7 "="))]
+  ""
+{
+  return "%G6\\n\\t"
+"1:\\n\\t"
+"ll.\\t%0,%1\\n\\t"
+"and\\t%7,%0,%z3\\n\\t"
+"or%i5\\t%7,%7,%5\\n\\t"
+"sc.\\t%7,%1\\n\\t"
+"beqz\\t%7,1b\\n\\t";
+}
+  [(set (attr "length") (const_int 20))])
+
 (define_expand "atomic_exchange"
   [(set (match_operand:SHORT 0 "register_operand")
(unspec_volatile:SHORT
@@ -459,9 +482,9 @@
   ""
 {
   union loongarch_gen_fn_ptrs generator;
-  generator.fn_7 = gen_atomic_cas_value_cmp_and_7_si;
+  generator.fn_7 = gen_atomic_cas_value_exchange_and_7_si;
   loongarch_expand_atomic_qihi (generator, operands[0], operands[1],
-   operands[1], operands[2], operands[3]);
+   const0_rtx, operands[2], operands[3]);
   DONE;
 })
 
diff --git a/gcc/testsuite/gcc.target/loongarch/sync-1.c 
b/gcc/testsuite/gcc.target/loongarch/sync-1.c
new file mode 100644
index 0..cebed6a9b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/loongarch/sync-1.c
@@ -0,0 +1,104 @@
+/* Test __sync_test_and_set in atomic_exchange */
+/* { dg-do run } */
+/* { dg-options "-lpthread -std=c11" } */
+
+#include 
+#include 
+#include 
+#include 
+
+#define NR_THREAD 16
+#define NR_DATA 1000
+#define ITER_COUNT 1
+
+static int _data[NR_DATA];
+static char _lock;
+static int _overcnt;
+
+static inline void proc_yield(int cnt)
+{
+  __asm__ __volatile__("":::"memory");
+}
+
+static void unlock()
+{
+  return atomic_store_explicit(&_lock, 0, memory_order_seq_cst);
+}
+
+static int trylock()
+{
+  return atomic_exchange_explicit(&_lock, 1, memory_order_acquire) == 0;
+}
+
+static void lockslow()
+{
+  for (int i = 0;; i++) {
+if (i < 10)
+  proc_yield(i);
+else
+  sched_yield();
+if (atomic_load_explicit(&_lock, memory_order_relaxed) == 0
+  && atomic_exchange_explicit(&_lock, 1, memory_order_acquire) == 0)
+  return;
+  }
+}
+
+static void lock()
+{
+  if (trylock())
+return;
+  lockslow();
+}
+
+static void checkeq(int a, int b)
+{
+  if (a != b)
+__builtin_abort();
+}
+
+static void adddata()
+{
+  int i, v;
+  lock();
+  v = _data[0];
+  for (i = 0; i < NR_DATA; i++) {
+checkeq(_data[i], v);
+_data[i]++;
+  }
+  unlock();
+}
+
+static void backoff()
+{
+  int i, data[NR_DATA] = {0};
+  for (i = 0; i < NR_DATA; i++) {
+data[i]++;
+checkeq(data[i], 1);
+  }
+}
+
+static void *write_mutex_thread(void *unused)
+{
+  int i;
+  for (i = 0; i < ITER_COUNT; i++) {
+adddata();
+backoff();
+  }
+  atomic_fetch_add(&_overcnt, 1);
+}
+
+int main()
+{
+  int cnt;
+
+  pthread_t threads[NR_THREAD];
+  for (int i = 0; i < NR_THREAD; i++)
+pthread_create([i], 0, write_mutex_thread, NULL);
+  for (int i = 0; i < NR_THREAD; i++)
+pthread_detach(threads[i]);
+  while(cnt != NR_THREAD) {
+sched_yield();
+cnt = atomic_load(&_overcnt);
+  }
+  return 0;
+}
-- 
2.34.3