Skip to content

HACL SIMD support incorrectly detected under WASI #126458

Closed
Closed
HACL SIMD support incorrectly detected under WASI#126458
Labels
OS-wasibuildThe build process and cross-buildtype-bugAn unexpected behavior, bug, or error

Description

@brettcannon
brettcannon
opened on Nov 5, 2024

Bug report

Bug description:

I'm getting a lot of:

/opt/wasi-sdk/bin/clang -c -I../../Modules/_hacl -I../../Modules/_hacl/include -D_BSD_SOURCE -D_DEFAULT_SOURCE -fno-strict-overflow -Wsign-compare -Wunreachable-code -DNDEBUG -g -O3 -Wall -Wno-unused-value -Wno-empty-body -Qunused-arguments -Wno-parentheses-equality -Wno-unused-value -Wno-empty-body -Qunused-arguments -Wno-parentheses-equality  -std=c11 -Wextra -Wno-unused-parameter -Wno-int-conversion -Wno-missing-field-initializers -Wstrict-prototypes -Werror=implicit-function-declaration -fvisibility=hidden  -I../../Include/internal -I../../Include/internal/mimalloc -IObjects -IInclude -IPython -I. -I../../Include     -msimd128 -msse -msse2 -msse3 -msse4.1 -msse4.2 -DHACL_CAN_COMPILE_VEC128 -o Modules/_hacl/Hacl_Hash_Blake2s_Simd128.o ../../Modules/_hacl/Hacl_Hash_Blake2s_Simd128.c
In file included from ../../Modules/_hacl/Hacl_Hash_Blake2s_Simd128.c:26:
In file included from ../../Modules/_hacl/internal/Hacl_Hash_Blake2s_Simd128.h:40:
../../Modules/_hacl/internal/../Hacl_Hash_Blake2s_Simd128.h:55:3: error: unknown type name 'Lib_IntVector_Intrinsics_vec128'

That seems to be defined at:

cpython/Modules/_hacl/libintvector.h

Line 30 in 5e91684

typedef __m128i Lib_IntVector_Intrinsics_vec128;

But that whole header file is guarded by:

cpython/Modules/_hacl/libintvector.h

Line 22 in 5e91684

#if defined(__x86_64__) || defined(_M_X64) || defined(__i386__) || defined(_M_IX86)

Unfortunately, if you add defined(__wasi__) to that guard you then get:

/opt/wasi-sdk-24.0-x86_64-linux/lib/clang/18/include/immintrin.h:14:2: error: "This header is only meant to be used on x86 and x64 architecture"
   14 | #error "This header is only meant to be used on x86 and x64 architecture"
      |  ^

To me that suggests the check at:

cpython/configure.ac

Lines 7854 to 7906 in 5e91684

# The SIMD files use aligned_alloc, which is not available on older versions of
# Android.
if test "$ac_sys_system" != "Linux-android" || test "$ANDROID_API_LEVEL" -ge 28; then
dnl This can be extended here to detect e.g. Power8, which HACL* should also support.
AX_CHECK_COMPILE_FLAG([-msse -msse2 -msse3 -msse4.1 -msse4.2],[
[LIBHACL_SIMD128_FLAGS="-msse -msse2 -msse3 -msse4.1 -msse4.2"]
AC_DEFINE([HACL_CAN_COMPILE_SIMD128], [1], [HACL* library can compile SIMD128 implementations])
# macOS universal2 builds *support* the -msse etc flags because they're
# available on x86_64. However, performance of the HACL SIMD128 implementation
# isn't great, so it's disabled on ARM64.
AC_MSG_CHECKING([for HACL* SIMD128 implementation])
if test "$UNIVERSAL_ARCHS" == "universal2"; then
[LIBHACL_SIMD128_OBJS="Modules/_hacl/Hacl_Hash_Blake2s_Simd128_universal2.o"]
AC_MSG_RESULT([universal2])
else
[LIBHACL_SIMD128_OBJS="Modules/_hacl/Hacl_Hash_Blake2s_Simd128.o"]
AC_MSG_RESULT([standard])
fi
], [], [-Werror])
fi
AC_SUBST([LIBHACL_SIMD128_FLAGS])
AC_SUBST([LIBHACL_SIMD128_OBJS])
# The SIMD files use aligned_alloc, which is not available on older versions of
# Android.
#
# Although AVX support is not guaranteed on Android
# (https://developer.android.com/ndk/guides/abis#86-64), this is safe because we do a
# runtime CPUID check.
if test "$ac_sys_system" != "Linux-android" || test "$ANDROID_API_LEVEL" -ge 28; then
AX_CHECK_COMPILE_FLAG([-mavx2],[
[LIBHACL_SIMD256_FLAGS="-mavx2"]
AC_DEFINE([HACL_CAN_COMPILE_SIMD256], [1], [HACL* library can compile SIMD256 implementations])
# macOS universal2 builds *support* the -mavx2 compiler flag because it's
# available on x86_64; but the HACL SIMD256 build then fails because the
# implementation requires symbols that aren't available on ARM64. Use a
# wrapped implementation if we're building for universal2.
AC_MSG_CHECKING([for HACL* SIMD256 implementation])
if test "$UNIVERSAL_ARCHS" == "universal2"; then
[LIBHACL_SIMD256_OBJS="Modules/_hacl/Hacl_Hash_Blake2b_Simd256_universal2.o"]
AC_MSG_RESULT([universal2])
else
[LIBHACL_SIMD256_OBJS="Modules/_hacl/Hacl_Hash_Blake2b_Simd256.o"]
AC_MSG_RESULT([standard])
fi
], [], [-Werror])
fi
AC_SUBST([LIBHACL_SIMD256_FLAGS])
AC_SUBST([LIBHACL_SIMD256_OBJS])

is too broad since the header files don't work on non-x86 architectures.

This probably requires either a CPU/platform check before checking the flag support or an explicit opt-out for at least WASI.

(And I have no idea why CI isn't running into this problem while I am locally; bad configure caching via

cpython/.github/workflows/reusable-wasi.yml

Lines 63 to 68 in 5e91684

- name: "Restore host config.cache"
uses: actions/cache@v4
with:
path: ${{ env.CROSS_BUILD_WASI }}/config.cache
# Should be kept in sync with the other config.cache step above.
key: ${{ github.job }}-${{ runner.os }}-${{ env.IMAGE_VERSION }}-${{ env.WASI_SDK_VERSION }}-${{ env.WASMTIME_VERSION }}-${{ inputs.config_hash }}-${{ hashFiles('Tools/wasm/wasi.py') }}-${{ env.pythonLocation }}
?)

CPython versions tested on:

CPython main branch

Operating systems tested on:

Other

Linked PRs

Activity

  1. brettcannon
    added
    buildThe build process and cross-build
    OS-wasi
    type-bugAn unexpected behavior, bug, or error
    on Nov 5, 2024

  2. brettcannon commented on Nov 5, 2024

    @brettcannon
    brettcannon
    on Nov 5, 2024
    MemberAuthor

    @erlend-aasland how would you prefer to structure this in configure.ac?

  3. erlend-aasland commented on Nov 5, 2024

    @erlend-aasland
    erlend-aasland
    on Nov 5, 2024
    Contributor

    @erlend-aasland how would you prefer to structure this in configure.ac?

    I think @picnixz already started hashing out better SIMD checks in #125022.

  4. picnixz commented on Nov 6, 2024

    @picnixz
    picnixz
    on Nov 6, 2024
    Member

    This probably requires either a CPU/platform check before checking the flag support or an explicit opt-out for at least WASI.

    The easiest way to patch it for now is to opt-out I think and just use the normal headers. That way, we won't have any issue.

    I think @picnixz already started hashing out better SIMD checks

    My checks only detect the SIMD compiler flags separately instead in one go so it could not necessarily be a solution. It does not care about the architecture itself and just to see whether the flags are supported or not. Concerning the headers that can or cannot be included, they should be done at compile time so we cannot rely on CPUID checks. But what we can do is create a .h which contains some #define telling me whether I can or cannot include that or this header (this would help a lot).

    Maybe @corona10 has some ideas on this topic? we could perhaps discuss it on the Discord (I've created a thread for #125022, so you could join (or I can just @ you)).

  5. corona10 commented on Nov 6, 2024

    @corona10
    corona10
    on Nov 6, 2024
    Member

    I will try to follow up the issue at this weekend :)

  6. brettcannon
    added a commit that references this issue on Nov 6, 2024

    pythonGH-126458: disable SIMD for HACL under WASI

    c8878ed

  7. brettcannon commented on Nov 6, 2024

    @brettcannon
    brettcannon
    on Nov 6, 2024
    MemberAuthor

    I have a PR up to at least opt out for now. If someone could give the two-liner (plus comments) a quick check I would appreciate it so I can get unblocked.

  8. brettcannon
    added a commit that references this issue on Nov 6, 2024

    GH-126458: disable SIMD for HACL under WASI (#126512)

    2a6b6b3
  9. brettcannon
    closed this as completedon Nov 6, 2024
  10. picnixz
    added a commit that references this issue on Dec 8, 2024

    pythonGH-126458: disable SIMD for HACL under WASI (python#126512)

    c2cea12
  11. ebonnal
    added a commit that references this issue on Jan 12, 2025

    pythonGH-126458: disable SIMD for HACL under WASI (python#126512)

    3d8ce1f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    OS-wasibuildThe build process and cross-buildtype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions