From: Nathan Chancellor <nathan@kernel.org>
To: Alexandre Ghiti <alexghiti@rivosinc.com>
Cc: Jonathan Corbet <corbet@lwn.net>,
Paul Walmsley <paul.walmsley@sifive.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Albert Ou <aou@eecs.berkeley.edu>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>, Will Deacon <will@kernel.org>,
Waiman Long <longman@redhat.com>,
Boqun Feng <boqun.feng@gmail.com>, Arnd Bergmann <arnd@arndb.de>,
Leonardo Bras <leobras@redhat.com>, Guo Ren <guoren@kernel.org>,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-riscv@lists.infradead.org, linux-arch@vger.kernel.org,
llvm@lists.linux.dev
Subject: Re: [PATCH 2/7] riscv: Implement cmpxchg8/16() using Zabha
Date: Wed, 29 May 2024 08:57:49 -0700 [thread overview]
Message-ID: <20240529155749.GA1339768@thelio-3990X> (raw)
In-Reply-To: <CAHVXubjYVjOH8RKaF1h=iogO3xBM6k+xrGwkPnc-md2oRxbxrQ@mail.gmail.com>
On Wed, May 29, 2024 at 02:49:58PM +0200, Alexandre Ghiti wrote:
> Then I missed that, I should have checked the generated code. Is the
> extension version "1p0" in '-march=' only required for experimental
> extensions?
I think so, if my understanding of the message is correct.
> But from Conor comment here [1], we should not enable extensions that
> are only experimental. In that case, we should be good with this.
>
> [1] https://lore.kernel.org/linux-riscv/20240528151052.313031-1-alexghiti@rivosinc.com/T/#mefb283477bce852f3713cbbb4ff002252281c9d5
Yeah, I tend to agree with Conor on that front. I had not noticed that
part of the message when I was looking at other parts of this thread. I
could see an argument for allowing experimental extensions for
qualification purposes but I think it does create a bit of a support
nightmare, especially when there are breaking changes across revisions.
> > config EXPERIMENTAL_EXTENSIONS
> > bool
> >
> > config TOOLCHAIN_HAS_ZABHA
> > def_bool y
> > select EXPERIMENTAL_EXETNSIONS if CC_IS_CLANG
> > ...
> >
> > config TOOLCHAIN_HAS_ZACAS
> > def_bool_y
> > # ZACAS was experimental until Clang 19: https://github.com/llvm/llvm-project/commit/95aab69c109adf29e183090c25dc95c773215746
> > select EXPERIMENTAL_EXETNSIONS if CC_IS_CLANG && CLANG_VERSION < 190000
> > ...
> >
> > Then in the Makefile:
> >
> > ifdef CONFIG_EXPERIMENTAL_EXTENSIONS
> > KBUILD_AFLAGS += -menable-experimental-extensions
> > KBUILD_CFLAGS += -menable-experimental-extensions
> > endif
Perhaps with that in mind, maybe EXPERIMENTAL_EXTENSIONS (or whatever)
should be a user selectable option and the TOOLCHAIN values depend on it
when the user has a clang version that does not support the ratified
version.
> That's a good idea to me, let's see what Conor thinks [2]
>
> [2] https://lore.kernel.org/linux-riscv/20240528151052.313031-1-alexghiti@rivosinc.com/T/#m1d798dfc4c27e5b6d9e14117d81b577ace123322
FWIW, I think your plan of removing support for the experimental version
of the extension and pushing to remove the experimental status in LLVM
(especially since it seems like it is ratified like zacas?
https://jira.riscv.org/browse/RVS-1685) is probably the best thing going
forward. If the LLVM folks are made aware soon, it should be easy to get
that change into clang-19, which is branching at the end of July I
believe.
> Thanks for your thorough review!
Thanks for taking LLVM support into consideration :)
Cheers,
Nathan
next prev parent reply other threads:[~2024-05-29 15:57 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-28 15:10 [PATCH 0/7] Zacas/Zabha support and qspinlocks Alexandre Ghiti
2024-05-28 15:10 ` [PATCH 1/7] riscv: Implement cmpxchg32/64() using Zacas Alexandre Ghiti
2024-05-28 15:34 ` Conor Dooley
2024-05-29 12:20 ` Alexandre Ghiti
2024-05-30 14:43 ` Conor Dooley
2024-05-28 18:16 ` Andrea Parri
2024-05-28 15:10 ` [PATCH 2/7] riscv: Implement cmpxchg8/16() using Zabha Alexandre Ghiti
2024-05-28 19:31 ` Nathan Chancellor
2024-05-29 12:49 ` Alexandre Ghiti
2024-05-29 15:57 ` Nathan Chancellor [this message]
2024-06-03 15:31 ` Alexandre Ghiti
2024-05-28 23:54 ` Andrea Parri
2024-05-29 12:29 ` Alexandre Ghiti
2024-05-29 12:55 ` Alexandre Ghiti
2024-05-28 15:10 ` [PATCH 3/7] riscv: Implement arch_cmpxchg128() using Zacas Alexandre Ghiti
2024-05-28 15:10 ` [PATCH 4/7] riscv: Implement xchg8/16() using Zabha Alexandre Ghiti
2024-05-28 15:22 ` Conor Dooley
2024-05-29 6:15 ` Alexandre Ghiti
2024-05-28 18:00 ` Andrea Parri
2024-05-29 8:04 ` Alexandre Ghiti
2024-05-28 15:10 ` [PATCH 5/7] asm-generic: ticket-lock: Reuse arch_spinlock_t of qspinlock Alexandre Ghiti
2024-05-28 15:10 ` [PATCH 6/7] asm-generic: ticket-lock: Add separate ticket-lock.h Alexandre Ghiti
2024-05-28 15:10 ` [PATCH 7/7] riscv: Add qspinlock support based on Zabha extension Alexandre Ghiti
2024-05-29 0:55 ` Andrea Parri
2024-05-31 13:37 ` Alexandre Ghiti
2024-05-31 15:52 ` Andrea Parri
2024-06-01 6:18 ` Guo Ren
2024-06-03 0:41 ` Andrea Parri
2024-05-29 9:23 ` Guo Ren
2024-05-29 13:03 ` Alexandre Ghiti
2024-05-30 1:54 ` Guo Ren
2024-05-30 5:30 ` Alexandre Ghiti
2024-05-31 1:57 ` Guo Ren
2024-05-31 6:22 ` Alexandre Ghiti
2024-05-31 6:42 ` Guo Ren
2024-06-03 9:21 ` Alexandre Ghiti
2024-06-03 11:11 ` Guo Ren
2024-05-31 13:10 ` Guo Ren
2024-06-03 9:49 ` Alexandre Ghiti
2024-06-03 11:28 ` Guo Ren
2024-06-03 11:34 ` Alexandre Ghiti
2024-06-03 11:44 ` Guo Ren
2024-06-03 11:49 ` Alexandre Ghiti
2024-06-03 11:57 ` Guo Ren
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=20240529155749.GA1339768@thelio-3990X \
--to=nathan@kernel.org \
--cc=alexghiti@rivosinc.com \
--cc=aou@eecs.berkeley.edu \
--cc=arnd@arndb.de \
--cc=boqun.feng@gmail.com \
--cc=corbet@lwn.net \
--cc=guoren@kernel.org \
--cc=leobras@redhat.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=llvm@lists.linux.dev \
--cc=longman@redhat.com \
--cc=mingo@redhat.com \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=peterz@infradead.org \
--cc=will@kernel.org \
/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