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

Replace tag dispatch #375

Closed
jk-jeon opened this issue Feb 14, 2023 · 5 comments
Closed

Replace tag dispatch #375

jk-jeon opened this issue Feb 14, 2023 · 5 comments

Comments

@jk-jeon
Copy link
Contributor

jk-jeon commented Feb 14, 2023

There are many functions that are likely end up as function call boundaries. Tag dispatch is not a zero-cost abstraction unless inlining takes place, and I think tag dispatch should be generally only used for simple forwarding functions. It is hardly likely that those little added overhead will cause any measurable slowdown of course, but I see no reason to insist on tag dispatches either, so why not.

It seems to be not so simple to turn the current instances of tag dispatches into forwarding, but you can also replace tag dispatches by template specializations anyway. Of course you can't partially specialize function templates, but you can simply move those functions inside a dummy class templates and specialize the template parameters for those dummy class templates instead.

@beached
Copy link
Owner

beached commented Mar 7, 2023

There are two major places that use it, one is always inlined and does a if constexpr over the tag to dispatch to the others. It could probably be switched to a template param and drop the function parameter but probably doesn't cost much at runtime. Probably should be remove that func param though.

The other, the exec tag uses a small linear hierarchy of to find the best overload where a simd exec tag is a runtime exec tag is a constexpr exec tag. This is/has been used to work around optimization possibilities in constexpr code and gracefully fallback when there is no alternative. When/if C++20 becomes the minimum a bunch of this is simplified though.

@beached
Copy link
Owner

beached commented Jul 1, 2024

I think this has been fixed. Or the sub functions are not using tag invoke where the genericness and open set to call isn't needed any longer.

@beached beached closed this as completed Jul 1, 2024
@jk-jeon
Copy link
Contributor Author

jk-jeon commented Jul 2, 2024

That's great, thanks!

Just a small comment: So according to the msvc ABI, tag dispatch is definitely not zero-cost. On the other hand, gcc/clang both optimize them away completely, regardless of where are the actual call boundaries. But I failed to find any documentation explaining this behavior, and my understanding of their ABI document is that they shouldn't optimize this away... Weird

@beached
Copy link
Owner

beached commented Jul 2, 2024

I think MS Win ABI requires it for all user type https://learn.microsoft.com/en-us/cpp/build/x64-calling-convention?view=msvc-170 . It says all 1, 2, 4, 8 byte sized classes can be passed in a register, otherwise they are passed by reference(to be copied I assume). Yeah, not passing empty would be a smart move in hindsight.

In the JSON Link case, parse_value was being used like that but now just uses named functions. There are a couple left, template_param<T> but they had a bunch of help with debugging large callstacks as I could keep the important params up high. I would consider removing those in the future or moving them to the end as MSVC does only have 4 registers available and that would put it on the stack or in a register if available.

@beached
Copy link
Owner

beached commented Aug 5, 2024

I have removed all the usages of tag types. If I missed any, I'll remove them. the debugging usage is no longer useful as my tooling is better and this free's up registers/stack on some platforms

Thanks for pointing this out

@beached beached closed this as completed Aug 5, 2024
beached added a commit that referenced this issue Aug 5, 2024
Removed usage of tag types to pass template params.  #375
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants