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

vector_algorithms.cpp: turn tails #3963

Closed
AlexGuteniev opened this issue Aug 13, 2023 · 3 comments · Fixed by #4113
Closed

vector_algorithms.cpp: turn tails #3963

AlexGuteniev opened this issue Aug 13, 2023 · 3 comments · Fixed by #4113
Labels
enhancement Something can be improved fixed Something works now, yay!

Comments

@AlexGuteniev
Copy link
Contributor

There are some scalar tails extracted in vector_algorithms.cpp,
Whereas some are used multiple times, some are used just one. A few examples:

return _Minmax_tail<_Mode, typename _Traits::_Signed_t, typename _Traits::_Unsigned_t>(
_First, _Last, _Res, _Sign, _Cur_min_val, _Cur_max_val);

return _Find_trivial_tail(_First, _Last, _Val);

return _Find_trivial_unsized_fallback(_First, _Val);

The tail extraction does not serve any purpose in such cases, it only provides an obstacle for debugging of optimized code (as tails are expectedly inlined. so the debugger does not navigate the lines).

@CaseyCarter CaseyCarter added the enhancement Something can be improved label Aug 14, 2023
@CaseyCarter
Copy link
Member

Checking my understanding: You're suggesting that we should hand-inline tail-called functions with only one callsite?

@AlexGuteniev
Copy link
Contributor Author

Checking my understanding: You're suggesting that we should hand-inline tail-called functions with only one callsite?

Exactly.

@StephanTLavavej
Copy link
Member

We talked about this at the weekly maintainer meeting and are highly in favor of such fusion/hand-inlining, as a stylistic preference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved fixed Something works now, yay!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants