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<mode> and fix atomic_exchange<mode>.
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.<amo>\\t%0,%1\\n\\t"
+ "and\\t%7,%0,%z3\\n\\t"
+ "or%i5\\t%7,%7,%5\\n\\t"
+ "sc.<amo>\\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 000000000..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. :-)
