Date: Mon, 3 Jun 2024 17:31:41 +0200
From: Alexandre Ghiti <alex@...ti.fr>
To: Nathan Chancellor <nathan@...nel.org>,
 Alexandre Ghiti <alexghiti@...osinc.com>,
 Conor Dooley <conor.dooley@...rochip.com>
Cc: Jonathan Corbet <corbet@....net>, Paul Walmsley
 <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>,
 Albert Ou <aou@...s.berkeley.edu>, Peter Zijlstra <peterz@...radead.org>,
 Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
 Waiman Long <longman@...hat.com>, Boqun Feng <boqun.feng@...il.com>,
 Arnd Bergmann <arnd@...db.de>, Leonardo Bras <leobras@...hat.com>,
 Guo Ren <guoren@...nel.org>, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org,
 linux-arch@...r.kernel.org, llvm@...ts.linux.dev
Subject: Re: [PATCH 2/7] riscv: Implement cmpxchg8/16() using Zabha

Hi Conor, Nathan,

On 29/05/2024 17:57, Nathan Chancellor wrote:
> 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.


FYI, it was just merged https://github.com/llvm/llvm-project/pull/93831

Thanks again,

Alex


>
>> Thanks for your thorough review!
> Thanks for taking LLVM support into consideration :)
>
> Cheers,
> Nathan
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Powered by blists - more mailing lists