◐ Shell
clean mode source ↗

gh-130213: Check availability of Intel SIMD types by jmroot · Pull Request #130332 · python/cpython

@jmroot

blake2module.c includes headers that use SIMD typedefs if an SIMD implementation will be built, but must not itself be compiled with the -m options that enable SIMD instructions. However, the *mmintrin headers are not always usable to get those typedefs if the corresponding -m option is not used.

msprotz

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is as discussed and matches my understanding of the problem. Thank you!

blake2module.c includes headers that use SIMD typedefs if an SIMD
implementation will be built, but must not itself be compiled with the
-m options that enable SIMD instructions. However, the *mmintrin
headers are not always usable to get those typedefs if the
corresponding -m option is not used.

@jmroot

The force push was just to fix a typo in the commit message.

picnixz

Check for SIMD -m flags before checking for header issues. Clarify
comment.

picnixz

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic wise, I think it indeed renders better now. Do we have a build bot for testing this change?

@jmroot

I don't think there's a buildbot that was reproducing the issue this fixes, or it would have been noticed sooner. I have locally confirmed that this fixes the build failure on some affected systems.

Speaking of tests, the ones for this PR appear to have gotten stuck. Can someone manually retry them?

@picnixz

I've merged main into your branch (that way it's also up-to-date)

@picnixz

I'll also run the buildbots in general so that we don't introduce some misconfiguration. In the meantime, please do not push new commits as I would need to reschedule the build bots.

EDIT: Huh, apparently buildbots are not being launched =/

@picnixz

Looks like there are some network failures:

Cannot initiate the connection to ports.ubuntu.com:80 (2620:2d:4000:1::19). - connect (101: Network is unreachable) Cannot initiate the connection to ports.ubuntu.com:80 (2620:2d:4000:1::16). - connect (101: Network is unreachable)

@chris-eibl

Just for visibility here: @msprotz is working on hiding the "problematic" #include "libintvector.h" (#130483 (comment)), and asking in #130483 (comment)

Also it would help me to understand how pressing this is, so that I can prioritize the upstream work accordingly. Thank you!

Maybe this temporary workaround is not needed for the alphas?

From https://peps.python.org/pep-0745/
3.14.0 alpha 6: Friday, 2025-03-14
3.14.0 alpha 7: Tuesday, 2025-04-08

@msprotz

alpha 7 is reasonable, alpha 6 is maybe -- does that sound right?

@picnixz

Maybe this temporary workaround is not needed for the alphas?

Depending on how the fix for libintvector turns out to be, it could help a lot the HMAC PR (#130157) that I want to add to the alpha. We only have until May for that because past this point, no features are accepted. And I'd like to include built-in HMAC if possible. If it can solve at the same time this and my issue, then I'd say it should be fixed ASAP.

However, I can also live without a better fix for libintvector. In this case, we can merge this fix and I'll try the alternative I was told (I still didn't have time to come back to that PR though)

@msprotz

I thought it'd be more productive if I tried to push a little on this while we have it all fresh in our heads.

https://github.com/hacl-star/hacl-star/compare/protz_abstract_struct please check this branch out -- none of the public headers include libintvector.h

can you confirm that in its current state, this branch works and eliminates the need for ugly workarounds? it might even eliminate the need to have the pesky configure check in #130213 and #130332

@chris-eibl

I gave it a quick try, but the "reorganizations" happening in refresh.sh made it less quick :)

If I didn't mess up, this seems good to me except some missing HACL_CAN_COMPILE_VEC* guards around Hacl_Streaming_Blake2_Types_two_vec128_s et al in internal/Hacl_Streaming_Types.h.

This is because now (even on hacl-star main) files like Hacl_Hash_SHA*.c include internal/Hacl_Streaming_Types.h and would no longer compile without intrinsics. But that's not a big deal, I've marked the spots in your PR hacl-star/hacl-star#1025.

With those guards applied locally, I was able to compile with clang-cl 18.1.8, which previously failed due to the intrinsics :)

This was referenced

Mar 7, 2025

@chris-eibl

@jmroot #130960 got merged. Can you test whether it works in your case too? Then I think we can close this PR and the issue.

@jmroot

The updated HACL* now in main does indeed seem to do the trick, so this is not needed.