Force SvPV_helper to be always inline #23571
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's usually a bad idea to try to work around a limitation in common code by copy-pasting and then modifiying to taste. Fixes/improvements to the common code rarely get propagated to the outlier. I wrote code in 1ef9039 that did just this for the prototype definition of SvPV_helper, because the place where it really belongs, embed.fnc, couldn't (and still doesn't) handle function pointers as arguments (patches welcome). I should have at least added a comment to the common code noting the existence of this outlier. It turns out that that limitation can be worked around by declaring a typedef of the pointer, and then using that in embed.fnc. That's what this commit does. This commit removes the final instance of duplicating the work of embed.fnc in the core, except for some in the regex system whose comments say the reason is to avoid making a typedef public. I haven't investigated these further.
(when the compiler cooperates, that is) I compiled toke.s using -O0 and looked at the output. It had a full copy of SvPV_helper in it. But this function is called with various constants that make much of the code dead when constant folded. That was not being done here. This commit changes to force inlining. Looking at a revised toke.s, I saw that SvPV_helper was inserted multiple times in the code, and that each occurrence had significantly fewer instructions than the original. I infer that the compiler did the constant folding and pruned out the dead code.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I compiled toke.s using -O0 and looked at the output. It had a full copy of SvPV_helper in it. But this function is called with various constants that make much of the code dead when constant folded. That was not being done here.
This commit changes to force inlining. Looking at a revised toke.s, I saw that SvPV_helper was inserted multiple times in the code, and that each occurrence had significantly fewer instructions than the original. I infer that the compiler did the constant folding and pruned out the dead code for each instance.