[PATCH] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`

Gary Guo posted 1 patch 1 month, 2 weeks ago
init/Kconfig                  |  6 +++++-
scripts/Kconfig.include       |  3 +++
scripts/rustc-llvm-version.sh | 22 ++++++++++++++++++++++
3 files changed, 30 insertions(+), 1 deletion(-)
create mode 100755 scripts/rustc-llvm-version.sh
[PATCH] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`
Posted by Gary Guo 1 month, 2 weeks ago
Each version of Rust supports a range of LLVM versions. There are cases where
we want to gate a config on the LLVM version instead of the Rust version.
Normalized cfi integer tags are one example [1].

The invocation of rustc-version is being moved from init/Kconfig to
scripts/Kconfig.include for consistency with cc-version.

Link: https://lore.kernel.org/all/20240925-cfi-norm-kasan-fix-v1-1-0328985cdf33@google.com/ [1]
Signed-off-by: Gary Guo <gary@garyguo.net>
---
 init/Kconfig                  |  6 +++++-
 scripts/Kconfig.include       |  3 +++
 scripts/rustc-llvm-version.sh | 22 ++++++++++++++++++++++
 3 files changed, 30 insertions(+), 1 deletion(-)
 create mode 100755 scripts/rustc-llvm-version.sh

diff --git a/init/Kconfig b/init/Kconfig
index fbd0cb06a50a..304e2bd32bfd 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -62,7 +62,7 @@ config LLD_VERSION
 
 config RUSTC_VERSION
 	int
-	default $(shell,$(srctree)/scripts/rustc-version.sh $(RUSTC))
+	default $(rustc-version)
 	help
 	  It does not depend on `RUST` since that one may need to use the version
 	  in a `depends on`.
@@ -78,6 +78,10 @@ config RUST_IS_AVAILABLE
 	  In particular, the Makefile target 'rustavailable' is useful to check
 	  why the Rust toolchain is not being detected.
 
+config RUSTC_LLVM_VERSION
+	int
+	default $(rustc-llvm-version)
+
 config CC_CAN_LINK
 	bool
 	default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m64-flag)) if 64BIT
diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
index 785a491e5996..33193ca6e803 100644
--- a/scripts/Kconfig.include
+++ b/scripts/Kconfig.include
@@ -65,6 +65,9 @@ cc-option-bit = $(if-success,$(CC) -Werror $(1) -E -x c /dev/null -o /dev/null,$
 m32-flag := $(cc-option-bit,-m32)
 m64-flag := $(cc-option-bit,-m64)
 
+rustc-version := $(shell,$(srctree)/scripts/rustc-version.sh $(RUSTC))
+rustc-llvm-version := $(shell,$(srctree)/scripts/rustc-llvm-version.sh $(RUSTC))
+
 # $(rustc-option,<flag>)
 # Return y if the Rust compiler supports <flag>, n otherwise
 # Calls to this should be guarded so that they are not evaluated if
diff --git a/scripts/rustc-llvm-version.sh b/scripts/rustc-llvm-version.sh
new file mode 100755
index 000000000000..b8ffa24afea8
--- /dev/null
+++ b/scripts/rustc-llvm-version.sh
@@ -0,0 +1,22 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+#
+# Usage: $ ./rustc-version.sh rustc
+#
+# Print the LLVM version that the Rust compiler uses in a 6 digit form.
+
+# Convert the version string x.y.z to a canonical up-to-6-digits form.
+get_canonical_version()
+{
+	IFS=.
+	set -- $1
+	echo $((10000 * $1 + 100 * $2 + $3))
+}
+
+if output=$("$@" --version --verbose 2>/dev/null | grep LLVM); then
+	set -- $output
+	get_canonical_version $3
+else
+	echo 0
+	exit 1
+fi

base-commit: f5e50614e39e74401b492a2fa125f2e2b6458bab
-- 
2.44.1
Re: [PATCH] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`
Posted by Miguel Ojeda 1 month, 1 week ago
On Fri, Oct 11, 2024 at 1:41 PM Gary Guo <gary@garyguo.net> wrote:
>
> Each version of Rust supports a range of LLVM versions. There are cases where
> we want to gate a config on the LLVM version instead of the Rust version.
> Normalized cfi integer tags are one example [1].
>
> The invocation of rustc-version is being moved from init/Kconfig to
> scripts/Kconfig.include for consistency with cc-version.
>
> Link: https://lore.kernel.org/all/20240925-cfi-norm-kasan-fix-v1-1-0328985cdf33@google.com/ [1]
> Signed-off-by: Gary Guo <gary@garyguo.net>

Applied to `rust-fixes` -- thanks everyone!

    [ Added missing `-llvm` to the Usage documentation. - Miguel ]

Cheers,
Miguel
Re: [PATCH] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`
Posted by Miguel Ojeda 1 month, 2 weeks ago
On Fri, Oct 11, 2024 at 1:41 PM Gary Guo <gary@garyguo.net> wrote:
>
> The invocation of rustc-version is being moved from init/Kconfig to
> scripts/Kconfig.include for consistency with cc-version.

Yeah, I am ambivalent. Dropping them would minimize changes and avoid
introducing something only used once, which would be nice too. Happy
either way.

> +if output=$("$@" --version --verbose 2>/dev/null | grep LLVM); then

Similarly, I wonder if we should use '^LLVM version: ' here or similar
to minimize the chances the "LLVM" string appears elsewhere in the
future (perhaps in a custom string a vendor adds, though I would
expect them to add it lowercase). We are relying on having a $3 when
splitting anyway.

Depending on what Masahiro prefers, I will take this one or the
one-invocation-only one.

Thanks Gary!

Cheers,
Miguel
Re: [PATCH] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`
Posted by Gary Guo 1 month, 2 weeks ago
On Fri, 11 Oct 2024 13:53:47 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Fri, Oct 11, 2024 at 1:41 PM Gary Guo <gary@garyguo.net> wrote:
> >
> > The invocation of rustc-version is being moved from init/Kconfig to
> > scripts/Kconfig.include for consistency with cc-version.  
> 
> Yeah, I am ambivalent. Dropping them would minimize changes and avoid
> introducing something only used once, which would be nice too. Happy
> either way.
> 

Another motivation is that in my helper inline series, I need to do
arithmetic on LLVM version (divide it by 10000 to get major verison),
which isn't possible for config options, but will work for variables
defiend in Kconfig.include.

I didn't mention it in the commit message for this patch because this
is not my patch series :)

Best,
Gary

> > +if output=$("$@" --version --verbose 2>/dev/null | grep LLVM); then  
> 
> Similarly, I wonder if we should use '^LLVM version: ' here or similar
> to minimize the chances the "LLVM" string appears elsewhere in the
> future (perhaps in a custom string a vendor adds, though I would
> expect them to add it lowercase). We are relying on having a $3 when
> splitting anyway.
> 
> Depending on what Masahiro prefers, I will take this one or the
> one-invocation-only one.
> 
> Thanks Gary!
> 
> Cheers,
> Miguel
Re: [PATCH] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`
Posted by Masahiro Yamada 1 month, 1 week ago
On Fri, Oct 11, 2024 at 9:06 PM Gary Guo <gary@garyguo.net> wrote:
>
> On Fri, 11 Oct 2024 13:53:47 +0200
> Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
>
> > On Fri, Oct 11, 2024 at 1:41 PM Gary Guo <gary@garyguo.net> wrote:
> > >
> > > The invocation of rustc-version is being moved from init/Kconfig to
> > > scripts/Kconfig.include for consistency with cc-version.
> >
> > Yeah, I am ambivalent. Dropping them would minimize changes and avoid
> > introducing something only used once, which would be nice too. Happy
> > either way.
>
> Another motivation is that in my helper inline series, I need to do
> arithmetic on LLVM version (divide it by 10000 to get major verison),
> which isn't possible for config options, but will work for variables
> defiend in Kconfig.include.
>
> I didn't mention it in the commit message for this patch because this
> is not my patch series :)


I tend to agree with Muguel.
The motivation of having cc-version in scripts/Kconfig.include
is to check the presence of the supported C compiler, as C compiler
is mandatory (in contrast, Rust compiler is optional).

If you have a reason to have it in scripts/Kconfig.include
for your future works, it is OK with me, but I cannot judge it.






>
> Best,
> Gary
>
> > > +if output=$("$@" --version --verbose 2>/dev/null | grep LLVM); then
> >
> > Similarly, I wonder if we should use '^LLVM version: ' here or similar
> > to minimize the chances the "LLVM" string appears elsewhere in the
> > future (perhaps in a custom string a vendor adds, though I would
> > expect them to add it lowercase). We are relying on having a $3 when
> > splitting anyway.
> >
> > Depending on what Masahiro prefers, I will take this one or the
> > one-invocation-only one.
> >
> > Thanks Gary!
> >
> > Cheers,
> > Miguel
>
>


-- 
Best Regards
Masahiro Yamada
Re: [PATCH] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`
Posted by Miguel Ojeda 1 month, 1 week ago
On Mon, Oct 14, 2024 at 6:27 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> I tend to agree with Muguel.
> The motivation of having cc-version in scripts/Kconfig.include
> is to check the presence of the supported C compiler, as C compiler
> is mandatory (in contrast, Rust compiler is optional).
>
> If you have a reason to have it in scripts/Kconfig.include
> for your future works, it is OK with me, but I cannot judge it.

Thanks Masahiro -- OK, since it seems Gary needs it, and we already
have the patch here, I will take that one.

Cheers,
Miguel