Skip to content

mono AOT compiler cannot resolve and inline static virtual calls in gshared methods #75801

Closed

Description

@vargaz

Testcase:

using System;
using System.Diagnostics;
using System.Numerics;

public class Tests
{
        private interface INegator<T> where T : struct
        {
            static abstract bool NegateIfNeeded(bool equals);
        }

        private readonly struct DontNegate<T> : INegator<T> where T : struct
        {
            public static bool NegateIfNeeded(bool equals) => equals;
        }

        private readonly struct Negate<T> : INegator<T> where T : struct
        {
            public static bool NegateIfNeeded(bool equals) => !equals;
        }

    private static int IndexOfValueType<TValue, TNegator>()
            where TValue : struct, INumber<TValue>
            where TNegator : struct, INegator<TValue>
    {
        for (int j = 0; j < 1000; ++j) {
            for (int i = 0; i < 100000; ++i)
                TNegator.NegateIfNeeded (true);
        }
        return 0;
    }

    public static void Main () {
        var w = Stopwatch.StartNew ();
        IndexOfValueType<byte, DontNegate<byte>> ();
        w.Stop ();
        Console.WriteLine (w.ElapsedMilliseconds);
    }
}

When calling IndexOfValueType<byte, DontNegate<byte>>, the AOT compiler will compile a shared instance for bytes/enums with byte basetype, but inside the shared method it can't resolve the

constrained. !!TNegator
 call       bool class Tests/INegator`1<!!TValue>::NegateIfNeeded(bool)

call to DontNegate so it doesn't get inlined.

This is hit with the recent BCL changes to SpanHelpers.

Activity

  1. ghost added
    untriagedNew issue has not been triaged by the area owner
    on Sep 17, 2022
  2. added and removed
    untriagedNew issue has not been triaged by the area owner
    on Sep 17, 2022
  3. self-assigned this
    on Sep 18, 2022
  4. SamMonoRT commented on Sep 19, 2022

    @SamMonoRT
    Member
  5. ghost added
    in-prThere is an active PR which will close this issue when it is merged
    on Sep 19, 2022
  6. added this to the 8.0.0 milestone on Sep 19, 2022
  7. adamsitnik commented on Sep 19, 2022

    @adamsitnik
    Member

    @vargaz As you know I am working on the 7.0 fix and I wonder whether I could avoid copying the old code back. I have two questions:

    1. Would constraining T help?
    - private interface INegator<T> where T : struct
    + private interface INegator<T> where T : struct, INumber<T>
    1. Would using typeof instead help?
    - equals = TNegator.NegateIfNeeded(Vector256.Equals(values, Vector256.LoadUnsafe(ref currentSearchSpace)));
    + equals = Vector256.Equals(values, Vector256.LoadUnsafe(ref currentSearchSpace));
    + if (typeof(TNegator) == typeof(Negate)) // assuming I would remove T from INegator
    + {
    +     equals = ~equals;
    + }
    
  8. vargaz commented on Sep 19, 2022

    @vargaz
    ContributorAuthor

    I think the lowest risk for net 7.0 is to copy the code back and put it inside ifdefs.

  9. adamsitnik commented on Sep 19, 2022

    @adamsitnik
    Member

    I think the lowest risk for net 7.0 is to copy the code back and put it inside ifdefs.

    @vargaz I assume that it means that the two workarounds posted above would not help? I have started copying the code and it's a lot of code (hundreds of lines) so it's not minimal risk.

  10. jkotas commented on Sep 19, 2022

    @jkotas
    Member

    You can add copy of the old implementation files verbatim under a different name, and just do minimal changes on top of it to get things compile.

  11. ghost removed
    in-prThere is an active PR which will close this issue when it is merged
    on Sep 27, 2022
  12. adamsitnik commented on Sep 27, 2022

    @adamsitnik
    Member

    @vargaz Can I revert #75917 now?

  13. vargaz commented on Sep 27, 2022

    @vargaz
    ContributorAuthor

    This only solves (part of) the AOT slowdowns, the interpreter slowdowns are still there.

  14. vargaz commented on Sep 28, 2022

    @vargaz
    ContributorAuthor

    So for a simple test case:

            var arr = new int [100000];
            var w = Stopwatch.StartNew ();
            var span = new Span<int> (arr);
            for (int i = 0; i < 100000; ++i)
                span.IndexOf ((int)50);
            w.Stop ();
            Console.WriteLine (w.ElapsedMilliseconds);
    
    • Without the span revert: 80s
    • Without span revert + this PR: 8.4s
    • With span revert: 8s

    So there is still about a 5% slowdown, but it's not that bad anymore.

  15. ghost locked as resolved and limited conversation to collaborators on Nov 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions