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

[contrib][linux] Fix build after introducing ASM HUF implementation #2790

Merged
merged 3 commits into from
Sep 24, 2021
Merged

[contrib][linux] Fix build after introducing ASM HUF implementation #2790

merged 3 commits into from
Sep 24, 2021

Conversation

solbjorn
Copy link
Contributor

This mostly fixes kernel build after introducing ASM HUF implementation, but also adds the ability to build the kernel with recently landed CONFIG_WERROR while in-kernel ZSTD library still uses the functions deprecated in 1.5.0.

Compile and run time tested on Linux 5.15-rc2 with Clang 12.

@facebook-github-bot
Copy link

Hi @solbjorn!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@terrelln
Copy link
Contributor

Thanks for the PR! The linux.mk changes look good. The macro changes seem right, but I have to think them through to be sure. Looks like we're missing a test case for !DYNAMIC_BMI2 with BMI2 enabled, I'll add that.

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

terrelln added a commit to terrelln/zstd that referenced this pull request Sep 21, 2021
@terrelln
[huf] Fix compilation when DYNAMIC_BMI2=0 && BMI2 is supported
1ee3562 
* Fix compilation issues pointed out in PR facebook#2790.
* Add test cases to GitHub actions that test all combinations of
  `DYNAMIC_BMI2` BMI2 support.
terrelln added a commit to terrelln/zstd that referenced this pull request Sep 21, 2021
@terrelln
[huf] Fix compilation when DYNAMIC_BMI2=0 && BMI2 is supported
9944d36 
* Fix compilation issues pointed out in PR facebook#2790.
* Add test cases to GitHub actions that test all combinations of
  `DYNAMIC_BMI2` BMI2 support.
@terrelln terrelln mentioned this pull request Sep 21, 2021
Merged
terrelln added a commit to terrelln/zstd that referenced this pull request Sep 21, 2021
@terrelln
[huf] Fix compilation when DYNAMIC_BMI2=0 && BMI2 is supported
9450876 
* Fix compilation issues pointed out in PR facebook#2790.
* Add test cases to GitHub actions that test all combinations of
  `DYNAMIC_BMI2` BMI2 support.
@terrelln
Copy link
Contributor

@solbjorn I'm going to fix the macro issues in PR #2791, where I also add a test case that covers them.

Once that is merged (later today or tomorrow), can you rebase on top of the latest dev branch, and just include the config changes?

@solbjorn
Copy link
Contributor Author

@terrelln, sure, great!

@solbjorn
Copy link
Contributor Author

Updated.

@solbjorn
[contrib][linux] Fix build with CONFIG_WERROR
e6d62bb 
Linux 5.15 introduces a new Kconfig option, CONFIG_WERROR, which
forces -Werror for the entire kernel.
Current in-kernel ZSTD implementation uses functions deprecated
in 1.5.0, and thus fails on -Wdeprecated-declarations.
Turn this particular error into warning to be able to build the
kernel with CONFIG_WERROR. I'm not disabling them completely to
make sure they'll be visible and [hopefully] fixed sooner or later.

Signed-off-by: Alexander Lobakin <alobakin@pm.me>
@solbjorn
[contrib][linux] Add huf_decompress_amd64.o target to Makefile
c45b27a 
Commit a5f2c45 ("Huffman ASM") added a new ASM source file,
but it wasn't added to the kernel Makefile despite that it received
support for Huffman ASM according to the internal definitions. This
leads to undefined references, as huf_decompress.o now calls those
ASM functions.
Add it to the list of sources when building inside the kernel tree.
Kbuild can handle .S files just fine, so none additional rules
needed.

Fixes: a5f2c45 ("Huffman ASM")
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
@solbjorn
[contrib][linux] Add contrib/linux-kernel/linux to .gitignore
d8b7fc5 
The mentioned path is being created/used by the 'import' rule for
generating source files for Linux kernel.

Signed-off-by: Alexander Lobakin <alobakin@pm.me>
@solbjorn solbjorn changed the title linux-kernel: fix build after introducing ASM HUF implementation [contrib][linux] Fix build after introducing ASM HUF implementation Sep 23, 2021
@terrelln terrelln merged commit 32a8443 into facebook:dev Sep 24, 2021
@terrelln
Copy link
Contributor

Thanks for the PR @solbjorn!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@terrelln terrelln terrelln approved these changes

Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@solbjorn @facebook-github-bot @terrelln