[PATCH 0/6] kbuild: fix dep-file processing for rust

[PATCH 0/6] kbuild: fix dep-file processing for rust

[Masahiro Yamada]

Masahiro Yamada (6):
  kbuild: specify output names separately for each emission type from
    rustc
  fixdep: parse Makefile more correctly to handle comments etc.
  kbuild: remove sed commands after rustc rules
  fixdep: refactor hash table lookup
  fixdep: avoid parsing the same file over again
  fixdep: do not parse *.so, *.rmeta, *.rlib

 rust/Makefile          |  16 ++-
 scripts/Makefile.build |  26 ++---
 scripts/Makefile.host  |  10 +-
 scripts/basic/fixdep.c | 234 +++++++++++++++++++++++++++--------------
 4 files changed, 173 insertions(+), 113 deletions(-)

-- 
2.34.1

[PATCH 1/6] kbuild: specify output names separately for each emission type from rustc

[Masahiro Yamada]

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 creates
and renames the same file path, samples/rust/rust_minimal.d.

This does not happen when compiling C or assembly files because we
explicitly specify the dependency filename by using the preprocessor
option, -Wp,-MMD,$(depfile). $(depfile) is a unique path for each target.

Currently, rustc is only given --out-dir and the list of emitted types.
So, all the rust build rules output the dep-info into the default
<CRATE_NAME>.d, causing the 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 command part. Also, remove
the redundant --out-dir because we specify the output path 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>
---

 rust/Makefile          | 10 ++++------
 scripts/Makefile.build | 14 +++++++-------
 scripts/Makefile.host  |  9 +++------
 3 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/rust/Makefile b/rust/Makefile
index ff70c4c916f8..0e2a32f4b3e9 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,9 @@ 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 da133780b751..4434cdbf7b8e 100644
--- a/scripts/Makefile.host
+++ b/scripts/Makefile.host
@@ -84,8 +84,8 @@ _hostc_flags   = $(KBUILD_HOSTCFLAGS)   $(HOST_EXTRACFLAGS)   \
                  $(HOSTCFLAGS_$(target-stem).o)
 _hostcxx_flags = $(KBUILD_HOSTCXXFLAGS) $(HOST_EXTRACXXFLAGS) \
                  $(HOSTCXXFLAGS_$(target-stem).o)
-_hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
-                  $(HOSTRUSTFLAGS_$(target-stem))
+hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
+                  $(HOSTRUSTFLAGS_$(target-stem)) --emit=dep-info=$(depfile)
 
 # $(objtree)/$(obj) for including generated headers from checkin source files
 ifeq ($(KBUILD_EXTMOD),)
@@ -97,7 +97,6 @@ endif
 
 hostc_flags    = -Wp,-MMD,$(depfile) $(_hostc_flags)
 hostcxx_flags  = -Wp,-MMD,$(depfile) $(_hostcxx_flags)
-hostrust_flags = $(_hostrust_flags)
 
 #####
 # Compile programs on the host
@@ -149,9 +148,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

[PATCH 2/6] fixdep: parse Makefile more correctly to handle comments etc.

[Masahiro Yamada]

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>
---

 scripts/basic/fixdep.c | 173 ++++++++++++++++++++++++++++-------------
 1 file changed, 117 insertions(+), 56 deletions(-)

diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c
index 37782a632494..37a40520686f 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,135 @@ 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)
-			break;
-
-		/* Find next "white space" */
-		p = m;
-		while (*p && *p != ' ' && *p != '\\' && *p != '\n')
-			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';
-
+	bool saw_any_target = false;
+	bool is_source = false;
+	bool searching_colon = true;
+	bool need_parse;
+	char *q, saved_c;
+
+	while (*p) {
+		/* handle some special characters first. */
+		switch (*p) {
+		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.
+			 * skip comments.
+			 * rustc may emit comments to dep-info.
 			 */
-			if (is_first_dep) {
+			p++;
+			while (*p != '\0' && *p != '\n') {
 				/*
-				 * 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.
+				 * escaped newlines continue the comment across
+				 * multiple lines.
 				 */
-				if (!saw_any_target) {
-					saw_any_target = 1;
-					printf("source_%s := %s\n\n",
-					       target, m);
-					printf("deps_%s := \\\n", target);
+				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;
+		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++;
+			searching_colon = true;
+			continue;
+		case ':':
+			/*
+			 * assume the first dependency after a colon as the
+			 * source file.
+			 */
+			p++;
+			searching_colon = false;
+			is_source = true;
+			continue;
+		}
+
+		/* find the end of the token */
+		q = p;
+		while (*q != ' ' && *q != '\t' && *q != '\n' && *q != '#' && *q != ':') {
+			if (*q == '\\') {
+				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 (searching_colon) {
+			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

[PATCH 3/6] kbuild: remove sed commands after rustc rules

[Masahiro Yamada]

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>
---

 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 0e2a32f4b3e9..c8941fec6955 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
@@ -349,8 +348,7 @@ quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L
 		$(filter-out $(skip_flags),$(rust_flags) $(rustc_target_flags)) \
 		--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 4434cdbf7b8e..bc782655d09e 100644
--- a/scripts/Makefile.host
+++ b/scripts/Makefile.host
@@ -148,8 +148,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

[PATCH 4/6] fixdep: refactor hash table lookup

[Masahiro Yamada]

Change the hash table code so it will be easier to add the second table.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 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 37a40520686f..b20777b888d7 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

[PATCH 5/6] fixdep: avoid parsing the same file over again

[Masahiro Yamada]

The dep files (*.d files) emitted by C compilers usually contain the
deduplicated list of included files.

There is an exceptional case; if a header is included by the -include
command line option, and also by #include directive, it appears twice
in the *.d file.

For example, the top Makefile specifies the command line option,
-include $(srctree)/include/linux/kconfig.h. You do not need to
add #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 including <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>
---

 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 b20777b888d7..cc8f6d34c2ca 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)
 {
@@ -361,6 +361,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;
@@ -368,7 +372,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

[PATCH 6/6] fixdep: do not parse *.so, *.rmeta, *.rlib

[Masahiro Yamada]

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 *.so, *.rmeta, *rlib to dep-info. fixdep needs them in
the dependency, but there is no point to parse such binary files.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 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 cc8f6d34c2ca..b70885116ed2 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
@@ -378,7 +387,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

Re: [PATCH 0/6] kbuild: fix dep-file processing for rust

[Miguel Ojeda]

On Sat, Dec 31, 2022 at 7:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Masahiro Yamada (6):
>   kbuild: specify output names separately for each emission type from
>     rustc
>   fixdep: parse Makefile more correctly to handle comments etc.
>   kbuild: remove sed commands after rustc rules
>   fixdep: refactor hash table lookup
>   fixdep: avoid parsing the same file over again
>   fixdep: do not parse *.so, *.rmeta, *.rlib

These cleanups are great, and it is a pleasure to see proper
integration with `fixdep` -- thanks a ton! :)

Will you want to take them through the kbuild tree? (I guess so, given
the bulk of it is on `fixdep`)

Cheers,
Miguel

Re: [PATCH 0/6] kbuild: fix dep-file processing for rust

[Masahiro Yamada]

On Sat, Dec 31, 2022 at 10:34 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sat, Dec 31, 2022 at 7:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > Masahiro Yamada (6):
> >   kbuild: specify output names separately for each emission type from
> >     rustc
> >   fixdep: parse Makefile more correctly to handle comments etc.
> >   kbuild: remove sed commands after rustc rules
> >   fixdep: refactor hash table lookup
> >   fixdep: avoid parsing the same file over again
> >   fixdep: do not parse *.so, *.rmeta, *.rlib
>
> These cleanups are great, and it is a pleasure to see proper
> integration with `fixdep` -- thanks a ton! :)
>
> Will you want to take them through the kbuild tree? (I guess so, given
> the bulk of it is on `fixdep`)


Yes, my plan is to get it in the kbuild tree with your ack.




> Cheers,
> Miguel



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/6] fixdep: parse Makefile more correctly to handle comments etc.

[Miguel Ojeda]

On Sat, Dec 31, 2022 at 7:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> 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.

Hmm... I couldn't apply this one, it turns out `#include <stdarg.h>`
is gone. Adding it and adjusting the patch fixes it.

Cheers,
Miguel

Re: [PATCH 2/6] fixdep: parse Makefile more correctly to handle comments etc.

[Miguel Ojeda]

On Sat, Dec 31, 2022 at 4:25 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> Hmm... I couldn't apply this one, it turns out `#include <stdarg.h>`
> is gone. Adding it and adjusting the patch fixes it.

Ah, I think you created them on top of kbuild/fixes.

Cheers,
Miguel

Re: [PATCH 1/6] kbuild: specify output names separately for each emission type from rustc

[Gary Guo]

On Sat, 31 Dec 2022 15:41:58 +0900
Masahiro Yamada <masahiroy@kernel.org> wrote:

> diff --git a/scripts/Makefile.host b/scripts/Makefile.host
> index da133780b751..4434cdbf7b8e 100644
> --- a/scripts/Makefile.host
> +++ b/scripts/Makefile.host
> @@ -84,8 +84,8 @@ _hostc_flags   = $(KBUILD_HOSTCFLAGS)   $(HOST_EXTRACFLAGS)   \
>                   $(HOSTCFLAGS_$(target-stem).o)
>  _hostcxx_flags = $(KBUILD_HOSTCXXFLAGS) $(HOST_EXTRACXXFLAGS) \
>                   $(HOSTCXXFLAGS_$(target-stem).o)
> -_hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
> -                  $(HOSTRUSTFLAGS_$(target-stem))
> +hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
> +                  $(HOSTRUSTFLAGS_$(target-stem)) --emit=dep-info=$(depfile)
>  
>  # $(objtree)/$(obj) for including generated headers from checkin source files
>  ifeq ($(KBUILD_EXTMOD),)
> @@ -97,7 +97,6 @@ endif
>  
>  hostc_flags    = -Wp,-MMD,$(depfile) $(_hostc_flags)
>  hostcxx_flags  = -Wp,-MMD,$(depfile) $(_hostcxx_flags)
> -hostrust_flags = $(_hostrust_flags)

Would it be better to have `--emit=dep-info=$(depfile)` added here
instead so that it mimics c/cxx flags?

>  
>  #####
>  # Compile programs on the host
> @@ -149,9 +148,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)

Best,
Gary

Re: [PATCH 1/6] kbuild: specify output names separately for each emission type from rustc

[Miguel Ojeda]

On Sat, Dec 31, 2022 at 7:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
>   $ make -j$(nproc) samples/rust/rust_minimal.o samples/rust/rust_minimal.rsi \
>                     samples/rust/rust_minimal.s samples/rust/rust_minimal.ll

Yeah, we were testing the single targets, but not multiple at once, thanks!

> +               --emit=dep-info=$(depfile) --emit=obj=$@ --emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \

Perhaps a newline here to avoid the lengthy line?

>  hostc_flags    = -Wp,-MMD,$(depfile) $(_hostc_flags)
>  hostcxx_flags  = -Wp,-MMD,$(depfile) $(_hostcxx_flags)
> -hostrust_flags = $(_hostrust_flags)

This was originally meant to be consistent with C and C++ indeed, but
if you prefer less variables, I guess it is fine, in which case,
should we update the C/C++ side too (in another series)?

Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel

Re: [PATCH 2/6] fixdep: parse Makefile more correctly to handle comments etc.

[Miguel Ojeda]

On Sat, Dec 31, 2022 at 7:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> This commit improves the parser a little more for better processing
> comments, escape sequences, etc.

Acked-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel

Re: [PATCH 3/6] kbuild: remove sed commands after rustc rules

[Miguel Ojeda]

On Sat, Dec 31, 2022 at 7:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> 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.

Finally!

Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel

Re: [PATCH 4/6] fixdep: refactor hash table lookup

[Miguel Ojeda]

On Sat, Dec 31, 2022 at 7:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> +/*
> + * 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[])

I think readers may find a bit surprising that a function named
`in_hashtable` mutates the table. Should we use a better name? Perhaps
`ensure_in_hashtable`?

In other words, the function is really "insert if not already there
and return the previous state". Similar methods in C++ and Rust are
called `insert`, though they return the opposite, i.e. whether the
insertion took place. If we did that, then `insert_into_hashtable` may
be a good name instead.

> +       unsigned int hash = strhash(name, len);

Nit: this could be `const`, but I see we are not using it in
`fixdep.c` (for non-pointees) and it was not done in the original. But
it could be nice to start...

Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel

Re: [PATCH 5/6] fixdep: avoid parsing the same file over again

[Miguel Ojeda]

On Sat, Dec 31, 2022 at 7:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> To skip the second parsing, this commit adds a hash table for parsed
> files, just like we did for CONFIG options.

Acked-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel

Re: [PATCH 6/6] fixdep: do not parse *.so, *.rmeta, *.rlib

[Miguel Ojeda]

On Sat, Dec 31, 2022 at 7:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> rustc outputs *.so, *.rmeta, *rlib to dep-info. fixdep needs them in
> the dependency, but there is no point to parse such binary files.

Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel

Re: [PATCH 0/6] kbuild: fix dep-file processing for rust

[Miguel Ojeda]

On Sat, Dec 31, 2022 at 4:06 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Yes, my plan is to get it in the kbuild tree with your ack.

Done, also compile- and boot-tested each (on top of -rc1).

Cheers,
Miguel

Re: [PATCH 1/6] kbuild: specify output names separately for each emission type from rustc

[Vincenzo]

Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>


On Sat Dec 31, 2022 at 7:41 AM CET, Masahiro Yamada wrote:
> 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 creates
> and renames the same file path, samples/rust/rust_minimal.d.
>
> This does not happen when compiling C or assembly files because we
> explicitly specify the dependency filename by using the preprocessor
> option, -Wp,-MMD,$(depfile). $(depfile) is a unique path for each target.
>
> Currently, rustc is only given --out-dir and the list of emitted types.
> So, all the rust build rules output the dep-info into the default
> <CRATE_NAME>.d, causing the 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 command part. Also, remove
> the redundant --out-dir because we specify the output path 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>
> ---
>
>  rust/Makefile          | 10 ++++------
>  scripts/Makefile.build | 14 +++++++-------
>  scripts/Makefile.host  |  9 +++------
>  3 files changed, 14 insertions(+), 19 deletions(-)
>
> diff --git a/rust/Makefile b/rust/Makefile
> index ff70c4c916f8..0e2a32f4b3e9 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,9 @@ 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 da133780b751..4434cdbf7b8e 100644
> --- a/scripts/Makefile.host
> +++ b/scripts/Makefile.host
> @@ -84,8 +84,8 @@ _hostc_flags   = $(KBUILD_HOSTCFLAGS)   $(HOST_EXTRACFLAGS)   \
>                   $(HOSTCFLAGS_$(target-stem).o)
>  _hostcxx_flags = $(KBUILD_HOSTCXXFLAGS) $(HOST_EXTRACXXFLAGS) \
>                   $(HOSTCXXFLAGS_$(target-stem).o)
> -_hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
> -                  $(HOSTRUSTFLAGS_$(target-stem))
> +hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
> +                  $(HOSTRUSTFLAGS_$(target-stem)) --emit=dep-info=$(depfile)
>  
>  # $(objtree)/$(obj) for including generated headers from checkin source files
>  ifeq ($(KBUILD_EXTMOD),)
> @@ -97,7 +97,6 @@ endif
>  
>  hostc_flags    = -Wp,-MMD,$(depfile) $(_hostc_flags)
>  hostcxx_flags  = -Wp,-MMD,$(depfile) $(_hostcxx_flags)
> -hostrust_flags = $(_hostrust_flags)
>  
>  #####
>  # Compile programs on the host
> @@ -149,9 +148,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

Re: [PATCH 3/6] kbuild: remove sed commands after rustc rules

[Vincenzo]

Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

> 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>
> ---
>
>  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 0e2a32f4b3e9..c8941fec6955 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
> @@ -349,8 +348,7 @@ quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L
>  		$(filter-out $(skip_flags),$(rust_flags) $(rustc_target_flags)) \
>  		--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 4434cdbf7b8e..bc782655d09e 100644
> --- a/scripts/Makefile.host
> +++ b/scripts/Makefile.host
> @@ -148,8 +148,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

Re: [PATCH 5/6] fixdep: avoid parsing the same file over again

[Vincenzo]

Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

On Sat Dec 31, 2022 at 7:42 AM CET, Masahiro Yamada wrote:
> The dep files (*.d files) emitted by C compilers usually contain the
> deduplicated list of included files.
>
> There is an exceptional case; if a header is included by the -include
> command line option, and also by #include directive, it appears twice
> in the *.d file.
>
> For example, the top Makefile specifies the command line option,
> -include $(srctree)/include/linux/kconfig.h. You do not need to
> add #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 including <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>
> ---
>
>  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 b20777b888d7..cc8f6d34c2ca 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)
>  {
> @@ -361,6 +361,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;
> @@ -368,7 +372,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

Re: [PATCH 1/6] kbuild: specify output names separately for each emission type from rustc

[Masahiro Yamada]

On Wed, Jan 4, 2023 at 5:45 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sat, Dec 31, 2022 at 7:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> >   $ make -j$(nproc) samples/rust/rust_minimal.o samples/rust/rust_minimal.rsi \
> >                     samples/rust/rust_minimal.s samples/rust/rust_minimal.ll
>
> Yeah, we were testing the single targets, but not multiple at once, thanks!
>
> > +               --emit=dep-info=$(depfile) --emit=obj=$@ --emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \
>
> Perhaps a newline here to avoid the lengthy line?


OK, I will wrap it in v2.


>
> >  hostc_flags    = -Wp,-MMD,$(depfile) $(_hostc_flags)
> >  hostcxx_flags  = -Wp,-MMD,$(depfile) $(_hostcxx_flags)
> > -hostrust_flags = $(_hostrust_flags)
>
> This was originally meant to be consistent with C and C++ indeed, but
> if you prefer less variables, I guess it is fine, in which case,
> should we update the C/C++ side too (in another series)?


Yup, we could do this with less variables.
I will send a clean up.


> Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
> Tested-by: Miguel Ojeda <ojeda@kernel.org>
>
> Cheers,
> Miguel



-- 
Best Regards
Masahiro Yamada