From: Jim Wilson <jimw@sifive.com>
To: Christoph Muellner <cmuellner@gcc.gnu.org>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>,
Kito Cheng <kito.cheng@sifive.com>
Subject: Re: [PATCH v2 09/10] RISC-V: Provide programmatic implementation of CAS [PR 100266]
Date: Wed, 5 May 2021 17:27:19 -0700 [thread overview]
Message-ID: <CAFyWVaZJDcVzNgn4d-Vu_7gF6oC4Pd9bnWvwRTfSxnO-pZ8Mrw@mail.gmail.com> (raw)
In-Reply-To: <20210505193651.2075405-10-cmuellner@gcc.gnu.org>
On Wed, May 5, 2021 at 12:37 PM Christoph Muellner <cmuellner@gcc.gnu.org>
wrote:
> The existing CAS implementation uses an INSN definition, which provides
> the core LR/SC sequence. Additionally to that, there is a follow-up code,
> that evaluates the results and calculates the return values.
> This has two drawbacks: a) an extension to sub-word CAS implementations
> is not possible (even if, then it would be unmaintainable), and b) the
> implementation is hard to maintain/improve.
> This patch provides a programmatic implementation of CAS, similar
> like many other architectures are having one.
>
A comment that Andrew Waterman made to me today about the safety of this
under various circumstances got me thinking, and I realized that without
the special cas pattern we can get reloads in the middle of the sequence
which would be bad. Experimenting a bit, I managed to prove it. This is
using the old version of the patch which I already had handy, but I'm sure
the new version will behave roughly the same way. Using the testsuite
testcase atomic-compare-exchange-3.c as before, and adding a lot of
-ffixed-X options to simulate high register pressure, with the compiler
command
./xgcc -B./ -O2 -S tmp.c -ffixed-x16 -ffixed-x17 -ffixed-x18 -ffixed-x19
-ffixed-x20 -ffixed-x21 -ffixed-x22 -ffixed-x23 -ffixed-x24 -ffixed-x25
-ffixed-x26 -ffixed-x27 -ffixed-x28 -ffixed-x29 -ffixed-x30 -ffixed-x31
-ffixed-x15 -ffixed-x14 -ffixed-x13 -ffixed-x12 -ffixed-s0 -ffixed-s1
-ffixed-t2 -ffixed-t1 -ffixed-t0
I get for the first lr/sc loop
.L2:
lui a1,%hi(v)
addi a0,a1,%lo(v)
lr.w a1, 0(a0)
ld a0,8(sp)
sw a1,24(sp)
bne a1,a0,.L39
lui a1,%hi(v)
addi a0,a1,%lo(v)
lw a1,16(sp)
sd ra,24(sp)
sc.w ra, a1, 0(a0)
sext.w a1,ra
ld ra,24(sp)
bne a1,zero,.L2
and note all of the misc load/store instructions added by reload. I don't
think this is safe or guaranteed to work. With the cas pattern, any
reloads are guaranteed to be emitted before and/or after the lr/sc loop.
With the separate patterns, there is no way to ensure that we won't get
accidental reloads in the middle of the lr/sc loop.
I think we need to keep the cas pattern. We can always put C code inside
the output template of the cas pattern if that is helpful. It can do any
necessary tests and then return an appropriate string for the instructions
we want.
Jim
next prev parent reply other threads:[~2021-05-06 0:27 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-05 19:36 [PATCH v2 00/10] [RISC-V] Atomics improvements [PR100265/PR100266] Christoph Muellner
2021-05-05 19:36 ` [PATCH v2 01/10] RISC-V: Simplify memory model code [PR 100265] Christoph Muellner
2021-05-05 19:36 ` [PATCH v2 02/10] RISC-V: Emit proper memory ordering suffixes for AMOs " Christoph Muellner
2021-05-05 19:36 ` [PATCH v2 03/10] RISC-V: Eliminate %F specifier from riscv_print_operand() " Christoph Muellner
2021-05-05 19:36 ` [PATCH v2 04/10] RISC-V: Use STORE instead of AMOSWAP for atomic stores " Christoph Muellner
2021-05-05 19:36 ` [PATCH v2 05/10] RISC-V: Emit fences according to chosen memory model " Christoph Muellner
2021-05-05 19:36 ` [PATCH v2 06/10] RISC-V: Implement atomic_{load,store} " Christoph Muellner
2021-05-05 19:36 ` [PATCH v2 07/10] RISC-V: Model INSNs for LR and SC [PR 100266] Christoph Muellner
2021-05-05 19:36 ` [PATCH v2 08/10] RISC-V: Add s.ext-consuming " Christoph Muellner
2021-05-05 19:36 ` [PATCH v2 09/10] RISC-V: Provide programmatic implementation of CAS " Christoph Muellner
2021-05-06 0:27 ` Jim Wilson [this message]
2021-05-05 19:36 ` [PATCH v2 10/10] RISC-V: Introduce predicate "riscv_sync_memory_operand" " Christoph Muellner
2022-10-11 19:06 ` [PATCH v2 00/10] [RISC-V] Atomics improvements [PR100265/PR100266] Vineet Gupta
2022-10-11 19:31 ` Palmer Dabbelt
2022-10-11 20:46 ` Christoph Müllner
2022-10-11 23:31 ` Vineet Gupta
2022-10-12 0:15 ` Palmer Dabbelt
2022-10-12 8:03 ` Christoph Müllner
2022-10-13 23:11 ` Jeff Law
2022-10-12 17:16 ` Andrea Parri
2022-10-20 19:01 ` Andrea Parri
2022-10-29 5:02 ` Jeff Law
2022-10-13 23:04 ` Jeff Law
2022-10-13 22:39 ` Jeff Law
2022-10-13 23:14 ` Palmer Dabbelt
2022-10-14 11:03 ` Christoph Müllner
2022-10-14 20:39 ` Jeff Law
2022-10-14 21:57 ` Palmer Dabbelt
2022-10-15 0:31 ` Palmer Dabbelt
2022-10-14 0:14 ` Vineet Gupta
2022-10-11 23:14 ` Jeff Law
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFyWVaZJDcVzNgn4d-Vu_7gF6oC4Pd9bnWvwRTfSxnO-pZ8Mrw@mail.gmail.com \
--to=jimw@sifive.com \
--cc=cmuellner@gcc.gnu.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=kito.cheng@sifive.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).