Page MenuHomePhabricator

driver: Don't warn about assembler flags being unused when not assembling; different approach
ClosedPublic

Authored by thakis on Jul 24 2019, 10:58 AM.

Details

Summary

This has mostly the same effect as https://reviews.llvm.org/D65108 but has a more localized implementation that duplicates more code.

This approach also doesn't validate flags for the integrated assembler if the assembler step doesn't run.

Diff Detail

Repository
rL LLVM

Event Timeline

thakis created this revision.Jul 24 2019, 10:58 AM
rnk accepted this revision.Jul 26 2019, 12:42 PM
rnk added a subscriber: rnk.

lgtm

clang/lib/Driver/ToolChains/Clang.cpp
3564 ↗(On Diff #211564)

This makes me think perhaps we should diagnose -mimplicit-it= on non-arm targets as an error and then simplify this, but that's a separable change.

This revision is now accepted and ready to land.Jul 26 2019, 12:42 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 26 2019, 6:12 PM
phosek added a subscriber: phosek.Jul 26 2019, 10:25 PM

We see test failures in as-options.s:

******************** TEST 'Clang :: Driver/as-options.s' FAILED ********************
Script:
--
: 'RUN: at line 3';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -target x86_64-linux-gnu -c -no-integrated-as /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s    -Ifoo_dir -### 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
: 'RUN: at line 7';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -target x86_64-linux-gnu -c -no-integrated-as /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s    -I foo_dir -### 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
: 'RUN: at line 11';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -target x86_64-linux-gnu -c -integrated-as /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s    -Ifoo_dir -### 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
: 'RUN: at line 15';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -target x86_64-linux-gnu -c -integrated-as /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s    -I foo_dir -### 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
: 'RUN: at line 21';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -target aarch64-linux-gnu -c -no-integrated-as /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s    -Ifoo_dir -### 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
: 'RUN: at line 25';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -target aarch64-linux-gnu -c -integrated-as /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s    -Ifoo_dir -### 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
: 'RUN: at line 29';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -target armv7-linux-gnueabihf -c -no-integrated-as /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s    -Ifoo_dir -### 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
: 'RUN: at line 33';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -target armv7-linux-gnueabihf -c -integrated-as /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s    -Ifoo_dir -### 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
: 'RUN: at line 42';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -mincremental-linker-compatible -E    -o /dev/null -x c++ /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck --check-prefix=NOWARN --allow-empty /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
: 'RUN: at line 45';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -mincremental-linker-compatible -E    -o /dev/null -x assembler-with-cpp /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck --check-prefix=NOWARN --allow-empty /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
: 'RUN: at line 48';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -mimplicit-it=always -target armv7-linux-gnueabi -E    -o /dev/null -x c++ /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck --check-prefix=NOWARN --allow-empty /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
: 'RUN: at line 51';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -mimplicit-it=always -target armv7-linux-gnueabi -E    -o /dev/null -x assembler-with-cpp /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck --check-prefix=NOWARN --allow-empty /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
: 'RUN: at line 54';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -Wa,-mbig-obj -target i386-pc-windows -E    -o /dev/null -x c++ /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck --check-prefix=NOWARN --allow-empty /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
: 'RUN: at line 57';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -Wa,-mbig-obj -target i386-pc-windows -E    -o /dev/null -x assembler-with-cpp /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck --check-prefix=NOWARN --allow-empty /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
: 'RUN: at line 60';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -Xassembler -mbig-obj -target i386-pc-windows -E    -o /dev/null -x c++ /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck --check-prefix=NOWARN --allow-empty /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
: 'RUN: at line 63';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -Xassembler -mbig-obj -target i386-pc-windows -E    -o /dev/null -x assembler-with-cpp /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck --check-prefix=NOWARN --allow-empty /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
: 'RUN: at line 70';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -Wa,-mno-warn-deprecated -fno-integrated-as /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s -S 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck --check-prefix=NOERROR --allow-empty /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
: 'RUN: at line 76';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -Wa,-mno-warn-deprecated -fno-integrated-as /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s -S 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck --check-prefix=NOWARN --allow-empty /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
: 'RUN: at line 81';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -mrelax-all -fintegrated-as /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s -S 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck --check-prefix=NOWARN --allow-empty /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
: 'RUN: at line 83';   /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/clang -mrelax-all -fno-integrated-as /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s -S 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangrtoFQg/llvm_build_dir/bin/FileCheck --check-prefix=WARN --allow-empty /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s
--
Exit Code: 1

Command Output (stderr):
--
/b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s:66:16: error: NOWARN-NOT: excluded string found in input
// NOWARN-NOT: unused
               ^
<stdin>:1:95: note: found here
clang-10: warning: /b/s/w/ir/k/llvm-project/clang/test/Driver/as-options.s: 'assembler' input unused [-Wunused-command-line-argument]
                                                                                              ^~~~~~

---

phosek: In what config / on what bot?

phosek: Does r367176 help?

phosek: In what config / on what bot?

It's our toolchain linux-x86 builder, see https://luci-milo.appspot.com/p/fuchsia/builders/ci/clang-x64-linux/b8906751725919481408, mac-x86 has been working fine.

phosek: Does r367176 help?

No, it's still failing with the same error, see https://luci-milo.appspot.com/p/fuchsia/g/clang/console.

bjope added a subscriber: bjope.Jul 27 2019, 1:27 PM

I made another fixup (similar to https://reviews.llvm.org/rL367176) here https://reviews.llvm.org/rL367182.

Can someone please take a look (a post-commit review) of https://reviews.llvm.org/rL367182 to verify that I did not misunderstand the intention with the test somehow?

I made another fixup (similar to https://reviews.llvm.org/rL367176) here https://reviews.llvm.org/rL367182.

Can someone please take a look (a post-commit review) of https://reviews.llvm.org/rL367182 to verify that I did not misunderstand the intention with the test somehow?

That seemed to have helped, thanks!

I made another fixup (similar to https://reviews.llvm.org/rL367176) here https://reviews.llvm.org/rL367182.

Can someone please take a look (a post-commit review) of https://reviews.llvm.org/rL367182 to verify that I did not misunderstand the intention with the test somehow?

Looks good. Thanks much for the fix!

I made another fixup (similar to https://reviews.llvm.org/rL367176) here https://reviews.llvm.org/rL367182.

Can someone please take a look (a post-commit review) of https://reviews.llvm.org/rL367182 to verify that I did not misunderstand the intention with the test somehow?

Looks good. Thanks much for the fix!

Nice to know that I did not mess up the test in my attempt to silence the buildbots. So thanks for taking a look.