Skip to content

BUG: fix compilation error for VS 141 and earlier#21366

Merged
charris merged 1 commit intonumpy:mainfrom
mattip:issue21346
Apr 21, 2022
Merged

BUG: fix compilation error for VS 141 and earlier#21366
charris merged 1 commit intonumpy:mainfrom
mattip:issue21346

Conversation

@mattip
Copy link
Copy Markdown
Member

@mattip mattip commented Apr 20, 2022

Compilers before vs 1922 are missing the _knot_mask16 intrinsic, use a simple NOT instead.

Also clean up a few stray npy_intp/npy_int warnings.

Fixes #21346

I verified the compiler version using https://godbolt.org/z/KMYhzaM1e.

@mattip
Copy link
Copy Markdown
Member Author

mattip commented Apr 20, 2022

The CI runs VS 2019 and will not test the other #ifdef branch. Testing this requires a AVX512 machine + building with @matthew-brett's formula to set the compiler chain to 141:

"C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Auxiliary\Build\vcvarsall.bat" x64 -vcvars_ver=14.16

I had to s/BuildTools/Community and use the Visual Studio installer to install the 14.16 toolchain.

@charris
Copy link
Copy Markdown
Member

charris commented Apr 20, 2022

The test failure looks similar to the previous failures of the cygwin and simd runs, the error message is not informative. I'll try fixing it later.

@serge-sans-paille Does this look right to you.

@serge-sans-paille
Copy link
Copy Markdown
Contributor

serge-sans-paille commented Apr 20, 2022

Looks good to me! https://godbolt.org/z/YohW1Tdd8

@charris
Copy link
Copy Markdown
Member

charris commented Apr 21, 2022

Close/reopen

@charris charris closed this Apr 21, 2022
@charris charris reopened this Apr 21, 2022
charris
charris reviewed Apr 21, 2022
View reviewed changes
numpy/core/src/npysort/x86-qsort.dispatch.cpp
if (arrsize > 1) {
qsort_<vector<npy_int>, npy_int>((npy_int *)arr, 0, arrsize - 1,
2 * log2(arrsize));
2 * (npy_int)log2(arrsize));
Copy link
Copy Markdown
Member

@charris charris Apr 21, 2022

Choose a reason for hiding this comment

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

Hmm. I don't know if this even matters much, but could do (npy_int)(2 * log2(arrsize)));

@charris
Copy link
Copy Markdown
Member

charris commented Apr 21, 2022

Let's give this a shot. I do worry about the complexity of this implementation of quicksort, but that is a bigger problem.

Thanks Matti.

@charris charris merged commit 03d6e82 into numpy:main Apr 21, 2022
@mattip mattip deleted the issue21346 branch December 27, 2022 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@charris charris charris left review comments

Assignees

No one assigned

Labels

00 - Bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

BUG: error compiling on MSVC 141 toolchain

3 participants

@mattip @charris @serge-sans-paille