RyuJIT: Loop hoist invariant struct field accesses

[sivarv]

Knucleotide benchmark heavily uses Dictionary lookups. It uses the following struct as key into hash table

struct ByteString
{
   public byte[] Array
   public int Start;
   public int Length;

    public override int GetHashCode()
    {
        int hc = 0;
        for (int i = 0; i < Length; i++)
            hc = hc * 31 + Array[Start + i];
        return hc;
    }

    public bool Equals(ByteString other)
    {
        if (Length != other.Length) return false;
        for (int i = 0; i < Length; i++)
            if (Array[Start+i] != other.Array[other.Start+i]) return false;
        return true;
    }
}

Call Chain:
Dictionary.TryGetValue() --> FindEntry() --> calls GetHashCode() and Equals() methods on ByteString.

In case of Equals() or GetHashCode() methods, ByteString accessed is either "thisptr" or argument passed to the method. Struct args on Windows are considered implicitly by-ref and hence struct fields get accessed through an indirection on a byref. Legacy JIT64 was able to loop hoist this.Array, this.Array.Length, this.Start, other.Array, other.Array.Length and other.Start by recognizing them as loop invariant. This shows up as execution perf difference between RyuJIT vs Legacy Jit64.

category:cq
theme:loop-opt
skill-level:expert
cost:medium

[sivarv]

The reason RyuJIT doesn't hoist the field accesses is that these reads from By-ref through indir are given unique value numbers. Note that these methods (Equals and GetHashCode) are pure methods in the sense that they only read from heap and no writes to it.

[sivarv]

Please note that ByteString struct used by knucleotide benchmark is similar in concept to Span<T>. Getting this right for ByteString would also likely to benefit Span<T> scenarios where Span<T> accessed is an argument.

[JosephTremoulet]

The changes I have in flight to address #6913 currently produce these diffs on these methods. Looks like the only loads we're leaving in the loops are the variant ones and the array lengths in the bounds-checks. I'll look into why we're not getting those array lengths...

[JosephTremoulet]

The failure to hoist the array lengths is an instance of #6554; opt repeat with count of 3 hoists them out of the loop (and with count of 2 eliminates the extra reg-reg copy/ies in the loop), just like the repro case at the bottom of #6554.