-
-
Notifications
You must be signed in to change notification settings - Fork 438
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
Fix vectorization error on copy algorithm #6567
base: master
Are you sure you want to change the base?
Conversation
0d43825
to
f3746fb
Compare
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
@Pansysk75 Wow, just wow! |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
@hkaiser I've narrowed down the remaining simd fail-case. I might find a way to convince the compiler to produce correct code, as I did above, however it's apparent that some simd directive causes too aggressive optimization and ignores blatant data dependencies. I'd prefer to try find a proper solution for that. |
@hkaiser The solution we discussed (passing data as arguments to Trying to recreate a simpler version of the failing case on Godbolt gives me
Which, to me, hints that Clang tries to do dependency analysis and not allow vectorization that would lead to incorrect results when using |
57a7f65
to
def399f
Compare
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
The checkout test is failing as the Kitware people have just removed the file we're trying to download from their CDash repo (see: Kitware/CDash#2570): Here is the file they removed:
We should add this to our CircleCI testing environment so we don't have to rely on their version of |
Fixed here: #6575 |
def399f
to
f71cc57
Compare
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
@@ -49,7 +49,7 @@ | |||
|
|||
#define HPX_HAVE_VECTOR_REDUCTION | |||
|
|||
#elif (_OPENMP >= 201307) || (defined(__clang__) && HPX_CLANG_VERSION >= 30700) | |||
#elif (_OPENMP >= 201307) && !defined(__clang__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#elif (_OPENMP >= 201307) && !defined(__clang__) | |
#elif (_OPENMP >= 201307) && !defined(HPX_CLANG_VERSION) |
f71cc57
to
3464c07
Compare
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
retest lsu |
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
Fixes bad vectorization that caused copy algorithms with unseq execution to malfunction.
This should fix recent test errors on CI builders with the -fopenmp flag.