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

c++ compatibility hack #377

Closed
wants to merge 1 commit into from
Closed

Conversation

divinity76
Copy link
Contributor

@divinity76 divinity76 commented Jan 24, 2024

adding BLAKE3 to a c++ project was painful.
There is still an issue with BLAKE3_ATOMICS=1, but now BLAKE3_ATOMICS=0 compiles without errors, previously:

hans@DESKTOP-EE15SLU:~/projects/misc/b3instructions$ g++ -o mini mini.cpp upstream_blake3/c/blake3.c upstream_blake3/c/blake3_dispatch.c upstream_blake3/c/blake3_portable.c -DBLAKE3_NO_SSE2 -DBLAKE3_NO_AVX2 -DBLAKE3_NO_AVX512 -DBLAKE3_NO_NEON -DBLAKE3_NO_SSE41 -DBLAKE3_ATOMICS=0 -std=c++20 
upstream_blake3/c/blake3_dispatch.c: In function ‘cpu_feature get_cpu_features()’:
upstream_blake3/c/blake3_dispatch.c:115:43: error: invalid conversion from ‘int’ to ‘cpu_feature’ [-fpermissive]
  115 |   enum cpu_feature features = ATOMIC_LOAD(g_cpu_features);
      |                                           ^~~~~~~~~~~~~~
      |                                           |
      |                                           int
upstream_blake3/c/blake3_dispatch.c:40:24: note: in definition of macro ‘ATOMIC_LOAD’
   40 | #define ATOMIC_LOAD(x) x
      |                        ^
upstream_blake3/c/blake3_dispatch.c:123:16: error: invalid conversion from ‘int’ to ‘cpu_feature’ [-fpermissive]
  123 |     features = 0;
      |                ^
      |                |
      |                int
upstream_blake3/c/blake3_dispatch.c:128:14: error: invalid conversion from ‘int’ to ‘cpu_feature’ [-fpermissive]
  128 |     features |= SSE2;
      |     ~~~~~~~~~^~~~~~~
      |              |
      |              int
upstream_blake3/c/blake3_dispatch.c:134:16: error: invalid conversion from ‘int’ to ‘cpu_feature’ [-fpermissive]
  134 |       features |= SSSE3;
      |       ~~~~~~~~~^~~~~~~~
      |                |
      |                int
upstream_blake3/c/blake3_dispatch.c:136:16: error: invalid conversion from ‘int’ to ‘cpu_feature’ [-fpermissive]
  136 |       features |= SSE41;
      |       ~~~~~~~~~^~~~~~~~
      |                |
      |                int
upstream_blake3/c/blake3_dispatch.c:142:20: error: invalid conversion from ‘int’ to ‘cpu_feature’ [-fpermissive]
  142 |           features |= AVX;
      |           ~~~~~~~~~^~~~~~
      |                    |
      |                    int
upstream_blake3/c/blake3_dispatch.c:146:22: error: invalid conversion from ‘int’ to ‘cpu_feature’ [-fpermissive]
  146 |             features |= AVX2;
      |             ~~~~~~~~~^~~~~~~
      |                      |
      |                      int
upstream_blake3/c/blake3_dispatch.c:149:24: error: invalid conversion from ‘int’ to ‘cpu_feature’ [-fpermissive]
  149 |               features |= AVX512VL;
      |               ~~~~~~~~~^~~~~~~~~~~
      |                        |
      |                        int
upstream_blake3/c/blake3_dispatch.c:151:24: error: invalid conversion from ‘int’ to ‘cpu_feature’ [-fpermissive]
  151 |               features |= AVX512F;
      |               ~~~~~~~~~^~~~~~~~~~
      |                        |
      |                        int

adding BLAKE3 to a c++ project was painful.
There is still an issue with BLAKE3_ATOMICS=1, but now it compiles without errors, previously:
```cpp
hans@DESKTOP-EE15SLU:~/projects/misc/b3instructions$ g++ -o mini mini.cpp upstream_blake3/c/blake3.c upstream_blake3/c/blake3_dispatch.c upstream_blake3/c/blake3_portable.c -DBLAKE3_NO_SSE2 -DBLAKE3_NO_AVX2 -DBLAKE3_NO_AVX512 -DBLAKE3_NO_NEON -DBLAKE3_NO_SSE41 -DBLAKE3_ATOMICS=0 -std=c++20 
upstream_blake3/c/blake3_dispatch.c: In function ‘cpu_feature get_cpu_features()’:
upstream_blake3/c/blake3_dispatch.c:115:43: error: invalid conversion from ‘int’ to ‘cpu_feature’ [-fpermissive]
  115 |   enum cpu_feature features = ATOMIC_LOAD(g_cpu_features);
      |                                           ^~~~~~~~~~~~~~
      |                                           |
      |                                           int
upstream_blake3/c/blake3_dispatch.c:40:24: note: in definition of macro ‘ATOMIC_LOAD’
   40 | #define ATOMIC_LOAD(x) x
      |                        ^
upstream_blake3/c/blake3_dispatch.c:123:16: error: invalid conversion from ‘int’ to ‘cpu_feature’ [-fpermissive]
  123 |     features = 0;
      |                ^
      |                |
      |                int
upstream_blake3/c/blake3_dispatch.c:128:14: error: invalid conversion from ‘int’ to ‘cpu_feature’ [-fpermissive]
  128 |     features |= SSE2;
      |     ~~~~~~~~~^~~~~~~
      |              |
      |              int
upstream_blake3/c/blake3_dispatch.c:134:16: error: invalid conversion from ‘int’ to ‘cpu_feature’ [-fpermissive]
  134 |       features |= SSSE3;
      |       ~~~~~~~~~^~~~~~~~
      |                |
      |                int
upstream_blake3/c/blake3_dispatch.c:136:16: error: invalid conversion from ‘int’ to ‘cpu_feature’ [-fpermissive]
  136 |       features |= SSE41;
      |       ~~~~~~~~~^~~~~~~~
      |                |
      |                int
upstream_blake3/c/blake3_dispatch.c:142:20: error: invalid conversion from ‘int’ to ‘cpu_feature’ [-fpermissive]
  142 |           features |= AVX;
      |           ~~~~~~~~~^~~~~~
      |                    |
      |                    int
upstream_blake3/c/blake3_dispatch.c:146:22: error: invalid conversion from ‘int’ to ‘cpu_feature’ [-fpermissive]
  146 |             features |= AVX2;
      |             ~~~~~~~~~^~~~~~~
      |                      |
      |                      int
upstream_blake3/c/blake3_dispatch.c:149:24: error: invalid conversion from ‘int’ to ‘cpu_feature’ [-fpermissive]
  149 |               features |= AVX512VL;
      |               ~~~~~~~~~^~~~~~~~~~~
      |                        |
      |                        int
upstream_blake3/c/blake3_dispatch.c:151:24: error: invalid conversion from ‘int’ to ‘cpu_feature’ [-fpermissive]
  151 |               features |= AVX512F;
      |               ~~~~~~~~~^~~~~~~~~~
      |                        |
      |                        int
```
@BurningEnlightenment
Copy link
Collaborator

I'm sorry, but I don't quite understand why you would even want to compile this library with a C++ compiler. Can you explain your use case in detail?

@divinity76
Copy link
Contributor Author

I'm trying to add BLAKE3 to PHP: php/php-src#13194
On the php-internals mailing list, it was asked if it was worthwhile to ship the SSE2/SSE41/AVX2/AVX512 optimized versions because, quote

Should we even be considering the specific instruction implementations?
I've always been in the camp
of you are not smarter than the compiler. As even the best human written
ASM code can be slower
than the obscure instructions the compiler might choose to use in a weird
and wonderful way

So i wanted to make a simple benchmark for the portable vs sse2/sse41/avx2/avx512,
and I'm just more comfortable with C++ than C, so I initially tried to make that benchmark program in C++ (just because of familiarity with C++ over C) .. thus here we are.

I ended up writing it in C anyway, and the benchmarking code is https://gist.github.com/divinity76/5729472dd5d77e94cd0acb245aac2226
and the result is
image

@BurningEnlightenment
Copy link
Collaborator

So i wanted to make a simple benchmark for the portable vs sse2/sse41/avx2/avx512,
and I'm just more comfortable with C++ than C, so I initially tried to make that benchmark program in C++ (just because of familiarity with C++ over C) .. thus here we are.

You can still link a C++ program with C libraries. Either use one of the provided buildsystems or for quick'n'dirty testing you could even bunch it up in a composite object file like so:

gcc -r -o libblake3.o upstream_blake3/c/blake3.c upstream_blake3/c/blake3_dispatch.c upstream_blake3/c/blake3_portable.c -DBLAKE3_NO_SSE2 -DBLAKE3_NO_AVX2 -DBLAKE3_NO_AVX512 -DBLAKE3_NO_NEON -DBLAKE3_NO_SSE41
g++ -o mini mini.cpp libblake3.o -std=c++20

@divinity76
Copy link
Contributor Author

i see, thanks! nevermind

@divinity76 divinity76 closed this Jan 25, 2024
@BurningEnlightenment
Copy link
Collaborator

@divinity76 and thank you for integrating this library into PHP 🙂

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.

None yet

2 participants