Refactor subrange assertion generation and propagation

[SingleAccretion]

This PR adds a new abstraction for dealing with casts, IntegralRange, a tuple of two symbolic values that represent the lower and upper bounds for an integral value. The symbolic representation has been chosen so that the range can be efficiently used on 32 hosts, since the alternative (using 64 bit integers) would have been, I feel, unacceptably inefficient. Notably, the range always interprets values produces by IR nodes as signed (since we're already using the signed semantic in a number of places, and it is easier to reason about for the purposes of casts as using unsigned representation would have meant a discontinuous range).

Because assertion propagation was using integral ranges already, that's where IntegralRange is used in this PR. This enables reasoning about long checked casts on 32 bit hosts (rare scenario) and ensures the correctness of how the assertions are interpreted, since they're created for both the cast's node output (in local assertion propagation) and input (in global assertion propagation). It also allows for, in the future, shrinking the size of the AssertionDsc structure. I note that since we're allocating a pretty sizeable array of them upfront in global assertion propagation (3K+ bytes), any savings in the size here translate into large percentages in memory allocation reduction (e. g., removing 8 bytes from the AssertionDsc, I've observed -2.2% reduction, and I think we can reduce the size by at least 20 bytes), though this is somewhat misleading as the memory is only actually used for active assertions. Note that this PR does not alter the size of AssertionDsc, the added if !defined(HOST_64BIT) is for clarity only.

Naturally, code propagating the assertions for casts has also been refactored, mostly to avoid retyping a long local to int if it happened to have a range of int. Such retyping is redundant with what optNarrowTree does already, and is in fact incorrect in global assertion propagation as it did not update the value number properly (leaving a node that has TYP_INT with a VN of TYP_LONG). Also, the retyping has been restricted to the known case where it is needed (and a comment added on why it is needed).

An interesting property of the old code was how it treated TYP_BOOL. TYP_BOOL in the compiler is pretty much a TYP_UBYTE, but with a hint that it comes from CORINFO_TYPE_BOOL. Assertion generation treated assignments with a TYP_BOOL source as those restricting the destination to a [0..1] range, however, that is incorrect as per the IL semantics of booleans being treated as equivalent to uint8s. This did not have ill effects except for perhaps dropping a checked cast once in a while, but it was an internal inconsistency in the IR. This PR just makes TYP_BOOLs equivalent to TYP_UBYTEs for the purposes of assertion generation.

(Note: the text below is an explanation for the quirk)

Another interesting property of the old code was that it did not propagate the assertion for casts to TYP_BOOL (perhaps hinting at the reason for why it was "safe" to give TYP_BOOL [0..1] ranges in the first place...). The new code does, by virtue of not special casing it, and that is where the bulk of the diffs (win-x64, win-x86) come from.

Unfortunately, there are, though a small number of, regressions with this handling of TYP_BOOL. There are two sources of them.

First, there is the VN problem, it accounts for the vast majority of the regressions (missing CSEs + missed redundant branch elimination). Say we have the following situation:

int Function(short a)
{
    a = (sbyte)a;
    
    if (a is 1)
        return 1;

    return a;
}

Today, local assertion propagation removes the normalizing cast for a is 1. Then in VN we value number the local without the cast and the normalizing cast for the same local differently:

***** BB01, STMT00002(after)
N004 (  6,  6) [000009] ------------              *  JTRUE     void  
N003 (  4,  4) [000008] N------N-U--              \--*  NE        int    $102
N001 (  2,  2) [000006] ------------                 +--*  LCL_VAR   short  V00 arg0         u:2 $101
N002 (  1,  1) [000007] ------------                 \--*  CNS_INT   int    1 $44


***** BB03, STMT00003(after)
N003 (  4,  5) [000011] ------------              *  RETURN    int    $140
N002 (  3,  4) [000016] ------------              \--*  CAST      int <- short <- int $103
N001 (  2,  2) [000010] ------------                 \--*  LCL_VAR   int    V00 arg0         u:2 (last use) $101

The core issue here is that the VNs for "normalize-on-load" locals are fundamentally "wrong". In the example above, both [000008] and [000010] have the same VN, but say V00's home was on the stack. The first tree would have produced 2 bytes of the value it was assigned, sign-extended, while the second would produce a full int, with the upper 2 bytes set to whatever happened to be next to V00 on the stack (this is how it actually works - the width of the local node signifies to the codegen how wide the load should be). Because of this representation problem, fixing this appears to be not so straightforward.

One possible fix is to recognize, in VN, when a cast a redundant. That runs into regressions with "false CSEs", since we leave the cast tree intact, while in actuality it is redundant and CSEing such a tree has negative profitability (but CSE cannot know that because we leave the costs intact as well).

Another possible fix is to turn these casts into no-ops while renaming the uses (ideally we'd be doing that in early prop, but that's much more expensive TP-wise). That runs into regressions because, sometimes, removing a cast inhibits redundant branch optimization. Consider the following:

// Say p0 is a `short` parameter.
if (p0 != 0) // here, the normalizing cast cannot be removed.
{
    p1 = p0;
   
    if (p1 != 0) // here, the normalizing cast can be removed.
    {
    }
}

// In this example, the two conditions will have different VNs if we remove the cast above p1.

I have spent a considerable amount of time trying out different approaches to fixing the above issues, but they all ran into various regressions, so I tabled fixing it in this PR.

The second source of regressions is the fact that in lowering we can recognize EQ(CAST(bool/ubyte <- int), SMALL_CONST) and retype the tree to UBYTE, leading to, effectively, containment for the cast. Expanding this to consider small LCL_VARs is not so trivial as well, because it can regress when we have those locals assigned RSI, RDI, RBP or RSP, which take up more space to encode in their byte form on Amd64, not to mention we can end up with byte-able register constraints on x86.

Due to the above issues and the fact that I was not able to mitigate them, I will be quirking the handling of TYP_BOOL in this PR to get to a near zero substantial regressions state.

Fixes #54842, but properly.
Fixes #12936

[kunalspathak]

@SingleAccretion - Is it possible to narrow this PR to just the fix for #54842 vs. improvement? We will take the fix for .NET6 and improvement can be merged in future milestone.

[SingleAccretion]

I will see what can be done.

[SingleAccretion]

With the quirk in place, this PR is ready for review.

Updated diffs: win-x64, win-x86, win-arm64, linux-arm.

Now almost all are improvements, with the TYP_BOOL handling accounting for the vast majority of them, and the fact that we do not generate useless subrange assertions for just a couple overly large methods in coreclr_tests.

The only regressions (including the rather large NormalizeProtoMember one on x86...) are register allocation + frame zeroing differences, so, while the "actual" diff may be this:

-       movzx    rdx, sil
-       mov      r8, gword ptr [rbp-18H]
+       mov      r8, gword ptr [rbp-20H]

LSRA now decides to use a callee-saved register, or the prolog zeroing code gets a bit larger while the frame is smaller, etc. This can also happen due to new copy propagations.

[EgorBo]

cc @dotnet/jit-contrib

[AndyAyersMS]

I like this refactoring, but I'm confused on a few points.

[AndyAyersMS]

Can you explain this case a bit more?

[AndyAyersMS]

Might as well use gtEffectiveVal here, since you're changing this bit.

[AndyAyersMS]

Seems like we might want ULongMax in here too, for completeness?

[AndyAyersMS]

Ditto here, why is this not [0..UINT_MAX] ?

[SingleAccretion]

Can you explain this case a bit more?

Seems like we might want ULongMax in here too, for completeness?

Ditto here, why is this not [0..UINT_MAX] ?

A very intentional part of the design here is that the IntegralRange is always interpreted as signed, it makes it easier to reason about consistently (in the time I've spent tweaking the cast handling, I've come to believe this is the best approach).

This makes ULongMax an impossible value, since we do not have Int128 integers.

This makes a cast like CAST_OVF(ulong <- uint) - [INT_MIN..INT_MAX] have the range it has:

1. For which values does the cast overflow?
2. No such values exist -> the full range of an integer is the "input" range.
3. The range is signed -> it is [INT_MIN..INT_MAX].

[AndyAyersMS]

A very intentional part of the design here is that the IntegralRange is always interpreted as signed

Ok, good. It is probably worth mentioning this in comments somewhere -- maybe in the header comment for IntegralRange.

Let me test my understanding. In ForCastInput we have just one case that uses a UINT symbolic bound

// CAST_OVF(uint <- ulong/long) - [0..UINT_MAX]

So we'd say -- yes this can overflow. Valid input range is indeed [0...UINT_MAX]. Interpreting this as signed still leaves [0...UINT_MAX].

[AndyAyersMS]

#58776 changed how address exposure is handled and is causing your builds to break.

[SingleAccretion]

changed how address exposure is handled and is causing your builds to break.

Yes, will be rebasing...

Ok, good. It is probably worth mentioning this in comments somewhere -- maybe in the header comment for IntegralRange.

Will write something more substantial than what is in the header of ForCastInput.

[AndyAyersMS]

Looks good.

Thanks again for your high-quality contributions.