| Message ID | 20240819213534.4080408-2-mmaurer@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/4] kbuild: rust: Define probing macros for rustc | expand |
Hi Matthew, On Mon, Aug 19, 2024 at 11:35 PM Matthew Maurer <mmaurer@google.com> wrote: > > Creates flag probe macro variants for `rustc`. These are helpful > because: > > 1. `rustc` support will soon be a minimum rather than a pinned version. > 2. We already support multiple LLVMs linked into `rustc`, and these are > needed to probe what LLVM parameters `rustc` will accept. > > Signed-off-by: Matthew Maurer <mmaurer@google.com> I had some feedback on v2 -- was it missed? https://lore.kernel.org/rust-for-linux/CANiq72khUrha-a+59KYZgc63w-3P9=Dp_fs=+sgmV_A17q+PTA@mail.gmail.com/ Thanks! Cheers, Miguel
On Tue, Aug 20, 2024 at 7:20 AM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > I had some feedback on v2 -- was it missed? > > https://lore.kernel.org/rust-for-linux/CANiq72khUrha-a+59KYZgc63w-3P9=Dp_fs=+sgmV_A17q+PTA@mail.gmail.com/ Sorry, I did miss that in the refresh. To respond to a few points before I send up a replacement for this patch: >> >> 1. `rustc` support will soon be a minimum rather than a pinned version. > In the meantime, this happened, so we should update this sentence. Will update. >> 2. We already support multiple LLVMs linked into `rustc`, and these are > I guess you mean `rustc` is able to use multiple major versions of > LLVM -- or what do you mean by "multiple LLVMs linked"? I meant that the `rustc` consumed by the kernel build may use a wide range of different LLVMs, including unreleased ones. This means that which options are valid fundamentally needs to be probed - there's not necessarily a clean "LLVM version" for us to use. I'll rephrase. >> +# $(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 >> +# CONFIG_HAVE_RUST is not set. >Hmm... why `HAVE_RUST`? Should that be `RUST_IS_AVAILABLE`? Or what is t>he intention? Perhaps a comment would help here -- e.g. something >like the comment I used in the original approach [1]. Otherwise we >will forget... :) Yes, this should be RUST_IS_AVAILABLE, will update. >Also, I guess you wanted to relax the precondition as much as >possible, which is great, just to double check, do we expect a case >outside `RUST=y`? I expect this to be potentially used for whether you're *allowed* to set `RUST=y` - for example, if a particular sanitizer is enabled, you may need to probe whether Rust+LLVM supports that sanitizer before allowing RUST to be set to y. >> rustc-option = $(success,trap "rm -rf .tmp_$$" EXIT; mkdir .tmp_$$; $(RUSTC) $(1) --crate-type=rlib /dev/null -o .tmp_$$/tmp.rlib) >I also had `out-dir` [1] since, if I remember correctly, `rustc` may >create temporary files in a potentially read-only location even in >this case. OK, I will add that. >> Also, should we do `-Dwarnings` here? I don't think so - I can't think of a case where we'd want to error on a warning from an empty crate (though that may be a failure of imagination.) Do you have an example of a warning we might trip that we'd want to make the build reject an option's availability? > > Thanks! > > Cheers, > Miguel
On Tue, Aug 20, 2024 at 7:22 PM Matthew Maurer <mmaurer@google.com> wrote: > > Sorry, I did miss that in the refresh. To respond to a few points > before I send up a replacement for this patch: No problem at all -- thanks! > I expect this to be potentially used for whether you're *allowed* to > set `RUST=y` - for example, if a particular sanitizer is enabled, you > may need to probe whether Rust+LLVM supports that sanitizer before > allowing RUST to be set to y. Yeah, makes sense if we do the dependency that way. > I don't think so - I can't think of a case where we'd want to error on > a warning from an empty crate (though that may be a failure of > imagination.) Do you have an example of a warning we might trip that > we'd want to make the build reject an option's availability? IIRC back then I was thinking about something like the "unknown target feature forwarded to backend" one, i.e. to identify whether a target feature was supported or not. However, that is not a warning even under `-Dwarning`s (https://github.com/rust-lang/rust/issues/91262) unless something recently changed. We can add it if/when we need it. Cheers, Miguel
diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include index 3ee8ecfb8c04..ffafe269fe9e 100644 --- a/scripts/Kconfig.include +++ b/scripts/Kconfig.include @@ -63,3 +63,11 @@ ld-version := $(shell,set -- $(ld-info) && echo $2) cc-option-bit = $(if-success,$(CC) -Werror $(1) -E -x c /dev/null -o /dev/null,$(1)) m32-flag := $(cc-option-bit,-m32) m64-flag := $(cc-option-bit,-m64) + +# $(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 +# CONFIG_HAVE_RUST is not set. +# If you are testing for unstable features, consider `rustc-min-version` +# instead, as features may have different completeness while available. +rustc-option = $(success,trap "rm -rf .tmp_$$" EXIT; mkdir .tmp_$$; $(RUSTC) $(1) --crate-type=rlib /dev/null -o .tmp_$$/tmp.rlib) diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler index 92be0c9a13ee..485d66768a32 100644 --- a/scripts/Makefile.compiler +++ b/scripts/Makefile.compiler @@ -72,3 +72,18 @@ clang-min-version = $(call test-ge, $(CONFIG_CLANG_VERSION), $1) # ld-option # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y) ld-option = $(call try-run, $(LD) $(KBUILD_LDFLAGS) $(1) -v,$(1),$(2),$(3)) + +# __rustc-option +# Usage: MY_RUSTFLAGS += $(call __rustc-option,$(RUSTC),$(MY_RUSTFLAGS),-Cinstrument-coverage,-Zinstrument-coverage) +__rustc-option = $(call try-run,\ + $(1) $(2) $(3) --crate-type=rlib /dev/null -o "$$TMP",$(3),$(4)) + +# rustc-option +# Usage: rustflags-y += $(call rustc-option,-Cinstrument-coverage,-Zinstrument-coverage) +rustc-option = $(call __rustc-option, $(RUSTC),\ + $(KBUILD_RUSTFLAGS),$(1),$(2)) + +# rustc-option-yn +# Usage: flag := $(call rustc-option-yn,-Cinstrument-coverage) +rustc-option-yn = $(call try-run,\ + $(RUSTC) $(KBUILD_RUSTFLAGS) $(1) --crate-type=rlib /dev/null -o "$$TMP",y,n)
Creates flag probe macro variants for `rustc`. These are helpful because: 1. `rustc` support will soon be a minimum rather than a pinned version. 2. We already support multiple LLVMs linked into `rustc`, and these are needed to probe what LLVM parameters `rustc` will accept. Signed-off-by: Matthew Maurer <mmaurer@google.com> --- scripts/Kconfig.include | 8 ++++++++ scripts/Makefile.compiler | 15 +++++++++++++++ 2 files changed, 23 insertions(+)