Re: [PATCH 2/7] riscv: Implement cmpxchg8/16() using Zabha

[Alexandre Ghiti]

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@xxxxxxxxxxxx/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@xxxxxxxxxxxx/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@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-riscv