Cannot build zstd 1.3.1 with -mbmi

[Totktonada]

ZStd has the following defines in common/threading.h:

#define pthread_mutex_t int
#define pthread_cond_t int

The defines interfere with /usr/include/bits/pthreadtypes.h, which included by ZStd transitively via immintrin.h when -mbmi is enabled:

$ gcc -mbmi   -I./common -c compress/zstdmt_compress.c
In file included from compress/zstdmt_compress.c:25:0:
./common/threading.h:82:25: error: expected ‘;’, identifier or ‘(’ before ‘int’
 #define pthread_mutex_t int   /* #define rather than typedef, because sometimes pthread support is implicit, resulting in duplicated symbols */
                         ^
./common/threading.h:82:25: warning: useless type name in empty declaration
In file included from compress/zstdmt_compress.c:25:0:
./common/threading.h:88:24: error: expected ‘;’, identifier or ‘(’ before ‘int’
 #define pthread_cond_t int
                        ^
./common/threading.h:88:24: warning: useless type name in empty declaration

The error becomes less ambiguous when the file preprocessed before trying to compile:

$ gcc -mbmi   -I./common -c compress/zstdmt_compress.c -E >ppp.c && gcc ppp.c
In file included from /usr/include/sys/types.h:266:0,
                 from /usr/include/stdlib.h:291,
                 from /usr/lib/gcc/x86_64-pc-linux-gnu/6.4.0/include/mm_malloc.h:27,
                 from /usr/lib/gcc/x86_64-pc-linux-gnu/6.4.0/include/xmmintrin.h:34,
                 from /usr/lib/gcc/x86_64-pc-linux-gnu/6.4.0/include/immintrin.h:29,
                 from ./common/bitstream.h:71,
                 from ./common/fse.h:307,
                 from ./common/zstd_internal.h:23,
                 from compress/zstdmt_compress.c:26:
/usr/include/bits/pthreadtypes.h:128:2: error: expected ‘;’, identifier or ‘(’ before ‘int’
 } pthread_mutex_t;
  ^~~
In file included from /usr/include/sys/types.h:266:0,
                 from /usr/include/stdlib.h:291,
                 from /usr/lib/gcc/x86_64-pc-linux-gnu/6.4.0/include/mm_malloc.h:27,
                 from /usr/lib/gcc/x86_64-pc-linux-gnu/6.4.0/include/xmmintrin.h:34,
                 from /usr/lib/gcc/x86_64-pc-linux-gnu/6.4.0/include/immintrin.h:29,
                 from ./common/bitstream.h:71,
                 from ./common/fse.h:307,
                 from ./common/zstd_internal.h:23,
                 from compress/zstdmt_compress.c:26:
/usr/include/bits/pthreadtypes.h:167:2: error: expected ‘;’, identifier or ‘(’ before ‘int’
 } pthread_cond_t;
  ^~~

The problem appears on every haswell, broadwell, skylake targets, when -march=native is used (it assumes -mbmi). It is very commom case for Gentoo Linux installations.

[Cyan4973]

Thanks for clear notification @Totktonada .

I tried to reproduce the issue, on both Linux and OS-X, using gcc and clang, with -mbmi, -mavx2 and -march=native, but failed to reproduce the error.
Could it be specific to some compiler version ?

[Totktonada]

$ gcc --version
gcc (Gentoo 6.4.0 p1.0) 6.4.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Can be reduced as follow for me:

$ pwd
/home/alex/tmp/zstd/zstd/lib
$ git status | head -1
HEAD detached at aecf3b47
$ gcc -mbmi   -I./common -c compress/zstdmt_compress.c
… errors here …

It seems dev branch has no pthread-related defines, so the problem was gone, but I’m not sure how dev and last release (v1.3.1 tag) corresponds each other. Maybe some related change should be backported and released.

Okay, let’s get back to reproducing the issue on 1.3.1. Maybe binutils version also affects the effect?

$ binutils-config --get-current-profile
x86_64-pc-linux-gnu-2.28.1

I have the following typedef in the /usr/include/bits/pthreadtypes.h header:

 88 /* Data structures for mutex handling.  The structure of the attribute
 89    type is not exposed on purpose.  */
 90 typedef union
 91 {
…
128 } pthread_mutex_t;

The root of a problem that the pthread_mutex_t define in common/threading.h affects this typedef (transitively included from compress/zstdmt_compress.c) when the following code from common/bitstream.h is in use:

 67 /*=========================================
 68 *  Target specific
 69 =========================================*/
 70 #if defined(__BMI__) && defined(__GNUC__)
 71 #  include <immintrin.h>   /* support for bextr (experimental) */
 72 #endif

Maybe __BMI__ is not defined on your system with -mbmi? Maybe immintrin.h does not include nothing contains pthread_mutex_t (the includes trace on my system can be shows in ppp.c build above)? Can I help to reproduce somehow else?

UPD: I mean a glibc version, which ships the sys/types.h header, when mistakelly said about binutils… that is following:

sys-libs/glibc-2.25-r5

[Cyan4973]

OK ! Using your shortened compilation command, I can reproduce the issue on Ubuntu
(OS-X would not fail).

So now we need a creative way to know when pthread.h is transitively included.
Quite an interesting build issue...

Any standard macro we could test to check the presence of pthread.h ?

[Cyan4973]

I remember now.
I believe I already met a similar problem, when the code was using typedef instead, as explained in this note :
https://github.com/facebook/zstd/blob/dev/lib/common/threading.h#L86
probably on OS-X.

Problem is, we are now dependent on #include order, since the redefined symbols are now used in later header files brought by immintrin.h.

So it's an issue.
A solution could be to stop using pthread symbols in our code, relying instead on our own versions (could be as simple as adding a ZSTD_ prefix in front of them). This way, no more symbol collision.

This is a lot to rewrite though ...

[Totktonada]

In the time we define pthread_mutex_t and pthread_cond_t the threadtypes.h header is not included, so any check for 'whether pthread was included' will fail.

Previous approach with typedefs just emit the warning about type redefinition? Maybe it is better than the error in some cases like that one. The right way would be using zstd-owned types, yep…

[Cyan4973]

@terrelln patch renames all pthread related variables to avoid such collision.
It should fix build issue on gentoo using -march=native with AVX2-enabled cpus.

[Totktonada]

Thanks, @terrelln and @Cyan4973!

I 'rebased' this fix upon v1.3.1 for ones who want the release version: https://gist.github.com/Totktonada/b603174e720b90b3b5372037a496e8a9

[Totktonada]

It does not mentioned in the release notes, but the fix available since v1.3.2.

[Cyan4973]

Yes @Totktonada , you are right.
I've added it in the Detailed list of changes section