Fix IMGREL32 static field addr value-num blindspot

[JosephTremoulet]

When fgMorphField lowers a static field reference for which
eeGetRelocTypeHint returns IMAGE_REL_BASED_REL32, it creates an
IntCon, flagged with GFT_ICON_STATIC_HDL and marked with the static
field in its gtFieldSeq field, to represent the field's address (as
opposed to the normal case where it generates a ClsVar). This change
updates the IsFieldAddr helper used by value-numbering/loop-hoisting to
recognize such constants as the appropriate field address, which then
allows the disambiguation/tracking of accesses to these fields to kick in
as it does for ClsVar nodes.

[JosephTremoulet]

@dotnet/jit-contrib PTAL. SPMI reports only one asm diff, and jit-diff reports none, but I think that's really due to eeGetRelocTypeHint not returning IMAGE_REL_BASED_REL32 in crossgen/ngen. I noticed this issue looking into a report of us not hoisting invariant bounds checks from this code:

using System;
using System.Diagnostics;

class ArrayPerf
{

    private static int[] m_arr;
    private const int MAX_ARRAY_SIZE = 4096;

    private static readonly int DIM_1 = MAX_ARRAY_SIZE;

    static void Main(string[] args)
    {
        long iterations = Int64.Parse(args[0]);

        int value;
        m_arr = new int[DIM_1];

        for (int i = 0; i < DIM_1; i++)
            m_arr[i] = i;

        for (int j = 0; j < DIM_1; j++)
            value = m_arr[j];

        //var sw = Stopwatch.StartNew();

        for (long i = 0; i < iterations; i++)
        {
            for (int j = 0; j < DIM_1; j++)
            {
                value = m_arr[j];
                value = m_arr[j];
                value = m_arr[j];
                value = m_arr[j];
                value = m_arr[j];
                value = m_arr[j];
                value = m_arr[j];
                value = m_arr[j];
                value = m_arr[j];
                value = m_arr[j];
            }
        }

        //sw.Stop();
        //Console.WriteLine(sw.ElapsedMilliseconds);
    }
}

When compiled duringCoreRun rather than crossgen, all those m_arr accesses become redundant loads without this change, and get CSEd correctly with this change.

[briansull]

If this is a static field and isFieldStatic(newFldSeq->m_fieldHnd)) returns true then you
may need to set baseAddr to 'this' I think as returning both pObj and pStatic as nullptr could be a problem.

Look into this area I can't tell for sure.
The method comment says it isn't allowed, but valuenum 5822 looks like it handles this case.

But if returning both as nullptr is what you want here, then update the comment in the method declaration.

[JosephTremoulet]

Yes, thanks, I think you're right and pStatic should be set in this case. Updated.

[JosephTremoulet]

@dotnet-bot retest Ubuntu x64 Checked Build and Test

[JosephTremoulet]

@dotnet-bot test OSX x64 Checked Build and Test

[JosephTremoulet]

@dotnet-bot test Linux ARM Emulator Cross Release Build

[JosephTremoulet]

Test GC/Scenarios/FinalizeTimeout was failing because it has a side-effect-free busy-loop checking a static variable for a change it expects a finalizer to make to it; with this change the jit recognizes the load as invariant and hoists it out of the loop, I've fixed the test by marking the static field volatile.

@briansull PTAL.

[JosephTremoulet]

@briansull, any more concerns with this change? I'm not entirely sure what the callers handling a return of two nullptrs are trying to do, but at any rate my intent was to match the ClsVar case and so I set pStatic as you suggested. I seem to get the same diffs whether or not pStatic is set.

[briansull]

Looks Good