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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux