Check STATIC_BMI2 instead of __BMI2__

[pps83]

ZSTD_ENABLE_ASM_X86_64_BMI2 already implies BMI2 by its name, and by the way it tests defines

[Cyan4973]

Looking at the definition of ZSTD_ENABLE_ASM_X86_64_BMI2, it appears it does not imply the presence of __BMI2__.

The full test is :

#if !defined(ZSTD_DISABLE_ASM) &&                       \
    ZSTD_ASM_SUPPORTED &&                               \
    defined(__x86_64__) &&                              \
    (DYNAMIC_BMI2 || defined(__BMI2__))

So, ZSTD_ENABLE_ASM_X86_64_BMI2 can be defined to 1 if DYNAMIC_BMI2 is set to 1 too, even if __BMI2__ is not defined.

It follows that ZSTD_ENABLE_ASM_X86_64_BMI2 && defined(__BMI2__) is not the same test as ZSTD_ENABLE_ASM_X86_64_BMI2 alone. So the proposed change is not obvious to validate.

The context into which this macro test is invoked is also a bit complex, or at least it's not trivial to me. I just note that, before this block, there are other blocks also triggered by similar though different macro tests, such as #if DYNAMIC_BMI2, and # if ZSTD_ENABLE_ASM_X86_64_BMI2, and they do something similar, such as setting loopFn function pointer.

I would look for @terrelln guidance on this code block, and in absence of such guidance, I would rather lean towards cautiousness and not validate the suggested modification.

[pps83]

Yep, you are right, it's not exact.

Please, ignore this one for now. Imo it should be refactored with STATIC_BMI instead

[pps83]

Please, ignore this one for now. Imo it should be refactored with STATIC_BMI instead

should be ok now.

[pps83]

linux-kernel fails with this error:

../linux/lib/zstd/compress/../common/portability_macros.h:66:11: error: "STATIC_BMI2" is not defined, evaluates to 0 [-Werror=undef]
   66 |       && !STATIC_BMI2

however, there is no STATIC_BMI2 on portability_macros.h:66 line

[Cyan4973]

Try to rebase your PR on top of latest dev branch commit

[pps83]

Try to rebase your PR on top of latest dev branch commit

done

[Cyan4973]

I'm not too sure what is happening with the linux test, but it's a significant issue. Maybe portability_macros.h gets altered or rebuilt by some script during the linux build process. cc @terrelln .

[pps83]

doesn't look like it, entire zstd repo only has a couple of mentions of it.

Code	File	Line	Column
#include "../common/portability_macros.h"	zstd\lib\compress\zstd_cwksp.h	19	21
#include "portability_macros.h"			zstd\lib\common\compiler.h	16	11
#include "../common/portability_macros.h"	zstd\lib\decompress\huf_decompress_amd64.S	11	21

also, it mentions some bogus line numbers in the error message:

In file included from ../linux/lib/zstd/compress/../common/compiler.h:17,
                 from ../linux/lib/zstd/compress/fse_compress.c:19:
../linux/lib/zstd/compress/../common/portability_macros.h:66:11: error: "STATIC_BMI2" is not defined, evaluates to 0 [-Werror=undef]
   66 |       && !STATIC_BMI2
      |           ^~~~~~~~~~~

On top of that, right before where STATIC_BMI2 is used in portability_macros.h there is a define for STATIC_BMI2.