Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSVC with AVX2 compile option, C4244 warnings. #3489

Closed
ghost opened this issue Feb 10, 2023 · 2 comments · Fixed by #3495
Closed

MSVC with AVX2 compile option, C4244 warnings. #3489

ghost opened this issue Feb 10, 2023 · 2 comments · Fixed by #3495

Comments

@ghost
Copy link

ghost commented Feb 10, 2023

If build with AVX2 instruction set, MSVC has dozens of C4244 warnings:
warning C4244: 'return': conversion from 'unsigned __int64' to 'unsigned int', possible loss of data

MEM_STATIC unsigned ZSTD_countTrailingZeros64(U64 val)
{
    assert(val != 0);
#   if defined(_MSC_VER) && defined(_WIN64)
#       if STATIC_BMI2 == 1
            return _tzcnt_u64(val);
#       else

Adding (unsinged) before _tzcnt_u64(val) should fixes this issue.

@Cyan4973
Copy link
Contributor

Maybe we should also update our Windows CI tests,
so that they can catch such issues.

@ghost
Copy link
Author

ghost commented Feb 10, 2023

Maybe we should also update our Windows CI tests,

IMO no need, these codes will not be changed for a long time.

Maybe refactor the architecture-related code into one file, such as clz/ctz/memcpy_16bytes code, this is handy for porting to new architecture.
PR #2831 tried to refactor clz/ctz code, it removes some similar code that spread everywhere.

@ghost ghost changed the title [good first PR] MSVC with AVX2 compile option, C4244 warnings. MSVC with AVX2 compile option, C4244 warnings. Feb 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant