Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

[WIP] Helpers to support Intrinsics in SpanHelpers.Char #22875

Closed
wants to merge 4 commits into from

Conversation

benaadams
Copy link
Member

Common helper methods to support the PRs (will base them on this PR)

@benaadams
Copy link
Member Author

@dotnet-bot test OSX10.12 x64 Checked Innerloop Build and Test

@benaadams benaadams force-pushed the char-intrinsics-base branch from ee2897e to b4351bc Compare February 28, 2019 19:43
@benaadams
Copy link
Member Author

@dotnet-bot test OSX10.12 x64 Checked Innerloop Build and Test

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static unsafe nint UnalignedCountVector128(ref char searchSpace)
{
const int elementsPerByte = sizeof(ushort) / sizeof(byte);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: search/repleace elementsPerByte => ElementsPerByte


[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static ref char Add(ref char source, nint elementOffset)
=> ref Unsafe.Add(ref source, (IntPtr)elementOffset);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helper is just to avoid a cast at the call site?

@stephentoub
Copy link
Member

Common helper methods to support the PRs (will base them on this PR)

Could we just include the helpers in the first PR that uses them? It always makes me nervous when we first check in helpers, with a promise that they'll all be used later, as a) it not-infrequently ends up happening that the thing that was going to use them either doesn't materialize or doesn't use all of them, and b) it not-infrequently happens that bugs in the helpers are discovered only when actually used in those subsequent things.

@benaadams benaadams force-pushed the char-intrinsics-base branch from 5c55d3e to 8afe70a Compare April 6, 2019 03:16
@benaadams
Copy link
Member Author

Could we just include the helpers in the first PR that uses them?

Not fussed about this PR being merged; however its 6 PRs that use it so its serving as a common base for me to rebase on.

@benaadams benaadams changed the title Helpers to support Intrinsics in SpanHelpers.Char [WIP] Helpers to support Intrinsics in SpanHelpers.Char Apr 6, 2019
@benaadams
Copy link
Member Author

Will close as have feedback for other PRs

@benaadams benaadams closed this Apr 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants