[PATCH] D64527: driver: Don't warn about assembler flags being unused when not assembling
nickdesaulniers added a comment. Fixup: https://reviews.llvm.org/D64655 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64527/new/ https://reviews.llvm.org/D64527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D64527: driver: Don't warn about assembler flags being unused when not assembling
Yes, the current behavior is erroring for assembler flags clang does not
recognize, which is irrelevant when compiling with `-no-integrated-as`,
which is breaking our assembler flag feature detection in building the
Linux kernel.
On Thu, Jul 11, 2019, 6:09 PM Reid Kleckner via Phabricator <
revi...@reviews.llvm.org> wrote:
> rnk added a comment.
>
> I think the intention of this change was to ignore assembler flags in
> pre-processing actions without warning about them. It implements that
> behavior by running the code that gathers and validates assembler flags.
> The validation is whats emitting the problematic errors. I don't think
> that's desirable, so I'd recommend reverting for now.
>
>
>
>
> Comment at: cfe/trunk/lib/Driver/ToolChains/Clang.cpp:3555
> + // in -E mode either for example, even though it's not really used
> either.
> + if (!isa(JA)) {
> +ArgStringList DummyArgs;
>
> nickdesaulniers wrote:
> > Should you be checking `!TC.IsIntegratedAssemblerDefault()` additionally?
> I don't think so. We want to ignore assembler flags in a preprocessor
> action even if we're not going to use the integrated assembler.
>
>
> Repository:
> rL LLVM
>
> CHANGES SINCE LAST ACTION
> https://reviews.llvm.org/D64527/new/
>
> https://reviews.llvm.org/D64527
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D64527: driver: Don't warn about assembler flags being unused when not assembling
rnk added a comment.
I think the intention of this change was to ignore assembler flags in
pre-processing actions without warning about them. It implements that behavior
by running the code that gathers and validates assembler flags. The validation
is whats emitting the problematic errors. I don't think that's desirable, so
I'd recommend reverting for now.
Comment at: cfe/trunk/lib/Driver/ToolChains/Clang.cpp:3555
+ // in -E mode either for example, even though it's not really used either.
+ if (!isa(JA)) {
+ArgStringList DummyArgs;
nickdesaulniers wrote:
> Should you be checking `!TC.IsIntegratedAssemblerDefault()` additionally?
I don't think so. We want to ignore assembler flags in a preprocessor action
even if we're not going to use the integrated assembler.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64527/new/
https://reviews.llvm.org/D64527
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D64527: driver: Don't warn about assembler flags being unused when not assembling
nickdesaulniers added inline comments.
Comment at: cfe/trunk/lib/Driver/ToolChains/Clang.cpp:3555
+ // in -E mode either for example, even though it's not really used either.
+ if (!isa(JA)) {
+ArgStringList DummyArgs;
Should you be checking `!TC.IsIntegratedAssemblerDefault()` additionally?
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64527/new/
https://reviews.llvm.org/D64527
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D64527: driver: Don't warn about assembler flags being unused when not assembling
nickdesaulniers added a comment. ah, maybe the addition of `-no-integrated-as` shouldn't produce this error? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64527/new/ https://reviews.llvm.org/D64527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D64527: driver: Don't warn about assembler flags being unused when not assembling
nickdesaulniers added a comment. > This change breaks building the Linux kernel for arm32 (at least): Follow up discussion in https://github.com/ClangBuiltLinux/linux/issues/598. I think the kernel is wrong here. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64527/new/ https://reviews.llvm.org/D64527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D64527: driver: Don't warn about assembler flags being unused when not assembling
nathanchance added a comment. This change breaks building the Linux kernel for arm32 (at least): ... YACCscripts/dtc/dtc-parser.tab.c HOSTCC scripts/dtc/yamltree.o HOSTCC scripts/dtc/dtc-parser.tab.o HOSTCC scripts/dtc/dtc-lexer.lex.o HOSTLD scripts/dtc/dtc UPD include/config/kernel.release UPD include/generated/utsrelease.h HOSTCC scripts/asn1_compiler HOSTCC scripts/extract-cert HOSTCC scripts/sortextable HOSTCC scripts/kallsyms HOSTCC scripts/conmakehash SYSNR arch/arm/include/generated/asm/unistd-nr.h SYSTBL arch/arm/include/generated/calls-oabi.S GEN arch/arm/include/generated/asm/mach-types.h SYSTBL arch/arm/include/generated/calls-eabi.S HOSTCC scripts/mod/mk_elfconfig CC scripts/mod/devicetable-offsets.s CC scripts/mod/empty.o clang-9: error: unsupported argument '-mno-warn-deprecated' to option 'Wa,' make[2]: *** [scripts/Makefile.build:112: scripts/mod/devicetable-offsets.s] Error 1 make[2]: *** Waiting for unfinished jobs clang-9: error: unsupported argument '-mno-warn-deprecated' to option 'Wa,' make[2]: *** [scripts/Makefile.build:279: scripts/mod/empty.o] Error 1 make[1]: *** [Makefile:1118: prepare0] Error 2 make: *** [Makefile:325: __build_one_by_one] Error 2 The full command line that causes the issue is: /home/nathan/cbl/tc-build/build/llvm/stage1/bin/clang -Wp,-MD,scripts/mod/.empty.o.d -nostdinc -isystem /home/nathan/cbl/tc-build/build/llvm/stage1/lib/clang/9.0.0/include -I./arch/arm/include -I./arch/arm/include/generated -I./include -I./arch/arm/include/uapi -I./arch/arm/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -mlittle-endian -Qunused-arguments -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration -Werror=implicit-int -Wno-format-security -std=gnu89 --target=arm-linux-gnueabi --prefix=/home/nathan/cbl/usr/bin/ --gcc-toolchain=/home/nathan/cbl/usr -no-integrated-as -Werror=unknown-warning-option -fno-dwarf2-cfi-asm -mabi=aapcs-linux -mfpu=vfp -funwind-tables -marm -Wa,-mno-warn-deprecated -D__LINUX_ARM_ARCH__=7 -march=armv7-a -msoft-float -Uarm -O2 -fstack-protector-strong -Wno-format-invalid-specifier -Wno-gnu -Wno-tautological-compare -mno-global-merge -fomit-frame-pointer -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Wno-initializer-overrides -Wno-unused-value -Wno-format -Wno-sign-compare -Wno-format-zero-length-DKBUILD_BASENAME='"empty"' -DKBUILD_MODNAME='"empty"' -c -o scripts/mod/empty.o scripts/mod/empty.c I'm not in the right state of mind (exhausted) to debug this but I wanted to let you know in case you have any immediate ideas. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64527/new/ https://reviews.llvm.org/D64527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D64527: driver: Don't warn about assembler flags being unused when not assembling
This revision was automatically updated to reflect the committed changes.
Closed by commit rL365703: driver: Dont warn about assembler flags being
unused when not assembling (authored by nico, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D64527?vs=209041=209081#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64527/new/
https://reviews.llvm.org/D64527
Files:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/as-options.s
Index: cfe/trunk/test/Driver/as-options.s
===
--- cfe/trunk/test/Driver/as-options.s
+++ cfe/trunk/test/Driver/as-options.s
@@ -35,3 +35,14 @@
// RUN: | FileCheck %s
// CHECK: "-I" "foo_dir"
+
+// Test that assembler options don't cause warnings when there's no assembler
+// stage.
+
+// RUN: %clang -mincremental-linker-compatible -E -o /dev/null %s 2>&1 \
+// RUN: | FileCheck --check-prefix=WARN --allow-empty %s
+// RUN: %clang -mimplicit-it=always -target armv7-linux-gnueabihf -E %s \
+// RUN: | FileCheck --check-prefix=WARN --allow-empty %s
+// RUN: %clang -Wa,-mbig-obj -target i386-pc-windows -E %s \
+// RUN: | FileCheck --check-prefix=WARN --allow-empty %s
+// WARN-NOT: unused
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -3545,6 +3545,18 @@
// Select the appropriate action.
RewriteKind rewriteKind = RK_None;
+ // If CollectArgsForIntegratedAssembler() isn't called below, call it here
+ // with a dummy args list to mark assembler flags as used even when not
+ // running an assembler. Otherwise, clang would emit "argument unused"
+ // warnings for assembler flags when e.g. adding "-E" to flags while
debugging
+ // something. That'd be somewhat inconvenient, and it's also inconsistent
with
+ // most other flags -- we don't warn on -ffunction-sections not being used
+ // in -E mode either for example, even though it's not really used either.
+ if (!isa(JA)) {
+ArgStringList DummyArgs;
+CollectArgsForIntegratedAssembler(C, Args, DummyArgs, D);
+ }
+
if (isa(JA)) {
assert(JA.getType() == types::TY_Plist && "Invalid output type.");
CmdArgs.push_back("-analyze");
Index: cfe/trunk/test/Driver/as-options.s
===
--- cfe/trunk/test/Driver/as-options.s
+++ cfe/trunk/test/Driver/as-options.s
@@ -35,3 +35,14 @@
// RUN: | FileCheck %s
// CHECK: "-I" "foo_dir"
+
+// Test that assembler options don't cause warnings when there's no assembler
+// stage.
+
+// RUN: %clang -mincremental-linker-compatible -E -o /dev/null %s 2>&1 \
+// RUN: | FileCheck --check-prefix=WARN --allow-empty %s
+// RUN: %clang -mimplicit-it=always -target armv7-linux-gnueabihf -E %s \
+// RUN: | FileCheck --check-prefix=WARN --allow-empty %s
+// RUN: %clang -Wa,-mbig-obj -target i386-pc-windows -E %s \
+// RUN: | FileCheck --check-prefix=WARN --allow-empty %s
+// WARN-NOT: unused
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -3545,6 +3545,18 @@
// Select the appropriate action.
RewriteKind rewriteKind = RK_None;
+ // If CollectArgsForIntegratedAssembler() isn't called below, call it here
+ // with a dummy args list to mark assembler flags as used even when not
+ // running an assembler. Otherwise, clang would emit "argument unused"
+ // warnings for assembler flags when e.g. adding "-E" to flags while debugging
+ // something. That'd be somewhat inconvenient, and it's also inconsistent with
+ // most other flags -- we don't warn on -ffunction-sections not being used
+ // in -E mode either for example, even though it's not really used either.
+ if (!isa(JA)) {
+ArgStringList DummyArgs;
+CollectArgsForIntegratedAssembler(C, Args, DummyArgs, D);
+ }
+
if (isa(JA)) {
assert(JA.getType() == types::TY_Plist && "Invalid output type.");
CmdArgs.push_back("-analyze");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D64527: driver: Don't warn about assembler flags being unused when not assembling
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64527/new/ https://reviews.llvm.org/D64527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D64527: driver: Don't warn about assembler flags being unused when not assembling
thakis created this revision.
thakis added a reviewer: rnk.
Herald added subscribers: kristof.beyls, javed.absar.
clang currently warns when passing flags for the assembler (e.g.
-Wa,-mbig-obj) to an invocation that doesn't run the assembler (e.g.
-E).
At first sight, that makes sense -- the flag really is unused. But many
other flags don't have an effect if no assembler runs (e.g.
-fno-integrated-as, -ffunction-sections, and many others), and those
currently don't warn. So this seems more like a side effect of how
CollectArgsForIntegratedAssembler() is implemented than like an
intentional feature.
Since it's a bit inconvenient when debugging builds and adding -E,
always call CollectArgsForIntegratedAssembler() to make sure assembler
args always get claimed. Currently, this affects only these flags:
-mincremental-linker-compatible, -mimplicit-it= (on ARM), -Wa, -Xassembler
It does have the side effect that assembler options now need to be valid
even if -E is passed. Previously, `-Wa,-mbig-obj` would error for
non-coff output only if the assembler ran, now it always errors. This
too makes assembler flags more consistent with all the other flags and
seems like a progression.
Fixes PR42066.
https://reviews.llvm.org/D64527
Files:
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/as-options.s
Index: clang/test/Driver/as-options.s
===
--- clang/test/Driver/as-options.s
+++ clang/test/Driver/as-options.s
@@ -35,3 +35,14 @@
// RUN: | FileCheck %s
// CHECK: "-I" "foo_dir"
+
+// Test that assembler options don't cause warnings when there's no assembler
+// stage.
+
+// RUN: %clang -mincremental-linker-compatible -E -o /dev/null %s 2>&1 \
+// RUN: | FileCheck --check-prefix=WARN --allow-empty %s
+// RUN: %clang -mimplicit-it=always -target armv7-linux-gnueabihf -E %s \
+// RUN: | FileCheck --check-prefix=WARN --allow-empty %s
+// RUN: %clang -Wa,-mbig-obj -target i386-pc-windows -E %s \
+// RUN: | FileCheck --check-prefix=WARN --allow-empty %s
+// WARN-NOT: unused
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3545,6 +3545,18 @@
// Select the appropriate action.
RewriteKind rewriteKind = RK_None;
+ // If CollectArgsForIntegratedAssembler() isn't called below, call it here
+ // with a dummy args list to mark assembler flags as used even when not
+ // running an assembler. Otherwise, clang would emit "argument unused"
+ // warnings for assembler flags when e.g. adding "-E" to flags while
debugging
+ // something. That'd be somewhat inconvenient, and it's also inconsistent
with
+ // most other flags -- we don't warn on -ffunction-sections not being used
+ // in -E mode either for example, even though it's not really used either.
+ if (!isa(JA)) {
+ArgStringList DummyArgs;
+CollectArgsForIntegratedAssembler(C, Args, DummyArgs, D);
+ }
+
if (isa(JA)) {
assert(JA.getType() == types::TY_Plist && "Invalid output type.");
CmdArgs.push_back("-analyze");
Index: clang/test/Driver/as-options.s
===
--- clang/test/Driver/as-options.s
+++ clang/test/Driver/as-options.s
@@ -35,3 +35,14 @@
// RUN: | FileCheck %s
// CHECK: "-I" "foo_dir"
+
+// Test that assembler options don't cause warnings when there's no assembler
+// stage.
+
+// RUN: %clang -mincremental-linker-compatible -E -o /dev/null %s 2>&1 \
+// RUN: | FileCheck --check-prefix=WARN --allow-empty %s
+// RUN: %clang -mimplicit-it=always -target armv7-linux-gnueabihf -E %s \
+// RUN: | FileCheck --check-prefix=WARN --allow-empty %s
+// RUN: %clang -Wa,-mbig-obj -target i386-pc-windows -E %s \
+// RUN: | FileCheck --check-prefix=WARN --allow-empty %s
+// WARN-NOT: unused
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3545,6 +3545,18 @@
// Select the appropriate action.
RewriteKind rewriteKind = RK_None;
+ // If CollectArgsForIntegratedAssembler() isn't called below, call it here
+ // with a dummy args list to mark assembler flags as used even when not
+ // running an assembler. Otherwise, clang would emit "argument unused"
+ // warnings for assembler flags when e.g. adding "-E" to flags while debugging
+ // something. That'd be somewhat inconvenient, and it's also inconsistent with
+ // most other flags -- we don't warn on -ffunction-sections not being used
+ // in -E mode either for example, even though it's not really used either.
+ if (!isa(JA)) {
+ArgStringList DummyArgs;
+CollectArgsForIntegratedAssembler(C, Args, DummyArgs, D);
+ }
+
if (isa(JA)) {
assert(JA.getType() == types::TY_Plist && "Invalid output type.");
