Re: [PATCH v2 01/10] riscv: Implement cmpxchg32/64() using Zacas

[Nathan Chancellor]

On Tue, Jul 16, 2024 at 02:19:57PM +0200, Alexandre Ghiti wrote:
> On Fri, Jul 5, 2024 at 7:27 PM Nathan Chancellor <nathan@xxxxxxxxxx> wrote:
> > Another alternative would be to require LLVM 17+ for RISC-V, which may
> > not be the worst alternative, since I think most people doing serious
> > work with clang will probably be living close to tip of tree anyways
> > because of all the extension work that goes on upstream.
> 
> Stupid question but why the fix in llvm 17 was not backported to
> previous versions?

Unfortunately, LLVM releases are only supported for a few months with
fixes, unlike GCC that supported their releases for a few years. By the
time this issue was uncovered and resolved in LLVM main (17 at the
time), LLVM 16 was no longer supported.

I could potentially patch the kernel.org toolchains but that doesn't fix
the issue for other versions of clang out there.

> Anyway, I'd rather require llvm 17+ than add a bunch of preprocessor
> guards in this file (IIUC what you said above) as it is complex
> enough.

Sure, this is not a super unreasonable issue to bump the minimum
supported version for RISC-V over in my opinion, so no real objections
from me.

> @Conor Dooley @Palmer Dabbelt WDYT? Is there any interest in
> supporting llvm < 17? We may encounter this bug again in the future so
> I'd be in favor of moving to llvm 17+.

FWIW, I would envision a diff like this (assuming it actually works to
resolve this issue, I didn't actually test it):

diff --git a/scripts/min-tool-version.sh b/scripts/min-tool-version.sh
index 91c91201212c..e81eb7ed257d 100755
--- a/scripts/min-tool-version.sh
+++ b/scripts/min-tool-version.sh
@@ -28,6 +28,8 @@ llvm)
 		echo 15.0.0
 	elif [ "$SRCARCH" = loongarch ]; then
 		echo 18.0.0
+	elif [ "$SRCARCH" = riscv ]; then
+		echo 17.0.0
 	else
 		echo 13.0.1
 	fi

Cheers,
Nathan