Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clang warns for unused Arm -march even when used by preprocessor #55656

Open
DavidSpickett opened this issue May 23, 2022 · 8 comments
Open

Clang warns for unused Arm -march even when used by preprocessor #55656

DavidSpickett opened this issue May 23, 2022 · 8 comments
Labels
backend:ARM clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl'

Comments

@DavidSpickett
Copy link
Collaborator

DavidSpickett commented May 23, 2022

(derived from ClangBuiltLinux/linux#1315)

Summary: clang should not warn that compiler -march arguments are unused when the preprocessor is run on assembly files.

/tmp/test.s is just an empty file. Each time we're going to pass -march to the compiler and assembler with -Wa.

$ ./bin/clang --target=arm-linux-gnueabi -march=armv7-m -Wa,-march=armv7-m -Wunused-command-line-argument /tmp/test.s -c
clang-15: warning: argument unused during compilation: '-march=armv7-m' [-Wunused-command-line-argument]

If we assemble only then we are warned that the compiler -march is unused. So far so good.

$ ./bin/clang --target=arm-linux-gnueabi -march=armv7-m -Wa,-march=armv7-m -Wunused-command-line-argument -x assembler-with-cpp /tmp/test.s -c
clang-15: warning: argument unused during compilation: '-march=armv7-m' [-Wunused-command-line-argument]

If we assemble but apply the preprocessor first we are still warned that it is unused. Which isn't correct because this option actually sets some preprocessor defines (justification for that at the end).

$ ./bin/clang --target=arm-linux-gnueabi -march=armv7-m -Wa,-march=armv7-m -Wunused-command-line-argument /tmp/test.s -E
clang-15: warning: /tmp/test.s: 'assembler' input unused [-Wunused-command-line-argument]
clang-15: warning: argument unused during compilation: '-Wa,-march=armv7-m' [-Wunused-command-line-argument]
clang-15: warning: argument unused during compilation: '-Wunused-command-line-argument' [-Wunused-command-line-argument]

Now if we only preprocess an assembly file there's nothing to do so everything is unused. Fine.

$ ./bin/clang --target=arm-linux-gnueabi -march=armv7-m -Wa,-march=armv7-m -Wunused-command-line-argument -x assembler-with-cpp /tmp/test.s -E
# 1 "/tmp/test.s"
# 1 "<built-in>" 1
# 1 "/tmp/test.s" 2

If we run the preprocessor on the assembly file, everything is used. Part of me says that the -Wa,-march should be unused here. You are running the preprocessor on an assembly file but also we know that only the compiler -march is used to set defines. Not sure.

The key issue is that the compiler -march should be used if you're running the preprocessor. Clang should not be warning you that it is unused.

Since it governs defines like __thumb2__ so clearly it does something. Here we see that removing the compiler -march causes this file to fail to assemble, therefore it is "used". If you put it back clang then tells you it is unused despite clearly being used to add the define.

$ cat /tmp/test2.s
#ifndef __thumb2__
#error
#endif
$ ./bin/clang --target=arm-linux-gnueabi -Wa,-march=armv7-m -Wunused-command-line-argument -x assembler-with-cpp /tmp/test2.s -c
/tmp/test2.s:2:2: error:
#error
 ^
1 error generated.
$ ./bin/clang --target=arm-linux-gnueabi -march=armv7-m -Wa,-march=armv7-m -Wunused-command-line-argument -x assembler-with-cpp /tmp/test2.
s -c
clang-15: warning: argument unused during compilation: '-march=armv7-m' [-Wunused-command-line-argument]

Somehow -march fixes the issue but is also unused.

@DavidSpickett DavidSpickett added backend:ARM clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels May 23, 2022
@DavidSpickett DavidSpickett self-assigned this May 23, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented May 23, 2022

@llvm/issue-subscribers-clang-driver

@llvmbot
Copy link
Collaborator

llvmbot commented May 23, 2022

@llvm/issue-subscribers-backend-arm

@DavidSpickett
Copy link
Collaborator Author

The warning is emitted in clang/lib/Driver/ToolChains/Arch/ARM.cpp::void arm::getARMTargetFeatures and this probably applies to -mcpu as well.

  if (WaArch) {
    if (ArchArg)
      D.Diag(clang::diag::warn_drv_unused_argument)
          << ArchArg->getAsString(Args);

Unfortunately it's not as simple as checking if ArchArg has been "claimed" since it's claimed 3/4 times even if it's going to be overriden by -Wa,-march. This can happen when you search for an argument so it doesn't (at least in practice) mean it is used but that someone looked at it.

The spirit of the warning is "hey you gave us two architecture values but we're going to prefer one hopefully it's the right one". So "unused" makes sense as the type of warning.

We do get told if this is for the assembler but it's true either way. We can't tell if this is for the preprocessor and the assembler. I could add that context.

$ ./bin/clang --target=arm-linux-gnueabi -march=armv7-m -Wa,-march=armv7-m -Wunused-command-line-argument /tmp/test.s -c
Warning unused, claimed: 1 ForAS: 1
clang-15: warning: argument unused during compilation: '-march=armv7-m' [-Wunused-command-line-argument]
$ ./bin/clang --target=arm-linux-gnueabi -march=armv7-m -Wa,-march=armv7-m -Wunused-command-line-argument -x assembler-with-cpp /tmp/test.s -c
Warning unused, claimed: 1 ForAS: 1
clang-15: warning: argument unused during compilation: '-march=armv7-m' [-Wunused-command-line-argument]

@nickdesaulniers
Copy link
Member

I think that

  if (WaArch) {
    if (ArchArg)
      D.Diag(clang::diag::warn_drv_unused_argument)
          << ArchArg->getAsString(Args);

just needs an additional check for !getLangOpts().AsmPreprocessor.

The spirit of the warning is "hey you gave us two architecture values but we're going to prefer one hopefully it's the right one". So "unused" makes sense as the type of warning.

I guess you could do something odd like -march=armv8 -Wa,-march=armv7, which would be weird for .s, .S, or .c (with inline asm) files...

But I think for the kernel, the versions match. Maybe we could not warn if the versions match?

@nico
Copy link
Contributor

nico commented May 24, 2022

https://reviews.llvm.org/D65233 (and abandoned https://reviews.llvm.org/D65108) were in this area, maybe they can provide inspiration. (Or maybe they can't, I didn't look into this issue very closely. It just looked "vaguely related" to me, and maybe that assessment is just wrong.)

@DavidSpickett
Copy link
Collaborator Author

Similar, thanks for the pointers. I think that issue is assembler vs not assembler where this is assembler vs preprocessor+assembler.

just needs an additional check for !getLangOpts().AsmPreprocessor.

Agreed, if I can find a way to get to that or something like it. Perhaps there is an equivalent hidden within the job object but I haven't found it yet.

But I think for the kernel, the versions match. Maybe we could not warn if the versions match?

As a new warning? Where the compiler flag is not unused, just different to the assembler one. Sounds like a good idea.
(once the current warning is correctly emitted we should only get it when the compiler march is being ignored)

@DavidSpickett DavidSpickett removed their assignment Jun 1, 2022
@DavidSpickett
Copy link
Collaborator Author

I'm not going to have time for this for the next few weeks. Anyone else feel free to pick it up.

intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Oct 17, 2022
When both -march= and -Wa,-march= are specified for assembler or
assembler-with-cpp sources, GCC and Clang will prefer the -Wa,-march=
value but Clang will warn that -march= is unused.

warning: argument unused during compilation: '-march=armv6k'
[-Wunused-command-line-argument]

This is the top group of warnings we observe when using clang to
assemble the kernel via `ARCH=arm make LLVM=1`.

Split the arch-y make variable into two, so that -march= flags only get
passed to the compiler, not the assembler. -D flags are added to
KBUILD_CPPFLAGS which is used for both C and assembler-with-cpp sources.

Clang is trying to warn that it doesn't support different values for
-march= and -Wa,-march= (like GCC does, but the kernel doesn't need this)
though the value of the preprocessor define __thumb2__ is based on
-march=. Make sure to re-set __thumb2__ via -D flag for assembler
sources now that we're no longer passing -march= to the assembler. Set
it to a different value than the preprocessor would for -march= in case
-march= gets accidentally re-added to KBUILD_AFLAGS in the future.
Thanks to Ard and Nathan for this suggestion.

Link: ClangBuiltLinux#1315
Link: ClangBuiltLinux#1587
Link: llvm/llvm-project#55656
Suggested-by: Ard Biesheuvel <ardb@kernel.org>
Suggested-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
jonhunter pushed a commit to jonhunter/linux that referenced this issue Nov 8, 2022
When both -march= and -Wa,-march= are specified for assembler or
assembler-with-cpp sources, GCC and Clang will prefer the -Wa,-march=
value but Clang will warn that -march= is unused.

warning: argument unused during compilation: '-march=armv6k'
[-Wunused-command-line-argument]

This is the top group of warnings we observe when using clang to
assemble the kernel via `ARCH=arm make LLVM=1`.

Split the arch-y make variable into two, so that -march= flags only get
passed to the compiler, not the assembler. -D flags are added to
KBUILD_CPPFLAGS which is used for both C and assembler-with-cpp sources.

Clang is trying to warn that it doesn't support different values for
-march= and -Wa,-march= (like GCC does, but the kernel doesn't need this)
though the value of the preprocessor define __thumb2__ is based on
-march=. Make sure to re-set __thumb2__ via -D flag for assembler
sources now that we're no longer passing -march= to the assembler. Set
it to a different value than the preprocessor would for -march= in case
-march= gets accidentally re-added to KBUILD_AFLAGS in the future.
Thanks to Ard and Nathan for this suggestion.

Link: ClangBuiltLinux#1315
Link: ClangBuiltLinux#1587
Link: llvm/llvm-project#55656

Suggested-by: Ard Biesheuvel <ardb@kernel.org>
Suggested-by: Nathan Chancellor <nathan@kernel.org>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
Tested-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
jonhunter pushed a commit to jonhunter/linux that referenced this issue Nov 9, 2022
When both -march= and -Wa,-march= are specified for assembler or
assembler-with-cpp sources, GCC and Clang will prefer the -Wa,-march=
value but Clang will warn that -march= is unused.

warning: argument unused during compilation: '-march=armv6k'
[-Wunused-command-line-argument]

This is the top group of warnings we observe when using clang to
assemble the kernel via `ARCH=arm make LLVM=1`.

Split the arch-y make variable into two, so that -march= flags only get
passed to the compiler, not the assembler. -D flags are added to
KBUILD_CPPFLAGS which is used for both C and assembler-with-cpp sources.

Clang is trying to warn that it doesn't support different values for
-march= and -Wa,-march= (like GCC does, but the kernel doesn't need this)
though the value of the preprocessor define __thumb2__ is based on
-march=. Make sure to re-set __thumb2__ via -D flag for assembler
sources now that we're no longer passing -march= to the assembler. Set
it to a different value than the preprocessor would for -march= in case
-march= gets accidentally re-added to KBUILD_AFLAGS in the future.
Thanks to Ard and Nathan for this suggestion.

Link: ClangBuiltLinux#1315
Link: ClangBuiltLinux#1587
Link: llvm/llvm-project#55656

Suggested-by: Ard Biesheuvel <ardb@kernel.org>
Suggested-by: Nathan Chancellor <nathan@kernel.org>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
Tested-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
@simonbcn
Copy link

Same problem on Arch when I try to compile mpv with meson using clang: mpv-player/mpv#11242

❯ clang --version
clang version 15.0.7
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:ARM clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl'
Projects
None yet
Development

No branches or pull requests

5 participants