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 */
> + 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 */
> 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.
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 <[email protected]>
School of Aerospace Science and Technology, Xidian University
