Skip to content

decode: fix NEON inclusion#714

Merged
eustas merged 1 commit into
google:masterfrom
stephenkyle-ARM:fix-neon-inclusion
Oct 8, 2018
Merged

decode: fix NEON inclusion#714
eustas merged 1 commit into
google:masterfrom
stephenkyle-ARM:fix-neon-inclusion

Conversation

@stephenkyle-ARM

Copy link
Copy Markdown
Contributor

The macro that checks for NEON support should be __ARM_NEON, not
ARM_NEON. [1]

AArch64 compilers define __ARM_NEON but not ARM_NEON.
AArch32 compilers currently seem to define both, but could be within their
rights to drop ARM_NEON in future versions.

This change moves the check into the common/platform.h file, checks for
both forms, and sets BROTLI_TARGET_NEON if NEON support is available.

[1] Section 6.5.4 of the ARM C Language Extensions.
(At the time of writing, the latest version was Release 2.1.)

The macro that checks for NEON support should be __ARM_NEON, not
__ARM_NEON__. [1]

AArch64 compilers define __ARM_NEON but not __ARM_NEON__.
AArch32 compilers currently seem to define both, but could be within their
rights to drop __ARM_NEON__ in future versions.

This change moves the check into the common/platform.h file, checks for
both forms, and sets BROTLI_TARGET_NEON if NEON support is available.

[1] Section 6.5.4 of the ARM C Language Extensions.
    (At the time of writing, the latest version was Release 2.1.)
@AppVeyorBot

Copy link
Copy Markdown

@AppVeyorBot

Copy link
Copy Markdown

@eustas

eustas commented Oct 8, 2018

Copy link
Copy Markdown
Collaborator

Thanks!

@eustas eustas merged commit cc7a74f into google:master Oct 8, 2018
@stephenkyle-ARM stephenkyle-ARM deleted the fix-neon-inclusion branch October 8, 2018 14:03
@eustas

eustas commented Oct 16, 2018

Copy link
Copy Markdown
Collaborator

In this PR BROTLI_TARGET_NEON is used before its definition is included... So... when neon codepath is enabled, <arm_neon.h> is not included...
Going to fix very soon.

@stephenkyle-ARM

Copy link
Copy Markdown
Contributor Author

Thanks for doing that @eustas. I was curious why the CI bot I added didn't pick this up, I should have a PR that fixes this shortly...

juj pushed a commit to Unity-Technologies/brotli that referenced this pull request May 13, 2023
The macro that checks for NEON support should be __ARM_NEON, not
__ARM_NEON__. [1]

AArch64 compilers define __ARM_NEON but not __ARM_NEON__.
AArch32 compilers currently seem to define both, but could be within their
rights to drop __ARM_NEON__ in future versions.

This change moves the check into the common/platform.h file, checks for
both forms, and sets BROTLI_TARGET_NEON if NEON support is available.

[1] Section 6.5.4 of the ARM C Language Extensions.
    (At the time of writing, the latest version was Release 2.1.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants