search

Cyan4973 / xxHash

Extremely fast non-cryptographic hash algorithm
http://www.xxhash.com/
Other
9.14k stars 777 forks source link

MSVC warning C4113 in xxh_x86dispatch.c #959

Closed GrahamKnapATS closed 1 month ago

GrahamKnapATS commented 3 months ago

With xxHash v0.8.2 and Visual Studio 2022 17.10, I receive C4113 warnings in xxh_x86dispatch.c.

The problem appears to be similar to #716 and can be resolved by annotating the input and secret parameters with XXH_RESTRICT, for the functions:

Cyan4973 commented 3 months ago

I wonder if we could reproduce this issue in CI ? This would help to observe, fix and then keep this problem under surveillance.

We already have VS2022 tests in Github CI : https://github.com/Cyan4973/xxHash/actions/runs/9964304949/job/27532289053 but they do not flag any warning, could it be a matter of varying some compilation parameters ?

GrahamKnapATS commented 3 months ago

It looks like CI is not making an MSVC build with DISPATCH enabled. The MSVC build output does not mention xxh_x86dispatch.c. Unfortunately, I don't know how to add that case to your CI. However, I can confirm that if I open the CMake project in VS2022, and add -DDISPATCH=1 to "CMake command arguments", then the warnings occur.

Cyan4973 commented 2 months ago

From what I can tell, the Visual test on Github Actions does feature a test with DISPATCH enabled : https://github.com/Cyan4973/xxHash/actions/runs/9964304949/job/27532289053#step:7:1

However, there are also multiple back ends available in Visual nowadays. So maybe you are using a different one than this test?

Cyan4973 commented 2 months ago

I could reproduce the issue in CI by employing a different back end

GrahamKnapATS commented 2 months ago

From what I can tell, the Visual test on Github Actions does feature a test with DISPATCH enabled : https://github.com/Cyan4973/xxHash/actions/runs/9964304949/job/27532289053#step:7:1

However, there are also multiple back ends available in Visual nowadays. So maybe you are using a different one than this test?

Correct. I'm using MSVC, not clang.

Cyan4973 commented 2 months ago

Thanks for the report @GrahamKnapATS , there is now a test that checks compilation of xxhsum with DISPATCH=1 using Visual compiler backend in CI.

It finds the same warning issues as you reported. They are now fixed in #963 .