-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
[bazel] Add support for LLVM_TOOL_LLVM_DRIVER_BUILD #86879
Conversation
This adds the bazel equivalent of the `llvm` binary produced by `LLVM_TOOL_LLVM_DRIVER_BUILD` in cmake. For the initial commit, this only includes `llvm-ar`, `llvm-nm`, and `llvm-size`. The rest are trivial to add in a followup commit, following the same pattern as here. By default it will include everything that supports the llvm-driver model, but it can be reduced to only build a subset, e.g. this will build only nm and size: ``` $ bazel build \ --@llvm-project//llvm:driver-tools=llvm-nm,llvm-size \ @llvm-project//llvm:llvm ```
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.
Looks great! I don't know too much about Bazel to check the rules but the code looks reasonable. I just made some nits.
@@ -4178,6 +4187,12 @@ cc_binary( | |||
], | |||
) | |||
|
|||
llvm_driver_cc_binary( | |||
name = "llvm-nm", | |||
stamp = 0, |
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.
Do we need guidance when and where we use stamp = 0
(https://bazel.build/reference/be/c-cpp) as a comment?
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 question. Some people may have strong opinions on stamping. I'm not sure if there's a reason we have stamp = 0
in these build files. Obviously there are caching benefits, but you also get those more broadly by building with bazel build --nostamp
. I can't find advice on this, e.g. in a style guide or best practices list, but I'll add a comment later if I can figure anything out.
(btw, this looks like a new line, but it's just moved from a few lines above)
This adds the bazel equivalent of the
llvm
binary produced byLLVM_TOOL_LLVM_DRIVER_BUILD
in cmake. For the initial commit, this only includesllvm-ar
,llvm-nm
, andllvm-size
. The rest are trivial to add in a followup commit, following the same pattern as here.By default it will include everything that supports the llvm-driver model, but it can be reduced to only build a subset, e.g. this will build only nm and size: