Disable JIT xarch asserts on gcref/byref computation (#56192)

The emitter has asserts that an xarch RMW inc/dec/add/sub of a byref
must have an incoming gcref/byref to be legal. This is (no longer)
true due to extensive use of Span and Unsafe constructs, where we
often see lclheap or other integer typed values cast to byref. Also,
the emitter only updates its GC info when an instruction is generated.
When one of these casts from integer to byref ends up getting the same
register allocated, and its instruction is thus omitted, the emitter
doesn't get the appropriate gcref update (this problem is being
attempted to be solved elsewhere).

For now, disable these asserts.

Re-enable the tests disabled in #54207

Fixes #51728, #54007

diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp
index c35a667..6e0de0d 100644 (file)
--- a/src/coreclr/jit/emitxarch.cpp
+++ b/src/coreclr/jit/emitxarch.cpp
@@ -11914,10 +11914,12 @@ BYTE* emitter::emitOutputR(BYTE* dst, instrDesc* id)
 #endif
             if (id->idGCref())
             {
-                // The reg must currently be holding either a gcref or a byref
-                // and the instruction must be inc or dec
-                assert(((emitThisGCrefRegs | emitThisByrefRegs) & regMask) &&
-                       (ins == INS_inc || ins == INS_dec || ins == INS_inc_l || ins == INS_dec_l));
+                assert(ins == INS_inc || ins == INS_dec || ins == INS_inc_l || ins == INS_dec_l);
+                // We would like to assert that the reg must currently be holding either a gcref or a byref.
+                // However, we can see cases where a LCLHEAP generates a non-gcref value into a register,
+                // and the first instruction we generate after the LCLHEAP is an `inc` that is typed as
+                // byref. We'll properly create the byref gcinfo when this happens.
+                //     assert((emitThisGCrefRegs | emitThisByrefRegs) & regMask);
                 assert(id->idGCref() == GCT_BYREF);
                 // Mark it as holding a GCT_BYREF
                 emitGCregLiveUpd(GCT_BYREF, id->idReg1(), dst);
@@ -12222,15 +12224,30 @@ BYTE* emitter::emitOutputRR(BYTE* dst, instrDesc* id)
                     case INS_sub:
                         assert(id->idGCref() == GCT_BYREF);
 
+#if 0
 #ifdef DEBUG
+                        // Due to elided register moves, we can't have the following assert.
+                        // For example, consider:
+                        //    t85 = LCL_VAR byref V01 arg1 rdx (last use) REG rdx
+                        //        /--*  t85    byref                                                       
+                        //        *  STORE_LCL_VAR byref  V40 tmp31 rdx REG rdx                 
+                        // Here, V01 is type `long` on entry, then is stored as a byref. But because
+                        // the register allocator assigned the same register, no instruction was
+                        // generated, and we only (currently) make gcref/byref changes in emitter GC info
+                        // when an instruction is generated. We still generate correct GC info, as this
+                        // instruction, if writing a GC ref even through reading a long, will go live here.
+                        // These situations typically occur due to unsafe casting, such as with Span<T>.
+
                         regMaskTP regMask;
                         regMask = genRegMask(reg1) | genRegMask(reg2);
 
                         // r1/r2 could have been a GCREF as GCREF + int=BYREF
-                        //                            or BYREF+/-int=BYREF
+                        //                               or BYREF+/-int=BYREF
                         assert(((regMask & emitThisGCrefRegs) && (ins == INS_add)) ||
                                ((regMask & emitThisByrefRegs) && (ins == INS_add || ins == INS_sub)));
-#endif
+#endif // DEBUG
+#endif // 0
+
                         // Mark r1 as holding a byref
                         emitGCregLiveUpd(GCT_BYREF, reg1, dst);
                         break;

diff --git a/src/libraries/System.IO.Hashing/tests/Crc32Tests.cs b/src/libraries/System.IO.Hashing/tests/Crc32Tests.cs
index 4e5ac87..6d2993f 100644 (file)
--- a/src/libraries/System.IO.Hashing/tests/Crc32Tests.cs
+++ b/src/libraries/System.IO.Hashing/tests/Crc32Tests.cs
@@ -93,7 +93,6 @@ namespace System.IO.Hashing.Tests
             InstanceAppendAllocateAndResetDriver(testCase);
         }
 
-        [SkipOnCoreClr("https://github.com/dotnet/runtime/issues/54007", RuntimeConfiguration.Checked)]
         [Theory]
         [MemberData(nameof(TestCases))]
         public void InstanceMultiAppendGetCurrentHash(TestCase testCase)

diff --git a/src/libraries/System.IO.Hashing/tests/Crc64Tests.cs b/src/libraries/System.IO.Hashing/tests/Crc64Tests.cs
index 546501a..d647e4b 100644 (file)
--- a/src/libraries/System.IO.Hashing/tests/Crc64Tests.cs
+++ b/src/libraries/System.IO.Hashing/tests/Crc64Tests.cs
@@ -97,7 +97,6 @@ namespace System.IO.Hashing.Tests
             InstanceAppendAllocateAndResetDriver(testCase);
         }
 
-        [SkipOnCoreClr("https://github.com/dotnet/runtime/issues/54007", RuntimeConfiguration.Checked)]
         [Theory]
         [MemberData(nameof(TestCases))]
         public void InstanceMultiAppendGetCurrentHash(TestCase testCase)

diff --git a/src/libraries/System.IO.Hashing/tests/XxHash32Tests.007.cs b/src/libraries/System.IO.Hashing/tests/XxHash32Tests.007.cs
index cf2e967..eed5aec 100644 (file)
--- a/src/libraries/System.IO.Hashing/tests/XxHash32Tests.007.cs
+++ b/src/libraries/System.IO.Hashing/tests/XxHash32Tests.007.cs
@@ -105,7 +105,6 @@ namespace System.IO.Hashing.Tests
             InstanceAppendAllocateAndResetDriver(testCase);
         }
 
-        [SkipOnCoreClr("https://github.com/dotnet/runtime/issues/54007", RuntimeConfiguration.Checked)]
         [Theory]
         [MemberData(nameof(TestCases))]
         public void InstanceMultiAppendGetCurrentHash(TestCase testCase)

diff --git a/src/libraries/System.IO.Hashing/tests/XxHash32Tests.cs b/src/libraries/System.IO.Hashing/tests/XxHash32Tests.cs
index 15c671d..f9dccad 100644 (file)
--- a/src/libraries/System.IO.Hashing/tests/XxHash32Tests.cs
+++ b/src/libraries/System.IO.Hashing/tests/XxHash32Tests.cs
@@ -118,7 +118,6 @@ namespace System.IO.Hashing.Tests
             InstanceAppendAllocateAndResetDriver(testCase);
         }
 
-        [SkipOnCoreClr("https://github.com/dotnet/runtime/issues/54007", RuntimeConfiguration.Checked)]
         [Theory]
         [MemberData(nameof(TestCases))]
         public void InstanceMultiAppendGetCurrentHash(TestCase testCase)

diff --git a/src/libraries/System.IO.Hashing/tests/XxHash32Tests.f00d.cs b/src/libraries/System.IO.Hashing/tests/XxHash32Tests.f00d.cs
index 007acd7..5171e62 100644 (file)
--- a/src/libraries/System.IO.Hashing/tests/XxHash32Tests.f00d.cs
+++ b/src/libraries/System.IO.Hashing/tests/XxHash32Tests.f00d.cs
@@ -105,7 +105,6 @@ namespace System.IO.Hashing.Tests
             InstanceAppendAllocateAndResetDriver(testCase);
         }
 
-        [SkipOnCoreClr("https://github.com/dotnet/runtime/issues/54007", RuntimeConfiguration.Checked)]
         [Theory]
         [MemberData(nameof(TestCases))]
         public void InstanceMultiAppendGetCurrentHash(TestCase testCase)

diff --git a/src/libraries/System.IO.Hashing/tests/XxHash64Tests.007.cs b/src/libraries/System.IO.Hashing/tests/XxHash64Tests.007.cs
index 4da264f..5c2e575 100644 (file)
--- a/src/libraries/System.IO.Hashing/tests/XxHash64Tests.007.cs
+++ b/src/libraries/System.IO.Hashing/tests/XxHash64Tests.007.cs
@@ -115,7 +115,6 @@ namespace System.IO.Hashing.Tests
             InstanceAppendAllocateAndResetDriver(testCase);
         }
 
-        [SkipOnCoreClr("https://github.com/dotnet/runtime/issues/54007", RuntimeConfiguration.Checked)]
         [Theory]
         [MemberData(nameof(TestCases))]
         public void InstanceMultiAppendGetCurrentHash(TestCase testCase)

diff --git a/src/libraries/System.IO.Hashing/tests/XxHash64Tests.cs b/src/libraries/System.IO.Hashing/tests/XxHash64Tests.cs
index 39245a9..973d108 100644 (file)
--- a/src/libraries/System.IO.Hashing/tests/XxHash64Tests.cs
+++ b/src/libraries/System.IO.Hashing/tests/XxHash64Tests.cs
@@ -131,7 +131,6 @@ namespace System.IO.Hashing.Tests
             InstanceAppendAllocateAndResetDriver(testCase);
         }
 
-        [SkipOnCoreClr("https://github.com/dotnet/runtime/issues/54007", RuntimeConfiguration.Checked)]
         [Theory]
         [MemberData(nameof(TestCases))]
         public void InstanceMultiAppendGetCurrentHash(TestCase testCase)

diff --git a/src/libraries/System.IO.Hashing/tests/XxHash64Tests.f00d.cs b/src/libraries/System.IO.Hashing/tests/XxHash64Tests.f00d.cs
index 79d370e..23006a5 100644 (file)
--- a/src/libraries/System.IO.Hashing/tests/XxHash64Tests.f00d.cs
+++ b/src/libraries/System.IO.Hashing/tests/XxHash64Tests.f00d.cs
@@ -115,7 +115,6 @@ namespace System.IO.Hashing.Tests
             InstanceAppendAllocateAndResetDriver(testCase);
         }
 
-        [SkipOnCoreClr("https://github.com/dotnet/runtime/issues/54007", RuntimeConfiguration.Checked)]
         [Theory]
         [MemberData(nameof(TestCases))]
         public void InstanceMultiAppendGetCurrentHash(TestCase testCase)