Support SHA256_Transform_shani() with MSVC

[cmb69]

This requires #15292 to be fixed; I've used the same macro guards as proposed by #15301.

I've only did most minimal cleanup; given that the implementation apparently does not rely on HAVE_FUNC_ATTRIBUTE_TARGET, I've removed the forward declaration and changed the guard.

As is, this does not support compile time __SHA__ detection, because that macro is apparently never defined on Windows. We would need to work around that in confutils.js like with other non standard MSVC macros:

php-src/win32/build/confutils.js

Lines 3341 to 3346 in bd77462

	avail.Add("sse", "__SSE__");
	avail.Add("sse2", "__SSE2__");
	avail.Add("sse3", "__SSE3__");
	avail.Add("ssse3", "__SSSE3__");
	avail.Add("sse4.1", "__SSE4_1__");
	avail.Add("sse4.2", "__SSE4_2__");

However, I wouldn't know where to insert __SHA__; likely this is "non-linear", and as such the whole --enable-native-intrinsics might need to be changed, or an extra configure option be introduced. Probably low priority, since official builds only use SSE(2) anyway.

Note also that the current implementation appears to be suboptimal since there is neither proper support for HAVE_FUNC_ATTRIBUTE_TARGET (which is not available with MSVC anyway), nor for ZEND_INTRIN_*_FUNC_PTR style which is likely supported everywhere.

@TimWolla might want to have a look at this, although for now it's just a draft.

[TimWolla]

I've only did most minimal cleanup; given that the implementation apparently does not rely on HAVE_FUNC_ATTRIBUTE_TARGET, I've removed the forward declaration and changed the guard.

As the CI failure explains: The declaration with the target attribute is necessary for the build to work / for making the intrinsics actually legal to use within the function. Passing the flags to the compiler directly is the "native support".

[cmb69]

As the CI failure explains: The declaration with the target attribute is necessary for the build to work / for making the intrinsics actually legal to use within the function. Passing the flags to the compiler directly is the "native support".

Indeed, but this only works if HAVE_FUNC_ATTRIBUTE_TARGET is defined. A full fledged implementation is more complex. #10436 may serve as reference; especially

4. With runtime detection of AVX2 performed by the module init function. The detection is done by checking the output of CPUID and then a function pointer is set accordingly. In this case, all calls to the UTF-8 validation routine are indirect calls through that function pointer.

would make sense, since that would support environments where HAVE_FUNC_ATTRIBUTE_TARGET is not defined. I assume this is faster than the current way of checking CPU support for each function call, and a bit slower than the HAVE_FUNC_ATTRIBUTE_TARGET style, although perhaps not by much, in which case we could skip the HAVE_FUNC_ATTRIBUTE_TARGET style. Maybe someone knows about the performance difference to expect.

So maybe we should start getting rid of the PHP_HASH_INTRIN_SHA_* defines, and move that to zend_portability.

[TimWolla]

Indeed, but this only works if HAVE_FUNC_ATTRIBUTE_TARGET is defined
since that would support environments where HAVE_FUNC_ATTRIBUTE_TARGET is not defined.

I don't follow here. The attribute is necessary to make the intrinsics available even if -mssse3 -msha is not explicitly passed, because otherwise the build fails. If there is an environment where the intrinsics are always available, without this "safety guardrail", then the SHA-NI implementation can simply be compiled unconditionally.

The CPU check at runtime is orthogonal to that.

I assume this is faster than the current way of checking CPU support for each function call,

I expect the branch to be easily predicted by a CPU, given that is will either always be taken or never be taken. In any case, a single SHA-256 transform still is fairly expensive and I expect it will easily dwarf the type to check the CPU features, as indicated by the 5x improvement that is already achieved with the current very simple implementation.

[cmb69]

I don't follow here. The attribute is necessary to make the intrinsics available even if -mssse3 -msha is not explicitly passed, because otherwise the build fails. If there is an environment where the intrinsics are always available, without this "safety guardrail", then the SHA-NI implementation can simply be compiled unconditionally.

Ah, indeed, I still didn't really understand that, and mixed up __attribute__((target())) and __attribute__(ifunc())). So we don't use the latter (which is fine by me), but still need to support MSVC, what that second commit of mine does, but in a very ugly and hacky way. I'll see if that can be improved.

[TimWolla]

I'll see if that can be improved.

The preprocessor check in hash_sha_ni.c can probably be replaced by:

#if PHP_HASH_INTRIN_SHA_NATIVE || PHP_HASH_INTRIN_SHA_RESOLVER

Then in php_hash_sha.h you would define PHP_HASH_INTRIN_SHA_RESOLVER for Windows and everything probably should work.

[cmb69]

Thanks @TimWolla. Simplified and rebased (to incorporate #15301).

[andypost]

it solved issue with build on Alpine, thank you!

[cmb69]

Rebased, and removed the erroneous ZEND_INTRIN_SSSE3_RESOLVER as noted above.

[cmb69]

Generally, zend_portability defines 4 relevant macros for each intrinsic family:

• ZEND_INTRIN_*_NATIVE (has compile time support)
• ZEND_INTRIN_*_RESOLVER (supports dynamic resolve)
• ZEND_INTRIN_*_FUNC_PROTO (uses function attribute to dispatch)
• ZEND_INTRIN_*_FUNC_PTR (uses a function pointer to dispatch)

In this case we have only PHP_HASH_INTRIN_SHA_NATIVE and PHP_HASH_INTRIN_SHA_RESOLVER; dispatching via a function pointer would not be very useful here, so there's no need for PHP_HASH_INTRIN_SHA_FUNC_PTR, and PHP_HASH_INTRIN_SHA_FUNC_PROTO is implicit in this case.

[arnaud-lb]

Tried in a VM: it uses SHA256_Transform_shani() as expected when the CPU has SSSE3/SHA features (with runtime detection), and the default implementation otherwise.

I get that MSVC can use the sha intrinsics without __attribute__((target)).

Looks good to me!

[TimWolla]

LGTM, except for the unresolved thread in hash_sha_ni.c.