Skip to content

Revert of "Revert the revert of "ColumnVector: optimize filter with AVX512 VBMI2 compress store" #40033"#41752

Merged
alexey-milovidov merged 1 commit intomasterfrom
revert-revert-revert
Sep 26, 2022
Merged

Revert of "Revert the revert of "ColumnVector: optimize filter with AVX512 VBMI2 compress store" #40033"#41752
alexey-milovidov merged 1 commit intomasterfrom
revert-revert-revert

Conversation

@alexey-milovidov
Copy link
Copy Markdown
Member

@alexey-milovidov alexey-milovidov commented Sep 25, 2022

This reverts #40033 due to the issues found in CI:

#41745
#41751

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

@alexey-milovidov
Revert "Merge pull request #40033 from ClickHouse/reenable-avx512-vbm…
e3a6f23 
…i-columnvector-filter"

This reverts commit 70f63d2, reversing
changes made to a0693c3.
This was referenced Sep 25, 2022
Closed
Closed
@robot-ch-test-poll robot-ch-test-poll added the pr-not-for-changelog This PR should not be mentioned in the changelog label Sep 25, 2022
@guowangy
Copy link
Copy Markdown
Contributor

guowangy commented Sep 26, 2022

@alexey-milovidov This should relate to #41247
I will try to find the root cause ASAP.

@guowangy
Copy link
Copy Markdown
Contributor

guowangy commented Sep 26, 2022

The root cause is when limit is zero, the original code will still try to load from data which is zero length.
Patch is working in progress.

inline void vectorIndexImpl(const Container & data, const PaddedPODArray<Type> & indexes, size_t limit, Container & res_data)

@guowangy
Copy link
Copy Markdown
Contributor

guowangy commented Sep 26, 2022

PR: #41765

@alexey-milovidov alexey-milovidov self-assigned this Sep 26, 2022
@alexey-milovidov alexey-milovidov added the pr-must-backport Pull request should be backported intentionally. Use this label with great care! label Sep 26, 2022
@alexey-milovidov alexey-milovidov merged commit 7bb2457 into master Sep 26, 2022
@alexey-milovidov alexey-milovidov deleted the revert-revert-revert branch September 26, 2022 12:16
@alexey-milovidov
Copy link
Copy Markdown
Member Author

alexey-milovidov commented Sep 26, 2022

@guowangy Thank you!
I will first revert, and backport it to 22.9, then we will merge your PR and it will be available from 22.10.

@guowangy
Copy link
Copy Markdown
Contributor

guowangy commented Sep 26, 2022

@alexey-milovidov
But it's not related to #40033, actually #41247 causes the problem.

This was referenced Sep 26, 2022
Closed
Closed
Merged
Merged
@alexey-milovidov
Copy link
Copy Markdown
Member Author

alexey-milovidov commented Sep 26, 2022

Ok. #41797

@robot-clickhouse robot-clickhouse mentioned this pull request Sep 26, 2022
Merged
robot-clickhouse pushed a commit that referenced this pull request Sep 26, 2022
Backport #41752 to 22.8: Revert of "Revert the revert of "ColumnVecto…
c053f99 
…r: optimize filter with AVX512 VBMI2 compress store" #40033"
@robot-clickhouse robot-clickhouse mentioned this pull request Sep 26, 2022
Merged
robot-clickhouse pushed a commit that referenced this pull request Sep 26, 2022
Backport #41752 to 22.9: Revert of "Revert the revert of "ColumnVecto…
a511ff1 
…r: optimize filter with AVX512 VBMI2 compress store" #40033"
@robot-clickhouse robot-clickhouse added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Sep 26, 2022
alexey-milovidov added a commit that referenced this pull request Sep 27, 2022
@alexey-milovidov
Merge pull request #41799 from ClickHouse/backport/22.8/41752
2095e8c 
Backport #41752 to 22.8: Revert of "Revert the revert of "ColumnVector: optimize filter with AVX512 VBMI2 compress store" #40033"
alexey-milovidov added a commit that referenced this pull request Sep 27, 2022
@alexey-milovidov
Merge pull request #41800 from ClickHouse/backport/22.9/41752
be24a17 
Backport #41752 to 22.9: Revert of "Revert the revert of "ColumnVector: optimize filter with AVX512 VBMI2 compress store" #40033"
This was referenced Sep 28, 2022
Closed
Merged
Closed
@alexey-milovidov alexey-milovidov mentioned this pull request Oct 4, 2022
Merged
guowangy added a commit to guowangy/ClickHouse that referenced this pull request Oct 8, 2022
@guowangy
Revert "Merge pull request ClickHouse#41752 from ClickHouse/revert-re…
9c309c9 
…vert-revert"

This reverts commit 7bb2457, reversing
changes made to 78002dc.
nickitat pushed a commit that referenced this pull request Oct 24, 2022
@guowangy
ColumnVector: re-enable AVX512_VBMI/AVX512_VBMI2 optimized filter and…
4e294b9 
… index (#41765)

* Revert "Merge pull request #41752 from ClickHouse/revert-revert-revert"

This reverts commit 7bb2457, reversing
changes made to 78002dc.

* Revert "Merge pull request #41797 from ClickHouse/revert-41247"

This reverts commit 69b13d9, reversing
changes made to 1719e00.

* AVX512VBMI::vectorIndexImpl: Fix heap buffer overflow when limit == 0

* use SIMD_ELEMENTS instead of SIMD_BYTES to avoid confusing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@alexey-milovidov alexey-milovidov

Labels

pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-must-backport Pull request should be backported intentionally. Use this label with great care! pr-not-for-changelog This PR should not be mentioned in the changelog

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@alexey-milovidov @guowangy @robot-clickhouse @robot-ch-test-poll