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

Use int16 min/max for _mm_set_epi16() calls #395

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mholt
Copy link

@mholt mholt commented Feb 23, 2023

This fixes a bug that causes g0 overflows when compiled on Intel processors and run on AMD Ryzen processors.

According to Intel, the _mm_set_epi16() function takes signed shorts (int16): https://www.intel.com/content/www/us/en/docs/intrinsics-guide/index.html#text=mm_set_epi16&ig_expand=6355,6355

But 0-65535 is the range of uint16, not int16; causing an overflow to -1. This is warned of by the compiler: implict conversion from int to short changes value from 65535 to -1

For whatever reason, compiling and running on AMD Ryzen chip succeeded fine. Compiling and runing on an Intel chip also worked fine. But compiling on Intel and running on AMD caused an exception.

Using the int16 range fixes the problem. However, this is not my skill set and I am not sure if this is the right fix. Please verify. Thank you!

This fixes a bug that causes g0 overflows when compiled on Intel processors and run on AMD Ryzen processors.

According to Intel, the `_mm_set_epi16()` function takes signed shorts (int16): https://www.intel.com/content/www/us/en/docs/intrinsics-guide/index.html#text=mm_set_epi16&ig_expand=6355,6355

But 0-65535 is the range of uint16, not int16; causing an overflow to -1. This is warned of by the compiler: `implict conversion from int to short changes value from 65535 to -1`

For whatever reason, compiling and running on AMD Ryzen chip succeeded fine. Compiling and runing on an Intel chip also worked fine. But compiling on Intel and running on AMD caused an exception.

Using the int16 range fixes the problem. However, this is not my skill set and I am not sure if this is the right fix. Please verify. Thank you!
mholt added a commit to mholt/goheif that referenced this pull request Feb 28, 2023
Hopefully. Still need to test and verify it.

See upstream PR: strukturag/libde265#395
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 this pull request may close these issues.

None yet

1 participant