All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nathan Chancellor <nathan@kernel.org>
To: masahiroy@kernel.org
Cc: ndesaulniers@google.com, nicolas@fjasle.eu, trix@redhat.com,
	 linux-kbuild@vger.kernel.org, llvm@lists.linux.dev,
	 Nathan Chancellor <nathan@kernel.org>
Subject: [PATCH v2 04/14] kbuild: Update assembler calls to use proper flags and language target
Date: Wed, 11 Jan 2023 20:05:01 -0700	[thread overview]
Message-ID: <20221228-drop-qunused-arguments-v2-4-9adbddd20d86@kernel.org> (raw)
In-Reply-To: <20221228-drop-qunused-arguments-v2-0-9adbddd20d86@kernel.org>

From: Nick Desaulniers <ndesaulniers@google.com>

as-instr uses KBUILD_AFLAGS, but as-option uses KBUILD_CFLAGS. This can
cause as-option to fail unexpectedly when CONFIG_WERROR is set, because
clang will emit -Werror,-Wunused-command-line-argument for various -m
and -f flags in KBUILD_CFLAGS for assembler sources.

Callers of as-option and as-instr should be adding flags to
KBUILD_AFLAGS / aflags-y, not KBUILD_CFLAGS / cflags-y. Use
KBUILD_AFLAGS in all macros to clear up the initial problem.

Unfortunately, -Wunused-command-line-argument can still be triggered
with clang by the presence of warning flags or macro definitions because
'-x assembler' is used, instead of '-x assembler-with-cpp', which will
consume these flags. Switch to '-x assembler-with-cpp' in places where
'-x assembler' is used, as the compiler is always used as the driver for
out of line assembler sources in the kernel.

Finally, add -Werror to these macros so that they behave consistently
whether or not CONFIG_WERROR is set.

Link: https://github.com/ClangBuiltLinux/linux/issues/1699
Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
[nathan: Reworded and expanded on problems in commit message
         Use '-x assembler-with-cpp' in a couple more places]
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
Benchmarking '-x assembler' vs. '-x assembler-with-cpp' does show that
invoking the preprocessor incurs some overhead but my kernel builds do
not show any noticeable slowdowns due to how infrequently these macros
are used.

$ hyperfine -L lang assembler,assembler-with-cpp -N -w 5 "/usr/bin/gcc -x {lang} -c -o /dev/null /dev/null"
Benchmark 1: /usr/bin/gcc -x assembler -c -o /dev/null /dev/null
  Time (mean ± σ):       5.5 ms ±   0.5 ms    [User: 4.1 ms, System: 1.3 ms]
  Range (min … max):     4.3 ms …   6.3 ms    472 runs

Benchmark 2: /usr/bin/gcc -x assembler-with-cpp -c -o /dev/null /dev/null
  Time (mean ± σ):      12.7 ms ±   1.0 ms    [User: 9.1 ms, System: 3.6 ms]
  Range (min … max):     9.6 ms …  14.2 ms    225 runs

Summary
  '/usr/bin/gcc -x assembler -c -o /dev/null /dev/null' ran
    2.29 ± 0.28 times faster than '/usr/bin/gcc -x assembler-with-cpp -c -o /dev/null /dev/null'

$ hyperfine -L lang assembler,assembler-with-cpp -N -w 5 "/usr/bin/clang -x {lang} -c -o /dev/null /dev/null"
Benchmark 1: /usr/bin/clang -x assembler -c -o /dev/null /dev/null
  Time (mean ± σ):      21.0 ms ±   1.1 ms    [User: 9.6 ms, System: 11.1 ms]
  Range (min … max):    13.9 ms …  22.0 ms    138 runs

Benchmark 2: /usr/bin/clang -x assembler-with-cpp -c -o /dev/null /dev/null
  Time (mean ± σ):      56.9 ms ±   3.2 ms    [User: 27.2 ms, System: 29.5 ms]
  Range (min … max):    53.9 ms …  62.1 ms    48 runs

Summary
  '/usr/bin/clang -x assembler -c -o /dev/null /dev/null' ran
    2.71 ± 0.20 times faster than '/usr/bin/clang -x assembler-with-cpp -c -o /dev/null /dev/null'
---
 scripts/Kconfig.include   | 2 +-
 scripts/Makefile.compiler | 8 ++++----
 scripts/as-version.sh     | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
index 274125307ebd..5a84b6443875 100644
--- a/scripts/Kconfig.include
+++ b/scripts/Kconfig.include
@@ -33,7 +33,7 @@ ld-option = $(success,$(LD) -v $(1))
 
 # $(as-instr,<instr>)
 # Return y if the assembler supports <instr>, n otherwise
-as-instr = $(success,printf "%b\n" "$(1)" | $(CC) $(CLANG_FLAGS) -c -x assembler -o /dev/null -)
+as-instr = $(success,printf "%b\n" "$(1)" | $(CC) $(CLANG_FLAGS) -c -x assembler-with-cpp -o /dev/null -)
 
 # check if $(CC) and $(LD) exist
 $(error-if,$(failure,command -v $(CC)),C compiler '$(CC)' not found)
diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
index 3d8adfd34af1..7aa1fbc4aafe 100644
--- a/scripts/Makefile.compiler
+++ b/scripts/Makefile.compiler
@@ -29,16 +29,16 @@ try-run = $(shell set -e;		\
 	fi)
 
 # as-option
-# Usage: cflags-y += $(call as-option,-Wa$(comma)-isa=foo,)
+# Usage: aflags-y += $(call as-option,-Wa$(comma)-isa=foo,)
 
 as-option = $(call try-run,\
-	$(CC) $(KBUILD_CFLAGS) $(1) -c -x assembler /dev/null -o "$$TMP",$(1),$(2))
+	$(CC) -Werror $(KBUILD_AFLAGS) $(1) -c -x assembler-with-cpp /dev/null -o "$$TMP",$(1),$(2))
 
 # as-instr
-# Usage: cflags-y += $(call as-instr,instr,option1,option2)
+# Usage: aflags-y += $(call as-instr,instr,option1,option2)
 
 as-instr = $(call try-run,\
-	printf "%b\n" "$(1)" | $(CC) $(KBUILD_AFLAGS) -c -x assembler -o "$$TMP" -,$(2),$(3))
+	printf "%b\n" "$(1)" | $(CC) -Werror $(KBUILD_AFLAGS) -c -x assembler-with-cpp -o "$$TMP" -,$(2),$(3))
 
 # __cc-option
 # Usage: MY_CFLAGS += $(call __cc-option,$(CC),$(MY_CFLAGS),-march=winchip-c6,-march=i586)
diff --git a/scripts/as-version.sh b/scripts/as-version.sh
index 1a21495e9ff0..af717476152d 100755
--- a/scripts/as-version.sh
+++ b/scripts/as-version.sh
@@ -45,7 +45,7 @@ orig_args="$@"
 # Get the first line of the --version output.
 IFS='
 '
-set -- $(LC_ALL=C "$@" -Wa,--version -c -x assembler /dev/null -o /dev/null 2>/dev/null)
+set -- $(LC_ALL=C "$@" -Wa,--version -c -x assembler-with-cpp /dev/null -o /dev/null 2>/dev/null)
 
 # Split the line on spaces.
 IFS=' '

