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.

Sign up for GitHub

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

Jump to bottom

[clang] -mfpmath turned into an error for assembly files in clang-17, previously it was a warning that it was unused. #65023

Closed
parona-source opened this issue Aug 27, 2023 · 6 comments · Fixed by llvm/llvm-project-release-prs#659
Closed

[clang] -mfpmath turned into an error for assembly files in clang-17, previously it was a warning that it was unused. #65023

parona-source opened this issue Aug 27, 2023 · 6 comments · Fixed by llvm/llvm-project-release-prs#659
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' release:backport release:fixed-in-main release:merged
Milestone
LLVM 17.0.X Release

Comments

@parona-source
Copy link

With clang-17 (rc2,rc3) -mfpmath gives an error with assembly files. Previously it was a warning that the option was unused.

$ clang --version
clang version 17.0.0
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/lib/llvm/17/bin
Configuration file: /etc/clang/x86_64-pc-linux-gnu-clang.cfg

$ clang-16 --version
clang version 16.0.6
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/lib/llvm/16/bin
Configuration file: /etc/clang/x86_64-pc-linux-gnu-clang.cfg

$ touch foo.{c,cpp,s}

$ clang -mfpmath=sse -c foo.c; echo $?
0

$ clang -mfpmath=sse -c foo.cpp; echo $?
0

$ clang -mfpmath=sse -c foo.s; echo $?
clang: error: unsupported option '-mfpmath=' for target 'x86_64-pc-linux-gnu'
1

Previous behavior

$ clang-16 -mfpmath=sse -c foo.c; echo $?
0

$ clang-16 -mfpmath=sse -c foo.cpp; echo $?
0

$ clang-16 -mfpmath=sse -c foo.s; echo $?
clang-16: warning: argument unused during compilation: '-mfpmath=sse' [-Wunused-command-line-argument]
0

https://bugs.gentoo.org/912246
@parona-source parona-source changed the title [clang] -mfpmath turned into an error for assembly files in clang-17, previously it was a warning that it was unsused. [clang] -mfpmath turned into an error for assembly files in clang-17, previously it was a warning that it was unused. Aug 27, 2023
@EugeneZelenko EugeneZelenko added clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' and removed new issue labels Aug 27, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Aug 27, 2023

@llvm/issue-subscribers-clang-driver

@to268
Copy link
Contributor

to268 commented Aug 27, 2023

CC @jansvoboda11 @MaskRay
This issue probably need to be addressed before Clang 17 release.

@thesamesam thesamesam added this to the LLVM 17.0.X Release milestone Aug 28, 2023
@MaskRay
Copy link
Member

MaskRay commented Aug 28, 2023

Patch: https://reviews.llvm.org/D159010

I've described the general problem in https://maskray.me/blog/2023-08-25-clang-wunused-command-line-argument#target-specific-options

In the long term we should move Clang::RenderTargetOptions to tools::renderTargetOptions be shared with ClangAs, but currently there are some rough corners.

@MaskRay
Copy link
Member

MaskRay commented Aug 28, 2023

/cherry-pick 081afa3

@MaskRay MaskRay reopened this Aug 28, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Aug 28, 2023

/branch llvm/llvm-project-release-prs/issue65023

llvmbot pushed a commit to llvm/llvm-project-release-prs that referenced this issue Aug 28, 2023
@MaskRay @llvmbot
[Driver,X86] Ignore -mfpmath= for assembler input
c3f0392 
Some options are only claimed in AddX86TargetArgs/etc (called by
Clang::RenderTargetOptions).
For assembler input, `Add*TargetArgs` is not called. If an option is
unclaimed, it either leads to a -Wunused-command-line-argument warning
or an error (if `TargetSpecific` is set)
```
// clang '-###' --target=x86_64 -mfpmath=sse -c a.s
clang: error: unsupported option '-mfpmath=sse' for target 'x86_64'
```

For -mfpmath=, it's actually claimed by RenderFloatingPointOptions,
which should be moved to AddARMTargetArgs/AddX86TargetArgs later
(non-AArch32-non-x86 targets give a frontend error).
This change is localized and similar to D153691, for release/17.x
backporting.

Fix llvm/llvm-project#65023

Reviewed By: thesamesam

Differential Revision: https://reviews.llvm.org/D159010

(cherry picked from commit 081afa3d04a4bc0d43c62b5b0e5a84f86a8a70ec)
@llvmbot llvmbot mentioned this issue Aug 28, 2023
Merged
@llvmbot
Copy link
Collaborator

llvmbot commented Aug 28, 2023

/pull-request llvm/llvm-project-release-prs#659

tru pushed a commit to llvm/llvm-project-release-prs that referenced this issue Aug 29, 2023
@MaskRay @tru
[Driver,X86] Ignore -mfpmath= for assembler input
ed108ee 
Some options are only claimed in AddX86TargetArgs/etc (called by
Clang::RenderTargetOptions).
For assembler input, `Add*TargetArgs` is not called. If an option is
unclaimed, it either leads to a -Wunused-command-line-argument warning
or an error (if `TargetSpecific` is set)
```
// clang '-###' --target=x86_64 -mfpmath=sse -c a.s
clang: error: unsupported option '-mfpmath=sse' for target 'x86_64'
```

For -mfpmath=, it's actually claimed by RenderFloatingPointOptions,
which should be moved to AddARMTargetArgs/AddX86TargetArgs later
(non-AArch32-non-x86 targets give a frontend error).
This change is localized and similar to D153691, for release/17.x
backporting.

Fix llvm/llvm-project#65023

Reviewed By: thesamesam

Differential Revision: https://reviews.llvm.org/D159010

(cherry picked from commit 081afa3d04a4bc0d43c62b5b0e5a84f86a8a70ec)
@MaskRay MaskRay mentioned this issue Sep 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' release:backport release:fixed-in-main release:merged
Projects
LLVM Release Status
Status: Done
Milestone
LLVM 17.0.X Release
Development

Successfully merging a pull request may close this issue.

PR for llvm/llvm-project#65023 llvm/llvm-project-release-prs
6 participants
@MaskRay @EugeneZelenko @thesamesam @to268 @parona-source @llvmbot