* [PATCH v2 2/7] kbuild: specify output names separately for each emission type from rustc
2023-01-07 9:18 [PATCH v2 1/7] kbuild: refactor host*_flags Masahiro Yamada
@ 2023-01-07 9:18 ` Masahiro Yamada
2023-01-07 9:18 ` [PATCH v2 3/7] fixdep: parse Makefile more correctly to handle comments etc Masahiro Yamada
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Masahiro Yamada @ 2023-01-07 9:18 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Boqun Feng, Gary Guo, Björn Roy Baron, rust-for-linux,
Masahiro Yamada, Vincenzo Palazzo, Nathan Chancellor,
Nick Desaulniers, Nicolas Schier, Tom Rix, llvm
In Kbuild, two different rules must not write to the same file, but
it happens when compiling rust source files.
For example, set CONFIG_SAMPLE_RUST_MINIMAL=m and run the following:
$ make -j$(nproc) samples/rust/rust_minimal.o samples/rust/rust_minimal.rsi \
samples/rust/rust_minimal.s samples/rust/rust_minimal.ll
[snip]
RUSTC [M] samples/rust/rust_minimal.o
RUSTC [M] samples/rust/rust_minimal.rsi
RUSTC [M] samples/rust/rust_minimal.s
RUSTC [M] samples/rust/rust_minimal.ll
mv: cannot stat 'samples/rust/rust_minimal.d': No such file or directory
make[3]: *** [scripts/Makefile.build:334: samples/rust/rust_minimal.ll] Error 1
make[3]: *** Waiting for unfinished jobs....
mv: cannot stat 'samples/rust/rust_minimal.d': No such file or directory
make[3]: *** [scripts/Makefile.build:309: samples/rust/rust_minimal.o] Error 1
mv: cannot stat 'samples/rust/rust_minimal.d': No such file or directory
make[3]: *** [scripts/Makefile.build:326: samples/rust/rust_minimal.s] Error 1
make[2]: *** [scripts/Makefile.build:504: samples/rust] Error 2
make[1]: *** [scripts/Makefile.build:504: samples] Error 2
make: *** [Makefile:2008: .] Error 2
The reason for the error is that 4 threads running in parallel renames
the same file, samples/rust/rust_minimal.d.
This does not happen when compiling C or assembly files because
-Wp,-MMD,$(depfile) explicitly specifies the dependency filepath.
$(depfile) is a unique path for each target.
Currently, rustc is only given --out-dir and --emit=<list-of-types>
So, all the rust build rules output the dep-info into the default
<CRATE_NAME>.d, which causes the path conflict.
Fortunately, the --emit option is able to specify the output path
individually, with the form --emit=<type>=<path>.
Add --emit=dep-info=$(depfile) to the common part. Also, remove the
redundant --out-dir because the output path is specified for each type.
The code gets much cleaner because we do not need to rename *.d files.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Miguel Ojeda <ojeda@kernel.org>
Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
---
Changes in v2:
- Wrap a too long line
rust/Makefile | 11 +++++------
scripts/Makefile.build | 14 +++++++-------
scripts/Makefile.host | 6 ++----
3 files changed, 14 insertions(+), 17 deletions(-)
diff --git a/rust/Makefile b/rust/Makefile
index ff70c4c916f8..865afb87bc9b 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -331,10 +331,9 @@ $(obj)/exports_kernel_generated.h: $(obj)/kernel.o FORCE
quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
cmd_rustc_procmacro = \
$(RUSTC_OR_CLIPPY) $(rust_common_flags) \
- --emit=dep-info,link --extern proc_macro \
- --crate-type proc-macro --out-dir $(objtree)/$(obj) \
+ --emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
+ --crate-type proc-macro \
--crate-name $(patsubst lib%.so,%,$(notdir $@)) $<; \
- mv $(objtree)/$(obj)/$(patsubst lib%.so,%,$(notdir $@)).d $(depfile); \
sed -i '/^\#/d' $(depfile)
# Procedural macros can only be used with the `rustc` that compiled it.
@@ -348,10 +347,10 @@ quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L
OBJTREE=$(abspath $(objtree)) \
$(if $(skip_clippy),$(RUSTC),$(RUSTC_OR_CLIPPY)) \
$(filter-out $(skip_flags),$(rust_flags) $(rustc_target_flags)) \
- --emit=dep-info,obj,metadata --crate-type rlib \
- --out-dir $(objtree)/$(obj) -L$(objtree)/$(obj) \
+ --emit=dep-info=$(depfile) --emit=obj=$@ \
+ --emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \
+ --crate-type rlib -L$(objtree)/$(obj) \
--crate-name $(patsubst %.o,%,$(notdir $@)) $<; \
- mv $(objtree)/$(obj)/$(patsubst %.o,%,$(notdir $@)).d $(depfile); \
sed -i '/^\#/d' $(depfile) \
$(if $(rustc_objcopy),;$(OBJCOPY) $(rustc_objcopy) $@)
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index a0d5c6cca76d..40de20246e50 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -285,11 +285,11 @@ rust_common_cmd = \
-Zcrate-attr=no_std \
-Zcrate-attr='feature($(rust_allowed_features))' \
--extern alloc --extern kernel \
- --crate-type rlib --out-dir $(obj) -L $(objtree)/rust/ \
- --crate-name $(basename $(notdir $@))
+ --crate-type rlib -L $(objtree)/rust/ \
+ --crate-name $(basename $(notdir $@)) \
+ --emit=dep-info=$(depfile)
rust_handle_depfile = \
- mv $(obj)/$(basename $(notdir $@)).d $(depfile); \
sed -i '/^\#/d' $(depfile)
# `--emit=obj`, `--emit=asm` and `--emit=llvm-ir` imply a single codegen unit
@@ -302,7 +302,7 @@ rust_handle_depfile = \
quiet_cmd_rustc_o_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
cmd_rustc_o_rs = \
- $(rust_common_cmd) --emit=dep-info,obj $<; \
+ $(rust_common_cmd) --emit=obj=$@ $<; \
$(rust_handle_depfile)
$(obj)/%.o: $(src)/%.rs FORCE
@@ -310,7 +310,7 @@ $(obj)/%.o: $(src)/%.rs FORCE
quiet_cmd_rustc_rsi_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
cmd_rustc_rsi_rs = \
- $(rust_common_cmd) --emit=dep-info -Zunpretty=expanded $< >$@; \
+ $(rust_common_cmd) -Zunpretty=expanded $< >$@; \
command -v $(RUSTFMT) >/dev/null && $(RUSTFMT) $@; \
$(rust_handle_depfile)
@@ -319,7 +319,7 @@ $(obj)/%.rsi: $(src)/%.rs FORCE
quiet_cmd_rustc_s_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
cmd_rustc_s_rs = \
- $(rust_common_cmd) --emit=dep-info,asm $<; \
+ $(rust_common_cmd) --emit=asm=$@ $<; \
$(rust_handle_depfile)
$(obj)/%.s: $(src)/%.rs FORCE
@@ -327,7 +327,7 @@ $(obj)/%.s: $(src)/%.rs FORCE
quiet_cmd_rustc_ll_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
cmd_rustc_ll_rs = \
- $(rust_common_cmd) --emit=dep-info,llvm-ir $<; \
+ $(rust_common_cmd) --emit=llvm-ir=$@ $<; \
$(rust_handle_depfile)
$(obj)/%.ll: $(src)/%.rs FORCE
diff --git a/scripts/Makefile.host b/scripts/Makefile.host
index dea494ef55d5..67ef852712d4 100644
--- a/scripts/Makefile.host
+++ b/scripts/Makefile.host
@@ -85,7 +85,7 @@ hostc_flags = $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) \
hostcxx_flags = $(KBUILD_HOSTCXXFLAGS) $(HOST_EXTRACXXFLAGS) \
$(HOSTCXXFLAGS_$(target-stem).o) -Wp,-MMD,$(depfile)
hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
- $(HOSTRUSTFLAGS_$(target-stem))
+ $(HOSTRUSTFLAGS_$(target-stem)) --emit=dep-info=$(depfile)
# $(objtree)/$(obj) for including generated headers from checkin source files
ifeq ($(KBUILD_EXTMOD),)
@@ -145,9 +145,7 @@ $(host-cxxobjs): $(obj)/%.o: $(src)/%.cc FORCE
# host-rust -> Executable
quiet_cmd_host-rust = HOSTRUSTC $@
cmd_host-rust = \
- $(HOSTRUSTC) $(hostrust_flags) --emit=dep-info,link \
- --out-dir=$(obj)/ $<; \
- mv $(obj)/$(target-stem).d $(depfile); \
+ $(HOSTRUSTC) $(hostrust_flags) --emit=link=$@ $<; \
sed -i '/^\#/d' $(depfile)
$(host-rust): $(obj)/%: $(src)/%.rs FORCE
$(call if_changed_dep,host-rust)
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 3/7] fixdep: parse Makefile more correctly to handle comments etc.
2023-01-07 9:18 [PATCH v2 1/7] kbuild: refactor host*_flags Masahiro Yamada
2023-01-07 9:18 ` [PATCH v2 2/7] kbuild: specify output names separately for each emission type from rustc Masahiro Yamada
@ 2023-01-07 9:18 ` Masahiro Yamada
2023-01-07 9:18 ` [PATCH v2 4/7] kbuild: remove sed commands after rustc rules Masahiro Yamada
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Masahiro Yamada @ 2023-01-07 9:18 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Boqun Feng, Gary Guo, Björn Roy Baron, rust-for-linux,
Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
Nicolas Schier, Tom Rix, llvm
fixdep parses dependency files (*.d) emitted by the compiler.
*.d files are Makefiles describing the dependencies of the main source
file.
fixdep understands minimal Makefile syntax. It works well enough for
GCC and Clang, but not for rustc.
This commit improves the parser a little more for better processing
comments, escape sequences, etc.
My main motivation is to drop comments. rustc may output comments
(e.g. env-dep). Currentyly, rustc build rules invoke sed to remove
comments, but it is more efficient to do it in fixdep.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Acked-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Miguel Ojeda <ojeda@kernel.org>
---
Changes in v2:
- Rename searching_colon -> is_target
- More comments
scripts/basic/fixdep.c | 173 ++++++++++++++++++++++++++++-------------
1 file changed, 119 insertions(+), 54 deletions(-)
diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c
index 37782a632494..f5a51770eb74 100644
--- a/scripts/basic/fixdep.c
+++ b/scripts/basic/fixdep.c
@@ -94,6 +94,7 @@
#include <unistd.h>
#include <fcntl.h>
#include <string.h>
+#include <stdbool.h>
#include <stdlib.h>
#include <stdio.h>
#include <ctype.h>
@@ -251,75 +252,139 @@ static int is_ignored_file(const char *s, int len)
* assignments are parsed not only by make, but also by the rather simple
* parser in scripts/mod/sumversion.c.
*/
-static void parse_dep_file(char *m, const char *target)
+static void parse_dep_file(char *p, const char *target)
{
- char *p;
- int is_last, is_target;
- int saw_any_target = 0;
- int is_first_dep = 0;
- void *buf;
-
- while (1) {
- /* Skip any "white space" */
- while (*m == ' ' || *m == '\\' || *m == '\n')
- m++;
-
- if (!*m)
+ bool saw_any_target = false;
+ bool is_target = true;
+ bool is_source = false;
+ bool need_parse;
+ char *q, saved_c;
+
+ while (*p) {
+ /* handle some special characters first. */
+ switch (*p) {
+ case '#':
+ /*
+ * skip comments.
+ * rustc may emit comments to dep-info.
+ */
+ p++;
+ while (*p != '\0' && *p != '\n') {
+ /*
+ * escaped newlines continue the comment across
+ * multiple lines.
+ */
+ if (*p == '\\')
+ p++;
+ p++;
+ }
+ continue;
+ case ' ':
+ case '\t':
+ /* skip whitespaces */
+ p++;
+ continue;
+ case '\\':
+ /*
+ * backslash/newline combinations continue the
+ * statement. Skip it just like a whitespace.
+ */
+ if (*(p + 1) == '\n') {
+ p += 2;
+ continue;
+ }
break;
-
- /* Find next "white space" */
- p = m;
- while (*p && *p != ' ' && *p != '\\' && *p != '\n')
+ case '\n':
+ /*
+ * Makefiles use a line-based syntax, where the newline
+ * is the end of a statement. After seeing a newline,
+ * we expect the next token is a target.
+ */
p++;
- is_last = (*p == '\0');
- /* Is the token we found a target name? */
- is_target = (*(p-1) == ':');
- /* Don't write any target names into the dependency file */
- if (is_target) {
- /* The /next/ file is the first dependency */
- is_first_dep = 1;
- } else if (!is_ignored_file(m, p - m)) {
- *p = '\0';
-
+ is_target = true;
+ continue;
+ case ':':
/*
- * Do not list the source file as dependency, so that
- * kbuild is not confused if a .c file is rewritten
- * into .S or vice versa. Storing it in source_* is
- * needed for modpost to compute srcversions.
+ * assume the first dependency after a colon as the
+ * source file.
*/
- if (is_first_dep) {
+ p++;
+ is_target = false;
+ is_source = true;
+ continue;
+ }
+
+ /* find the end of the token */
+ q = p;
+ while (*q != ' ' && *q != '\t' && *q != '\n' && *q != '#' && *q != ':') {
+ if (*q == '\\') {
/*
- * If processing the concatenation of multiple
- * dependency files, only process the first
- * target name, which will be the original
- * source name, and ignore any other target
- * names, which will be intermediate temporary
- * files.
+ * backslash/newline combinations work like as
+ * a whitespace, so this is the end of token.
*/
- if (!saw_any_target) {
- saw_any_target = 1;
- printf("source_%s := %s\n\n",
- target, m);
- printf("deps_%s := \\\n", target);
+ if (*(q + 1) == '\n')
+ break;
+
+ /* escaped special characters */
+ if (*(q + 1) == '#' || *(q + 1) == ':') {
+ memmove(p + 1, p, q - p);
+ p++;
}
- is_first_dep = 0;
- } else {
- printf(" %s \\\n", m);
+
+ q++;
}
- buf = read_file(m);
- parse_config_file(buf);
- free(buf);
+ if (*q == '\0')
+ break;
+ q++;
}
- if (is_last)
- break;
+ /* Just discard the target */
+ if (is_target) {
+ p = q;
+ continue;
+ }
+
+ saved_c = *q;
+ *q = '\0';
+ need_parse = false;
/*
- * Start searching for next token immediately after the first
- * "whitespace" character that follows this token.
+ * Do not list the source file as dependency, so that kbuild is
+ * not confused if a .c file is rewritten into .S or vice versa.
+ * Storing it in source_* is needed for modpost to compute
+ * srcversions.
*/
- m = p + 1;
+ if (is_source) {
+ /*
+ * The DT build rule concatenates multiple dep files.
+ * When processing them, only process the first source
+ * name, which will be the original one, and ignore any
+ * other source names, which will be intermediate
+ * temporary files.
+ */
+ if (!saw_any_target) {
+ saw_any_target = true;
+ printf("source_%s := %s\n\n", target, p);
+ printf("deps_%s := \\\n", target);
+ need_parse = true;
+ }
+ } else if (!is_ignored_file(p, q - p)) {
+ printf(" %s \\\n", p);
+ need_parse = true;
+ }
+
+ if (need_parse) {
+ void *buf;
+
+ buf = read_file(p);
+ parse_config_file(buf);
+ free(buf);
+ }
+
+ is_source = false;
+ *q = saved_c;
+ p = q;
}
if (!saw_any_target) {
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 4/7] kbuild: remove sed commands after rustc rules
2023-01-07 9:18 [PATCH v2 1/7] kbuild: refactor host*_flags Masahiro Yamada
2023-01-07 9:18 ` [PATCH v2 2/7] kbuild: specify output names separately for each emission type from rustc Masahiro Yamada
2023-01-07 9:18 ` [PATCH v2 3/7] fixdep: parse Makefile more correctly to handle comments etc Masahiro Yamada
@ 2023-01-07 9:18 ` Masahiro Yamada
2023-01-07 9:18 ` [PATCH v2 5/7] fixdep: refactor hash table lookup Masahiro Yamada
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Masahiro Yamada @ 2023-01-07 9:18 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Boqun Feng, Gary Guo, Björn Roy Baron, rust-for-linux,
Masahiro Yamada, Vincenzo Palazzo, Nathan Chancellor,
Nick Desaulniers, Nicolas Schier, Tom Rix, llvm
rustc may put comments in dep-info, so sed is used to drop them before
passing it to fixdep.
Now that fixdep can remove comments, Makefiles do not need to run sed.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Miguel Ojeda <ojeda@kernel.org>
Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
---
(no changes since v1)
rust/Makefile | 6 ++----
scripts/Makefile.build | 18 ++++--------------
scripts/Makefile.host | 3 +--
3 files changed, 7 insertions(+), 20 deletions(-)
diff --git a/rust/Makefile b/rust/Makefile
index 865afb87bc9b..f403b79cae5a 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -333,8 +333,7 @@ quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
$(RUSTC_OR_CLIPPY) $(rust_common_flags) \
--emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
--crate-type proc-macro \
- --crate-name $(patsubst lib%.so,%,$(notdir $@)) $<; \
- sed -i '/^\#/d' $(depfile)
+ --crate-name $(patsubst lib%.so,%,$(notdir $@)) $<
# Procedural macros can only be used with the `rustc` that compiled it.
# Therefore, to get `libmacros.so` automatically recompiled when the compiler
@@ -350,8 +349,7 @@ quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L
--emit=dep-info=$(depfile) --emit=obj=$@ \
--emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \
--crate-type rlib -L$(objtree)/$(obj) \
- --crate-name $(patsubst %.o,%,$(notdir $@)) $<; \
- sed -i '/^\#/d' $(depfile) \
+ --crate-name $(patsubst %.o,%,$(notdir $@)) $< \
$(if $(rustc_objcopy),;$(OBJCOPY) $(rustc_objcopy) $@)
rust-analyzer:
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 40de20246e50..76323201232a 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -289,9 +289,6 @@ rust_common_cmd = \
--crate-name $(basename $(notdir $@)) \
--emit=dep-info=$(depfile)
-rust_handle_depfile = \
- sed -i '/^\#/d' $(depfile)
-
# `--emit=obj`, `--emit=asm` and `--emit=llvm-ir` imply a single codegen unit
# will be used. We explicitly request `-Ccodegen-units=1` in any case, and
# the compiler shows a warning if it is not 1. However, if we ever stop
@@ -301,9 +298,7 @@ rust_handle_depfile = \
# would not match each other.
quiet_cmd_rustc_o_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
- cmd_rustc_o_rs = \
- $(rust_common_cmd) --emit=obj=$@ $<; \
- $(rust_handle_depfile)
+ cmd_rustc_o_rs = $(rust_common_cmd) --emit=obj=$@ $<
$(obj)/%.o: $(src)/%.rs FORCE
$(call if_changed_dep,rustc_o_rs)
@@ -311,24 +306,19 @@ $(obj)/%.o: $(src)/%.rs FORCE
quiet_cmd_rustc_rsi_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
cmd_rustc_rsi_rs = \
$(rust_common_cmd) -Zunpretty=expanded $< >$@; \
- command -v $(RUSTFMT) >/dev/null && $(RUSTFMT) $@; \
- $(rust_handle_depfile)
+ command -v $(RUSTFMT) >/dev/null && $(RUSTFMT) $@
$(obj)/%.rsi: $(src)/%.rs FORCE
$(call if_changed_dep,rustc_rsi_rs)
quiet_cmd_rustc_s_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
- cmd_rustc_s_rs = \
- $(rust_common_cmd) --emit=asm=$@ $<; \
- $(rust_handle_depfile)
+ cmd_rustc_s_rs = $(rust_common_cmd) --emit=asm=$@ $<
$(obj)/%.s: $(src)/%.rs FORCE
$(call if_changed_dep,rustc_s_rs)
quiet_cmd_rustc_ll_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
- cmd_rustc_ll_rs = \
- $(rust_common_cmd) --emit=llvm-ir=$@ $<; \
- $(rust_handle_depfile)
+ cmd_rustc_ll_rs = $(rust_common_cmd) --emit=llvm-ir=$@ $<
$(obj)/%.ll: $(src)/%.rs FORCE
$(call if_changed_dep,rustc_ll_rs)
diff --git a/scripts/Makefile.host b/scripts/Makefile.host
index 67ef852712d4..a45a97b027d1 100644
--- a/scripts/Makefile.host
+++ b/scripts/Makefile.host
@@ -145,8 +145,7 @@ $(host-cxxobjs): $(obj)/%.o: $(src)/%.cc FORCE
# host-rust -> Executable
quiet_cmd_host-rust = HOSTRUSTC $@
cmd_host-rust = \
- $(HOSTRUSTC) $(hostrust_flags) --emit=link=$@ $<; \
- sed -i '/^\#/d' $(depfile)
+ $(HOSTRUSTC) $(hostrust_flags) --emit=link=$@ $<
$(host-rust): $(obj)/%: $(src)/%.rs FORCE
$(call if_changed_dep,host-rust)
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 5/7] fixdep: refactor hash table lookup
2023-01-07 9:18 [PATCH v2 1/7] kbuild: refactor host*_flags Masahiro Yamada
` (2 preceding siblings ...)
2023-01-07 9:18 ` [PATCH v2 4/7] kbuild: remove sed commands after rustc rules Masahiro Yamada
@ 2023-01-07 9:18 ` Masahiro Yamada
2023-01-07 9:18 ` [PATCH v2 6/7] fixdep: avoid parsing the same file over again Masahiro Yamada
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Masahiro Yamada @ 2023-01-07 9:18 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Boqun Feng, Gary Guo, Björn Roy Baron, rust-for-linux,
Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
Nicolas Schier
Change the hash table code so it will be easier to add the second table.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Miguel Ojeda <ojeda@kernel.org>
---
(no changes since v1)
scripts/basic/fixdep.c | 47 ++++++++++++++++++++++--------------------
1 file changed, 25 insertions(+), 22 deletions(-)
diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c
index f5a51770eb74..74f90a0deeb9 100644
--- a/scripts/basic/fixdep.c
+++ b/scripts/basic/fixdep.c
@@ -113,7 +113,7 @@ struct item {
};
#define HASHSZ 256
-static struct item *hashtab[HASHSZ];
+static struct item *config_hashtab[HASHSZ];
static unsigned int strhash(const char *str, unsigned int sz)
{
@@ -125,25 +125,11 @@ static unsigned int strhash(const char *str, unsigned int sz)
return hash;
}
-/*
- * Lookup a value in the configuration string.
- */
-static int is_defined_config(const char *name, int len, unsigned int hash)
-{
- struct item *aux;
-
- for (aux = hashtab[hash % HASHSZ]; aux; aux = aux->next) {
- if (aux->hash == hash && aux->len == len &&
- memcmp(aux->name, name, len) == 0)
- return 1;
- }
- return 0;
-}
-
/*
* Add a new value to the configuration string.
*/
-static void define_config(const char *name, int len, unsigned int hash)
+static void add_to_hashtable(const char *name, int len, unsigned int hash,
+ struct item *hashtab[])
{
struct item *aux = malloc(sizeof(*aux) + len);
@@ -158,17 +144,34 @@ static void define_config(const char *name, int len, unsigned int hash)
hashtab[hash % HASHSZ] = aux;
}
+/*
+ * Lookup a string in the hash table. If found, just return true.
+ * If not, add it to the hashtable and return false.
+ */
+static bool in_hashtable(const char *name, int len, struct item *hashtab[])
+{
+ struct item *aux;
+ unsigned int hash = strhash(name, len);
+
+ for (aux = hashtab[hash % HASHSZ]; aux; aux = aux->next) {
+ if (aux->hash == hash && aux->len == len &&
+ memcmp(aux->name, name, len) == 0)
+ return true;
+ }
+
+ add_to_hashtable(name, len, hash, hashtab);
+
+ return false;
+}
+
/*
* Record the use of a CONFIG_* word.
*/
static void use_config(const char *m, int slen)
{
- unsigned int hash = strhash(m, slen);
-
- if (is_defined_config(m, slen, hash))
- return;
+ if (in_hashtable(m, slen, config_hashtab))
+ return;
- define_config(m, slen, hash);
/* Print out a dependency path from a symbol name. */
printf(" $(wildcard include/config/%.*s) \\\n", slen, m);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 6/7] fixdep: avoid parsing the same file over again
2023-01-07 9:18 [PATCH v2 1/7] kbuild: refactor host*_flags Masahiro Yamada
` (3 preceding siblings ...)
2023-01-07 9:18 ` [PATCH v2 5/7] fixdep: refactor hash table lookup Masahiro Yamada
@ 2023-01-07 9:18 ` Masahiro Yamada
2023-01-07 9:18 ` [PATCH v2 7/7] fixdep: do not parse *.rlib, *.rmeta, *.so Masahiro Yamada
2023-01-07 15:12 ` [PATCH v2 1/7] kbuild: refactor host*_flags Miguel Ojeda
6 siblings, 0 replies; 10+ messages in thread
From: Masahiro Yamada @ 2023-01-07 9:18 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Boqun Feng, Gary Guo, Björn Roy Baron, rust-for-linux,
Masahiro Yamada, Vincenzo Palazzo, Nathan Chancellor,
Nick Desaulniers, Nicolas Schier
The dep files (*.d files) emitted by C compilers usually contain the
deduplicated list of included files.
One exceptional case is when a header is included by the -include
command line option, and also by #include directive.
For example, the top Makefile adds the command line option,
"-include $(srctree)/include/linux/kconfig.h". You do not need to
include <linux/kconfig.h> in every source file.
In fact, include/linux/kconfig.h is listed twice in many .*.cmd files
due to include/linux/xarray.h having "#include <linux/kconfig.h>".
I did not fix that since it is a small redundancy.
However, this is more annoying for rustc. rustc emits the dependency
for each emission type.
For example, cmd_rustc_library emits dep-info, obj, and metadata.
So, the emitted *.d file contains the dependency for those 3 targets,
which makes fixdep parse the same file 3 times.
$ grep rust/alloc/raw_vec.rs rust/.alloc.o.cmd
rust/alloc/raw_vec.rs \
rust/alloc/raw_vec.rs \
rust/alloc/raw_vec.rs \
To skip the second parsing, this commit adds a hash table for parsed
files, just like we did for CONFIG options.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Acked-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Miguel Ojeda <ojeda@kernel.org>
Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
---
(no changes since v1)
scripts/basic/fixdep.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c
index 74f90a0deeb9..e22e689de61e 100644
--- a/scripts/basic/fixdep.c
+++ b/scripts/basic/fixdep.c
@@ -113,7 +113,7 @@ struct item {
};
#define HASHSZ 256
-static struct item *config_hashtab[HASHSZ];
+static struct item *config_hashtab[HASHSZ], *file_hashtab[HASHSZ];
static unsigned int strhash(const char *str, unsigned int sz)
{
@@ -365,6 +365,10 @@ static void parse_dep_file(char *p, const char *target)
* name, which will be the original one, and ignore any
* other source names, which will be intermediate
* temporary files.
+ *
+ * rustc emits the same dependency list for each
+ * emission type. It is enough to list the source name
+ * just once.
*/
if (!saw_any_target) {
saw_any_target = true;
@@ -372,7 +376,8 @@ static void parse_dep_file(char *p, const char *target)
printf("deps_%s := \\\n", target);
need_parse = true;
}
- } else if (!is_ignored_file(p, q - p)) {
+ } else if (!is_ignored_file(p, q - p) &&
+ !in_hashtable(p, q - p, file_hashtab)) {
printf(" %s \\\n", p);
need_parse = true;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 7/7] fixdep: do not parse *.rlib, *.rmeta, *.so
2023-01-07 9:18 [PATCH v2 1/7] kbuild: refactor host*_flags Masahiro Yamada
` (4 preceding siblings ...)
2023-01-07 9:18 ` [PATCH v2 6/7] fixdep: avoid parsing the same file over again Masahiro Yamada
@ 2023-01-07 9:18 ` Masahiro Yamada
2023-01-07 15:12 ` [PATCH v2 1/7] kbuild: refactor host*_flags Miguel Ojeda
6 siblings, 0 replies; 10+ messages in thread
From: Masahiro Yamada @ 2023-01-07 9:18 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Boqun Feng, Gary Guo, Björn Roy Baron, rust-for-linux,
Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
Nicolas Schier
fixdep is designed only for parsing text files. read_file() appends
a terminating null byte ('\0') and parse_config_file() calls strstr()
to search for CONFIG options.
rustc outputs *.rlib, *.rmeta, *.so to dep-info. fixdep needs them in
the dependency, but there is no point in parsing such binary files.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Miguel Ojeda <ojeda@kernel.org>
---
(no changes since v1)
scripts/basic/fixdep.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c
index e22e689de61e..3a61b037d5ba 100644
--- a/scripts/basic/fixdep.c
+++ b/scripts/basic/fixdep.c
@@ -250,6 +250,15 @@ static int is_ignored_file(const char *s, int len)
str_ends_with(s, len, "include/generated/autoksyms.h");
}
+/* Do not parse these files */
+static int is_no_parse_file(const char *s, int len)
+{
+ /* rustc may output binary files into dep-info */
+ return str_ends_with(s, len, ".rlib") ||
+ str_ends_with(s, len, ".rmeta") ||
+ str_ends_with(s, len, ".so");
+}
+
/*
* Important: The below generated source_foo.o and deps_foo.o variable
* assignments are parsed not only by make, but also by the rather simple
@@ -382,7 +391,7 @@ static void parse_dep_file(char *p, const char *target)
need_parse = true;
}
- if (need_parse) {
+ if (need_parse && !is_no_parse_file(p, q - p)) {
void *buf;
buf = read_file(p);
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2 1/7] kbuild: refactor host*_flags
2023-01-07 9:18 [PATCH v2 1/7] kbuild: refactor host*_flags Masahiro Yamada
` (5 preceding siblings ...)
2023-01-07 9:18 ` [PATCH v2 7/7] fixdep: do not parse *.rlib, *.rmeta, *.so Masahiro Yamada
@ 2023-01-07 15:12 ` Miguel Ojeda
2023-01-08 13:06 ` Masahiro Yamada
6 siblings, 1 reply; 10+ messages in thread
From: Miguel Ojeda @ 2023-01-07 15:12 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, linux-kernel, Miguel Ojeda, Alex Gaynor,
Wedson Almeida Filho, Boqun Feng, Gary Guo, Björn Roy Baron,
rust-for-linux, Nathan Chancellor, Nick Desaulniers,
Nicolas Schier
On Sat, Jan 7, 2023 at 10:18 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Remove _host*_flags.
>
> This will change the -Wp,-MMD,$(depfile) order in the command line
> but no functional change is intended.
Looks fine to me. I gave it a quick test with just this patch on top
of v6.2-rc2 and checked that the order indeed changed (was there a
reason to not keep it the same?).
Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Miguel Ojeda <ojeda@kernel.org>
Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 1/7] kbuild: refactor host*_flags
2023-01-07 15:12 ` [PATCH v2 1/7] kbuild: refactor host*_flags Miguel Ojeda
@ 2023-01-08 13:06 ` Masahiro Yamada
2023-01-08 14:05 ` Miguel Ojeda
0 siblings, 1 reply; 10+ messages in thread
From: Masahiro Yamada @ 2023-01-08 13:06 UTC (permalink / raw)
To: Miguel Ojeda
Cc: linux-kbuild, linux-kernel, Miguel Ojeda, Alex Gaynor,
Wedson Almeida Filho, Boqun Feng, Gary Guo, Björn Roy Baron,
rust-for-linux, Nathan Chancellor, Nick Desaulniers,
Nicolas Schier
On Sun, Jan 8, 2023 at 12:12 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sat, Jan 7, 2023 at 10:18 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > Remove _host*_flags.
> >
> > This will change the -Wp,-MMD,$(depfile) order in the command line
> > but no functional change is intended.
>
> Looks fine to me. I gave it a quick test with just this patch on top
> of v6.2-rc2 and checked that the order indeed changed (was there a
> reason to not keep it the same?).
No, there is no reason. Just my whimsy.
I will do something like the following just in case.
hostc_flags = -Wp,-MMD,$(depfile) \
$(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) \
$(HOSTCFLAGS_$(target-stem).o)
>
> Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
> Tested-by: Miguel Ojeda <ojeda@kernel.org>
>
> Thanks!
>
> Cheers,
> Miguel
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 1/7] kbuild: refactor host*_flags
2023-01-08 13:06 ` Masahiro Yamada
@ 2023-01-08 14:05 ` Miguel Ojeda
0 siblings, 0 replies; 10+ messages in thread
From: Miguel Ojeda @ 2023-01-08 14:05 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, linux-kernel, Miguel Ojeda, Alex Gaynor,
Wedson Almeida Filho, Boqun Feng, Gary Guo, Björn Roy Baron,
rust-for-linux, Nathan Chancellor, Nick Desaulniers,
Nicolas Schier
On Sun, Jan 8, 2023 at 2:06 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> No, there is no reason. Just my whimsy.
>
> I will do something like the following just in case.
>
> hostc_flags = -Wp,-MMD,$(depfile) \
> $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) \
> $(HOSTCFLAGS_$(target-stem).o)
Yeah, it sounds simpler to keep it the same, we could always do the
order change in an independent patch if needed.
Cheers,
Miguel
^ permalink raw reply [flat|nested] 10+ messages in thread