[asm] Share portability macros and restrict ASM further #2893
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
terrelln
changed the title
terrelln
force-pushed
the
issue-2789
branch
2 times, most recently
from
808b40c
to
40aa7a8
Compare
terrelln
force-pushed
the
issue-2789
branch
3 times, most recently
from
cf493af
to
537089d
Compare
Cyan4973
approved these changes
Dec 2, 2021
|
Is there any need to add any CI test to detect issues like #2789 ? |
|
I'll look into adding a CMake based Windows CI. I'll put it in a separate PR, with a failing CI. Then rebase this on top of that. |
terrelln
force-pushed
the
issue-2789
branch
2 times, most recently
from
70a5998
to
c284569
Compare
Move portability macros to `lib/common/portability_macros.h`. This file only contains platform/feature detection (e.g. 0/1 macros). This file is shared between C and ASM code, so it cannot include any C code. Rename `HUF_` ASM macros to be `ZSTD_` prefixed, and move to the new header. Restrict `ZSTD_ASM_SUPPORTED` to `__GNUC__`, because we need the GAS assembler. Finally, only include the ASM code if we are actually going to use it. This disables it on all Windows platforms, which should resolve the problem brought up in Issue facebook#2789.
terrelln
added a commit
to terrelln/zstd
that referenced
this pull request
Dec 8, 2021
Disable assembly when dataflow sanitizer is enabled. This regressed in PR facebook#2893, which accidentally removed the check for dataflow sanitizer.
Merged
terrelln
added a commit
to terrelln/zstd
that referenced
this pull request
Dec 8, 2021
Disable assembly when dataflow sanitizer is enabled. This regressed in PR facebook#2893, which accidentally removed the check for dataflow sanitizer.
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.
Move portability macros to
lib/common/portability_macros.h. This fileonly contains platform/feature detection (e.g. 0/1 macros). This file is
shared between C and ASM code, so it cannot include any C code.
Rename
HUF_ASM macros to beZSTD_prefixed, and move to the newheader.
Restrict
ZSTD_ASM_SUPPORTEDto__GNUC__, because we need the GASassembler.
Finally, only include the ASM code if we are actually going to use it.
This disables it on all Windows platforms, which should resolve the
problem brought up in Issue #2789.