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

V0ldek/simd target perf #305

Merged
merged 11 commits into from
Oct 13, 2023
Merged

V0ldek/simd target perf #305

merged 11 commits into from
Oct 13, 2023

Conversation

V0ldek
Copy link
Member

@V0ldek V0ldek commented Oct 11, 2023

Short description

Major change to the SIMD resolution macros.

Analysis of generated assembly for v0.8.0 shows multiple places where the transition to portable binaries caused critical functions to not be inlined/resolved as intrinsics. The biggest culprit is popcnt, which seems to be never inlined in the depth classifier.

Following analysis, we introduce an additional step into the resolution mechanism. After obtaining a ResolvedSimd with all generic parameters bound to concrete SIMD implementations, we allow for another macro-based dispatch to construct a local function with relevant target_feature annotations and pass the environment inside via arguments. Since ResolvedSimd is already resolved statically, this doesn't introduce code bloat into the final binary, but allows the compiler to see the target_feature annotations when generating code for that function. By introducing that dispatch to the three topmost "entry points" of the engine (run_on_subtree, run_head_skipping, skip (tail-skipping)) we ensure all downstream intrinsics are properly resolved and inlined.

The result is a massive throughput increase of 5, 10, 20, or in case of google_map::travel_modes/rsonpath_direct_count 59 (fifty-nine) percent.

Issue

This was done as part of work on #276

Checklist

All of these should be ticked off before you submit the PR.

  • I ran just verify locally and it succeeded.
  • Issue was given go ahead and is linked above OR I have included justification for a minor change.
  • Unit tests for my changes are included OR no functionality was changed.

@V0ldek V0ldek merged commit 56472ed into main Oct 13, 2023
46 checks passed
@V0ldek V0ldek deleted the v0ldek/simd-target-perf branch October 13, 2023 19:57
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

Successfully merging this pull request may close these issues.

1 participant