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

[JIT] Naiive implementation of Span Reverse should produce optimal codegen #9577

Open
ahsonkhan opened this issue Jan 19, 2018 · 0 comments
Open
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI enhancement Product code improvement that does NOT require public API changes/additions JitUntriaged CLR JIT issues needing additional triage optimization tenet-performance Performance related issue
Milestone

Comments

@ahsonkhan
Copy link
Contributor

From dotnet/corefx#26381 (comment):

Here is the naiive implementation which uses a single temporary local for swapping elements. This produces sub-optimal disassembly since it contains an unnecessary mov/lea in the loop body.:

public static void Reverse<T>(this Span<T> span)
{
    ref T p = ref MemoryMarshal.GetReference(span);
    int i = 0;
    int j = span.Length - 1;
    while (i < j)
    {
        T temp = Unsafe.Add(ref p, i);
        Unsafe.Add(ref p, i) = Unsafe.Add(ref p, j);
        Unsafe.Add(ref p, j) = temp;
        i++;
        j--;
    }
}

The following implementation results in better disassembly, but we shouldn't expect users to write code in this way without the unnecessary mov/lea instructions within the loop body:

public static unsafe void Reverse<T>(Span<T> span)
{
    if (span.Length > 1)
    {
        ref T p = ref MemoryMarshal.GetReference(span);
        IntPtr i = IntPtr.Zero;
        IntPtr j = (IntPtr)span.Length - 1;

        do
        {
            var temp1 = Unsafe.Add(ref p, i);
            var temp2 = Unsafe.Add(ref p, j);
            Unsafe.Add(ref p, i) = temp2;
            Unsafe.Add(ref p, j) = temp1;

            i += 1;
            j -= 1;
        } while (i.ToPointer() < j.ToPointer());
    }
}

cc @jkotas, @AndyAyersMS, @GrabYourPitchforks

category:cq
theme:basic-cq
skill-level:intermediate
cost:medium

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@BruceForstall BruceForstall added the JitUntriaged CLR JIT issues needing additional triage label Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI enhancement Product code improvement that does NOT require public API changes/additions JitUntriaged CLR JIT issues needing additional triage optimization tenet-performance Performance related issue
Projects
None yet
Development

No branches or pull requests

3 participants