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

Avoid calling size() in valarray loops #3968

Merged
merged 3 commits into from Aug 31, 2023

Conversation

AlexGuteniev
Copy link
Contributor

@AlexGuteniev AlexGuteniev commented Aug 15, 2023

Resolves #3915

I've chosen "eliminating indirection helps vectorization" wording that does not mention aliasing.

The correctly looking formula would be "avoid suspected false aliasing" and it is confusing. The wording like "inhibit aliasing" is not quite correct, because the actual aliasing is not happening anyway.

I'm sure that wording mentioning "indirection" and "vectorization" strongly suggests what happening for those who familiar with the problem, and for those who are not, the margin of this variable declaration is too small to fit an explanation.

@AlexGuteniev AlexGuteniev requested a review from a team as a code owner August 15, 2023 08:42
@github-actions github-actions bot added this to Initial Review in Code Reviews Aug 15, 2023
@StephanTLavavej StephanTLavavej added the performance Must go faster label Aug 15, 2023
@StephanTLavavej StephanTLavavej self-assigned this Aug 15, 2023
stl/inc/valarray Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej removed their assignment Aug 16, 2023
@StephanTLavavej StephanTLavavej moved this from Initial Review to Final Review in Code Reviews Aug 16, 2023
@AlexGuteniev
Copy link
Contributor Author

I've also created DevCom-10442357 to optimize even without pre-indirection after more checks, it has some votes, but I'm skeptical, as other compilers miss the vectorization without pre-indirected size too,

stl/inc/valarray Outdated Show resolved Hide resolved
stl/inc/valarray Show resolved Hide resolved
stl/inc/valarray Outdated Show resolved Hide resolved
@CaseyCarter CaseyCarter self-assigned this Aug 29, 2023
@CaseyCarter CaseyCarter moved this from Final Review to Ready To Merge in Code Reviews Aug 29, 2023
@CaseyCarter
Copy link
Member

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

@CaseyCarter CaseyCarter merged commit b091f2a into microsoft:main Aug 31, 2023
35 checks passed
Code Reviews automation moved this from Ready To Merge to Done Aug 31, 2023
@CaseyCarter
Copy link
Member

Thanks for convincing the compiler that size() doesn't change in these loops.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

<valarray>: Consider replacing size() calls with _Mysize
3 participants