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.

Sign up for GitHub

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

Jump to bottom

Fix silent bad codegen for vectorized meow_element() above 4 GB #3619

Merged
merged 6 commits into from
Apr 7, 2023
Merged

Fix silent bad codegen for vectorized meow_element() above 4 GB #3619

merged 6 commits into from
Apr 7, 2023

Conversation

StephanTLavavej
Copy link
Member

@StephanTLavavej StephanTLavavej commented Apr 3, 2023 •
edited
Loading

Fixes #3617.

  • Fix truncation by adding static_cast<size_t>.
    • The bug was lurking in the expression _Base + _V_pos * 16 + _H_pos, where _Base was const char *, _V_pos was unsigned int, and _H_pos was unsigned long.
  • Pre-existing: test_various_containers as we disable instructions.
  • Extract test_min_max_element_support.hpp.
  • Add GH_003617_vectorized_meow_element.
    • This uses the fast_matrix.lst (one configuration, optimized) because it takes a while and consumes a ton of memory if the usual matrix is used. (I originally tried to fuse it into VSO_0000000_vector_algorithms but that was infeasible.)

@StephanTLavavej StephanTLavavej added the bug Something isn't working label Apr 3, 2023
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner April 3, 2023 21:42
StephanTLavavej and others added 2 commits April 4, 2023 00:57
@StephanTLavavej @AlexGuteniev
Use size_t{_V_pos}.
3589b28 
Co-authored-by: Alex Guteniev <gutenev@gmail.com>
@StephanTLavavej
Revert "Use size_t{_V_pos}."
ccba045 
This reverts commit 3589b28.
@StephanTLavavej StephanTLavavej mentioned this pull request Apr 4, 2023
Closed
@StephanTLavavej StephanTLavavej self-assigned this Apr 7, 2023
@StephanTLavavej
Copy link
Member Author

I'm speculatively mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit cb86d7e into microsoft:main Apr 7, 2023
@StephanTLavavej StephanTLavavej deleted the meow_element branch April 7, 2023 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexGuteniev AlexGuteniev AlexGuteniev left review comments

@strega-nil-ms strega-nil-ms strega-nil-ms approved these changes

Assignees

@StephanTLavavej StephanTLavavej

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

<algorithm>: Silent bad codegen for vectorized meow_element() above 4 GB
4 participants
@StephanTLavavej @AlexGuteniev @strega-nil-ms @barcharcraz