-- 
2.39.0


  parent reply	other threads:[~2023-01-12  3:05 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-12  3:04 [PATCH v2 00/14] Remove clang's -Qunused-arguments from KBUILD_CPPFLAGS Nathan Chancellor
2023-01-12  3:04 ` Nathan Chancellor
2023-01-12  3:04 ` Nathan Chancellor
2023-01-12  3:04 ` [PATCH v2 01/14] x86/boot/compressed: prefer cc-option for CFLAGS additions Nathan Chancellor
2023-01-12  3:04 ` [PATCH v2 02/14] MIPS: Always use -Wa,-msoft-float and eliminate GAS_HAS_SET_HARDFLOAT Nathan Chancellor
2023-01-12  3:05 ` [PATCH v2 03/14] MIPS: Prefer cc-option for additions to cflags Nathan Chancellor
2023-01-12  3:05 ` Nathan Chancellor [this message]
2023-01-12  3:05 ` [PATCH v2 05/14] powerpc: Remove linker flag from KBUILD_AFLAGS Nathan Chancellor
2023-01-12  3:05   ` Nathan Chancellor
2023-01-25  4:11   ` Michael Ellerman
2023-01-25  4:11     ` Michael Ellerman
2023-01-26  1:29     ` Masahiro Yamada
2023-01-26  1:29       ` Masahiro Yamada
2023-01-26  2:07       ` Nathan Chancellor
2023-01-26  2:07         ` Nathan Chancellor
2023-01-26  4:22         ` Masahiro Yamada
2023-01-26  4:22           ` Masahiro Yamada
2023-01-26 10:05           ` Michael Ellerman
2023-01-26 10:05             ` Michael Ellerman
2023-01-12  3:05 ` [PATCH v2 06/14] powerpc/vdso: Remove unused '-s' flag from ASFLAGS Nathan Chancellor
2023-01-12  3:05   ` Nathan Chancellor
2023-01-25  4:12   ` Michael Ellerman
2023-01-25  4:12     ` Michael Ellerman
2023-01-12  3:05 ` [PATCH v2 07/14] powerpc/vdso: Improve linker flags Nathan Chancellor
2023-01-12  3:05   ` Nathan Chancellor
2023-01-12 18:02   ` Sedat Dilek
2023-01-12 18:02     ` Sedat Dilek
2023-01-12 18:21     ` Nathan Chancellor
2023-01-12 18:21       ` Nathan Chancellor
2023-01-12 18:47       ` Sedat Dilek
2023-01-12 18:47         ` Sedat Dilek
2023-01-22 17:27       ` Masahiro Yamada
2023-01-22 17:27         ` Masahiro Yamada
2023-01-22 18:01         ` Nathan Chancellor
2023-01-22 18:01           ` Nathan Chancellor
2023-01-23 15:07   ` Segher Boessenkool
2023-01-23 15:07     ` Segher Boessenkool
2023-01-24 16:14     ` Nathan Chancellor
2023-01-24 16:14       ` Nathan Chancellor
2023-01-24 16:30       ` Segher Boessenkool
2023-01-24 16:30         ` Segher Boessenkool
2023-01-12  3:05 ` [PATCH v2 08/14] powerpc/vdso: Remove an unsupported flag from vgettimeofday-32.o with clang Nathan Chancellor
2023-01-12  3:05   ` Nathan Chancellor
2023-01-12  3:05 ` [PATCH v2 09/14] s390/vdso: Drop unused '-s' flag from KBUILD_AFLAGS_64 Nathan Chancellor
2023-01-12  3:05 ` [PATCH v2 10/14] s390/vdso: Drop '-shared' from KBUILD_CFLAGS_64 Nathan Chancellor
2023-01-12  3:05 ` [PATCH v2 11/14] s390/purgatory: Remove unused '-MD' and unnecessary '-c' flags Nathan Chancellor
2023-01-12  3:05 ` [PATCH v2 12/14] drm/amd/display: Do not add '-mhard-float' to dml_ccflags for clang Nathan Chancellor
2023-01-12  3:05   ` Nathan Chancellor
2023-01-12  3:05 ` [PATCH v2 13/14] kbuild: Turn a couple more of clang's unused option warnings into errors Nathan Chancellor
2023-01-12  3:05 ` [PATCH v2 14/14] kbuild: Stop using '-Qunused-arguments' with clang Nathan Chancellor
2023-01-22 17:28 ` [PATCH v2 00/14] Remove clang's -Qunused-arguments from KBUILD_CPPFLAGS Masahiro Yamada
2023-01-22 17:28   ` Masahiro Yamada
2023-01-22 17:28   ` Masahiro Yamada
2023-01-23 13:58 ` Naresh Kamboju
2023-01-23 13:58   ` Naresh Kamboju
2023-01-23 13:58   ` Naresh Kamboju
2023-01-23 16:11   ` Nathan Chancellor
2023-01-23 16:11     ` Nathan Chancellor
2023-01-23 16:11     ` Nathan Chancellor
2023-01-24 15:29     ` Naresh Kamboju
2023-01-24 15:29       ` Naresh Kamboju
2023-01-24 15:29       ` Naresh Kamboju

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221228-drop-qunused-arguments-v2-4-9adbddd20d86@kernel.org \
    --to=nathan@kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=masahiroy@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=nicolas@fjasle.eu \
    --cc=trix@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.