Make prelude support nasm #652
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
the
CLA Signed
JakobDegen
approved these changes
May 19, 2024
|
GH says there are conflicts, could you rebase? Will import afterwards |
|
Done |
|
@JakobDegen has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@JakobDegen has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
facebook-github-bot
pushed a commit
to facebook/buck2-prelude
that referenced
this pull request
May 24, 2024
Summary: We use a certain open source third party library which has some assembly files which can only be built with [nasm](https://nasm.us/). nasm has a couple of command line differences that are relevant for the purposes of getting minimal nasm support: 1. response files don't use the syntax `foo.argsfile`, rather they use the syntax `-@foo.argsfile`. 2. Content of response files cannot be quoted. 3. src file input is not prefixed with `-c`. Since our toolchain now conditionally uses nasm or the default toolchain assembler, we are taking advantage of `cxx_toolchain_override()`. This was missing the ability to override some of the assembler arguments (e.g. we need to set `compiler_type="nasm"`), and also we discovered a few bugs where some fields were not null checked. So all of this is fixed in this PR. One thing I don't entirely understand is depfiles, or if I need to do anything. nasm accepts a couple of relevant command line options, which I don't fully understand how to use ``` -M generate Makefile dependencies on stdout -MG d:o, missing files assumed generated -MF file set Makefile dependency file -MD file assemble and generate dependencies -MT file dependency target name -MQ file dependency target name (quoted) -MP emit phony target ``` I can get a `.d` file to generate by passing `-MD /path/to/d/file -MT /path/to/obj/file` but I don't know what's going to happen if I omit this. Everything seems to work for me. but maybe it's because none of my asm files depend on any other external files. Willing to investigate this further with some guidance, but the PR already works for me as is, so if people are ok with the current missing depfile support, so am I. X-link: facebook/buck2#652 Reviewed By: christolliday Differential Revision: D57592001 Pulled By: JakobDegen fbshipit-source-id: 5b56586ae396b2d507d7bb3dab84cb6dad4e9e96
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We use a certain open source third party library which has some assembly files which can only be built with nasm. nasm has a couple of command line differences that are relevant for the purposes of getting minimal nasm support:
@foo.argsfile, rather they use the syntax-@foo.argsfile.-c.Since our toolchain now conditionally uses nasm or the default toolchain assembler, we are taking advantage of
cxx_toolchain_override(). This was missing the ability to override some of the assembler arguments (e.g. we need to setcompiler_type="nasm"), and also we discovered a few bugs where some fields were not null checked. So all of this is fixed in this PR.One thing I don't entirely understand is depfiles, or if I need to do anything. nasm accepts a couple of relevant command line options, which I don't fully understand how to use
I can get a
.dfile to generate by passing-MD /path/to/d/file -MT /path/to/obj/filebut I don't know what's going to happen if I omit this. Everything seems to work for me. but maybe it's because none of my asm files depend on any other external files. Willing to investigate this further with some guidance, but the PR already works for me as is, so if people are ok with the current missing depfile support, so am I.