-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Add command line options and build system support for specifying LLVM target cpu and features #2595
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work! This will be really useful, especially for benchmarking certain cores and CPUs
src/link.cpp
Outdated
@@ -22,7 +22,7 @@ struct LinkJob { | |||
static CodeGen *create_child_codegen(CodeGen *parent_gen, Buf *root_src_path, OutType out_type, | |||
ZigLibCInstallation *libc) | |||
{ | |||
CodeGen *child_gen = codegen_create(nullptr, root_src_path, parent_gen->zig_target, out_type, | |||
CodeGen *child_gen = codegen_create(nullptr, root_src_path, parent_gen->zig_target, out_type, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a stray space at the end of this line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know some people care a lot about formatting, but here the policy is to not give contributors a hard time about it. Don't worry about stray spaces or formatting. Let's focus on the logic and the bigger picture.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, my apologies. I didn't mean to be harsh.
Cached files in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this patch.
Although LLVM is currently the only backend of Zig, the language does not depend on LLVM, and it is planned to support non-LLVM backends. Even though the target cpu and features options need to eventually be given to LLVM in a way that LLVM understands, we still need to expose these options in a way that is independent of LLVM. That means the CLI option cannot be called --llvm-cpu
or --llvm-
anything. zig targets
must list all the possible options. The CLI should be able to reject invalid options without consulting LLVM.
As mentioned in #2051 (in this comment specifically), the Zig compiler currently does not allow the override of the LLVM cpu (-mcpu) and features (-mattr) options. This PR adds the command-line flags
--llvm-cpu
and--llvm-features
to the build-* compiler commands. In addition, the build system has been updated to support these options as well. The strings accepted by the--llvm-cpu
and--llvm-features
options are equivalent to those accepted by clang and llc with-mcpu
and-mattr
, respectively.This PR does not include a way of listing supported cpus or features of a given architecture. The easiest way to obtain such a list is to run
llc -march=<arch> -mcpu=help
, replacing<arch>
with the desired architecture, i.e.arm
oravr
.