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.

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

Feature/split x86 dependencies #272

Closed
wants to merge 13 commits into from

Conversation

markos
Copy link

@markos markos commented Sep 24, 2020

Hello,
This is the first part of an ongoing effort to port hyperscan to ARM, in particular ARM NEON and Advanced SIMD support. Before we add actual ARM support, it was necessary to make the source code properly platform agnostic, with platform specific implementations in a separate folder, so that it does not make code maintainance more difficult and for aesthetic reasons (not too many ifdef ARCH in the same function).

For that reason, we did the following:

  • move x86 specific types, functions and definitions to util/arch/x86 folder.
  • where x86 intrinsics were used throughout the code, we replaced them with virtual intrinsics (like eg. set32x8()).
  • crc32, bitutils and popcount functions were virtualized and the actual functions call the respective *_impl() functions, which can be platform-specific.
  • moved cpuid specific changes to util/arch/x86.
  • Minor other changes

So far, the changes made don't break compilation or performance or unit tests.
We would appreciate any feedback on this PR.

UPDATE: popcount abstraction was reverted, the tests were failing for no apparent reason, will need to investigate.

@mr-c
Copy link

mr-c commented Sep 24, 2020

@markos Maybe using https://github.com/simd-everywhere/simde would help the port? It would also enable other architectures as well

@markos
Copy link
Author

markos commented Sep 24, 2020

@markos Maybe using https://github.com/simd-everywhere/simde would help the port? It would also enable other architectures as well

@mr-c This would require substantial refactoring of the internals of hyperscan, esp the algorithm implementations, more drastic than just adding NEON support, and it would also take more time. I'm not personally against it but something like that would have to be agreed by the Intel maintainers upfront. I do have a similar idea however, but I need to discuss it further.

@markos
Copy link
Author

markos commented Sep 25, 2020

@mr-c for the time being, the focus is on getting NEON support in there. That does not exclude another approach in the future however.

@hulksmaaash
Copy link

@xiangwang1 Do you see any issues with this approach?

This was referenced Dec 8, 2020
@markos
Copy link
Author

markos commented Dec 16, 2020

It has been communicated that Intel is not interested in supporting multiple architectures in this project. Closing the PR. If interested in other architectures support, please check our fork at https://github.com/VectorCamp/hyperscan.

@markos markos closed this Dec 16, 2020
@markos markos deleted the feature/split-x86-dependencies branch November 17, 2023 14:07
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