From 8b612a601458ab102fc32119e914dbe18255eb6b Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 14 Jul 2026 09:45:40 -0700 Subject: [PATCH 01/26] Generalize IsBitwiseEquatable to field-wise IEquatable value types Recognize an unmanaged value type that implements IEquatable of self as bitwise-equatable when its Equals is provably a plain field-wise comparison (equivalent to memcmp), including a single forward through a field-wise op_Equality. Implemented in the CoreCLR VM and mirrored in the ILC/NativeAOT intrinsic, with tests covering the ILC scanner. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Test.CoreLib/src/System/IEquatable.cs | 10 + .../Test.CoreLib/src/Test.CoreLib.csproj | 1 + .../TypeSystem/IL/Stubs/ComparerIntrinsics.cs | 151 ++++++++++ .../IL/Stubs/RuntimeHelpersIntrinsics.cs | 9 + .../BitwiseEquatableTests.cs | 79 +++++ .../EquatableAssets/BitwiseEquatable.cs | 79 +++++ .../EquatableAssets/EquatableAssets.csproj | 21 ++ .../ILCompiler.Compiler.Tests.csproj | 6 + src/coreclr/vm/corelib.h | 1 + src/coreclr/vm/jitinterface.cpp | 283 +++++++++++++++++- 10 files changed, 634 insertions(+), 6 deletions(-) create mode 100644 src/coreclr/nativeaot/Test.CoreLib/src/System/IEquatable.cs create mode 100644 src/coreclr/tools/aot/ILCompiler.Compiler.Tests/BitwiseEquatableTests.cs create mode 100644 src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/BitwiseEquatable.cs create mode 100644 src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/EquatableAssets.csproj diff --git a/src/coreclr/nativeaot/Test.CoreLib/src/System/IEquatable.cs b/src/coreclr/nativeaot/Test.CoreLib/src/System/IEquatable.cs new file mode 100644 index 00000000000000..d694629537297c --- /dev/null +++ b/src/coreclr/nativeaot/Test.CoreLib/src/System/IEquatable.cs @@ -0,0 +1,10 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System +{ + public interface IEquatable + { + bool Equals(T other); + } +} diff --git a/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj b/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj index e1b9c28a56c2c9..f33002cf626c01 100644 --- a/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj +++ b/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj @@ -236,6 +236,7 @@ + diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs index e183c0f85b9d44..dd3c93522e7d85 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using Internal.TypeSystem; +using Internal.TypeSystem.Ecma; using Debug = System.Diagnostics.Debug; @@ -317,6 +318,156 @@ public static bool CanCompareValueTypeBitsUntilOffset(MetadataType type, MethodD return result; } + /// + /// Determines whether a value type's implementation of self is a + /// plain field-wise comparison that is equivalent to a bitwise (memcmp) comparison. This lets a type + /// that implements IEquatable<T> still be reported as bitwise-equatable when its Equals does + /// nothing more than compare every field with ==. + /// + public static bool IsIEquatableEqualsFieldwise(MetadataType type) + { + // Keep token resolution simple by only handling non-generic value types, matching the VM. + if (type.HasInstantiation) + return false; + + MetadataType iequatableType = type.Context.SystemModule.GetKnownType("System"u8, "IEquatable`1"u8); + MethodDesc equalsInterfaceMethod = iequatableType.MakeInstantiatedType(type).GetMethod("Equals"u8, null); + if (equalsInterfaceMethod == null) + return false; + + MethodDesc equalsImpl = type.ResolveInterfaceMethodToVirtualMethodOnType(equalsInterfaceMethod); + if (equalsImpl is not EcmaMethod) + return false; + + MethodIL methodIL = EcmaMethodIL.Create((EcmaMethod)equalsImpl); + + // A common pattern forwards `bool Equals(T other) => this == other;` to a user-defined + // `op_Equality`. Follow that single forward before scanning the field-wise comparison. + if (TryGetOpEqualityForward(methodIL, type) is EcmaMethod forwarded) + methodIL = EcmaMethodIL.Create(forwarded); + + return ScanFieldwiseEqualsBody(methodIL, type); + } + + private static MethodDesc TryGetOpEqualityForward(MethodIL methodIL, MetadataType type) + { + // ldarg.0; ldobj T; ldarg.1; call op_Equality; ret + ILReader reader = new ILReader(methodIL.GetILBytes()); + + if (!reader.HasNext || reader.ReadILOpcode() != ILOpcode.ldarg_0) + return null; + if (!reader.HasNext || reader.ReadILOpcode() != ILOpcode.ldobj) + return null; + if (methodIL.GetObject(reader.ReadILToken()) as TypeDesc != type) + return null; + if (!reader.HasNext || reader.ReadILOpcode() != ILOpcode.ldarg_1) + return null; + if (!reader.HasNext || reader.ReadILOpcode() != ILOpcode.call) + return null; + MethodDesc callee = methodIL.GetObject(reader.ReadILToken()) as MethodDesc; + if (!reader.HasNext || reader.ReadILOpcode() != ILOpcode.ret) + return null; + if (reader.HasNext) + return null; + + if (callee == null || !callee.Signature.IsStatic || callee.OwningType != type || callee.Name != "op_Equality"u8) + return null; + + return callee; + } + + private static bool ScanFieldwiseEqualsBody(MethodIL methodIL, MetadataType type) + { + // Verifies the body is a plain field-wise equality: every instance field is compared exactly once + // with `==` and the results are ANDed together, which is equivalent to a bitwise (memcmp) comparison. + int instanceFieldCount = 0; + foreach (FieldDesc field in type.GetFields()) + { + if (!field.IsStatic) + instanceFieldCount++; + } + + if (instanceFieldCount == 0) + return false; + + HashSet comparedFields = new HashSet(); + ILReader reader = new ILReader(methodIL.GetILBytes()); + + int falseTarget = -1; + bool sawFinalCompare = false; + + while (!sawFinalCompare) + { + // Each field comparison loads `ldarg.0; ldfld F; ldarg.1; ldfld F`. + if (!reader.HasNext || reader.ReadILOpcode() != ILOpcode.ldarg_0) + return false; + if (reader.ReadILOpcode() != ILOpcode.ldfld) + return false; + FieldDesc leftField = methodIL.GetObject(reader.ReadILToken()) as FieldDesc; + if (reader.ReadILOpcode() != ILOpcode.ldarg_1) + return false; + if (reader.ReadILOpcode() != ILOpcode.ldfld) + return false; + FieldDesc rightField = methodIL.GetObject(reader.ReadILToken()) as FieldDesc; + + if (leftField == null || leftField != rightField || leftField.IsStatic || leftField.OwningType != type) + return false; + + // Each field must be compared exactly once. + if (!comparedFields.Add(leftField)) + return false; + + if (!IsBitwiseComparablePrimitive(leftField.FieldType)) + return false; + + ILOpcode compareOpcode = reader.ReadILOpcode(); + if (compareOpcode == ILOpcode.bne_un_s) + { + // Non-final field: `bne.un.s FALSE` jumps to the shared `return false` tail. + int target = reader.ReadBranchDestination(compareOpcode); + if (falseTarget == -1) + falseTarget = target; + else if (falseTarget != target) + return false; + } + else if (compareOpcode == ILOpcode.ceq) + { + // Final field: `ceq; ret` produces the result directly. + if (reader.ReadILOpcode() != ILOpcode.ret) + return false; + sawFinalCompare = true; + } + else + { + return false; + } + } + + if (falseTarget != -1) + { + // Shared tail for a mismatch: `ldc.i4.0; ret`. + if (reader.Offset != falseTarget) + return false; + if (!reader.HasNext || reader.ReadILOpcode() != ILOpcode.ldc_i4_0) + return false; + if (reader.ReadILOpcode() != ILOpcode.ret) + return false; + } + + return !reader.HasNext && comparedFields.Count == instanceFieldCount; + } + + private static bool IsBitwiseComparablePrimitive(TypeDesc fieldType) + { + if (fieldType.IsPrimitive || fieldType.IsEnum || fieldType.IsPointer || fieldType.IsFunctionPointer) + { + TypeFlags category = fieldType.UnderlyingType.Category; + return category != TypeFlags.Single && category != TypeFlags.Double; + } + + return false; + } + private struct OverlappingFieldTracker { private BitArray _usedBytes; diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/RuntimeHelpersIntrinsics.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/RuntimeHelpersIntrinsics.cs index 7ec96db0eb5fbb..91c1e84dd323c2 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/RuntimeHelpersIntrinsics.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/RuntimeHelpersIntrinsics.cs @@ -73,6 +73,15 @@ public static MethodIL EmitIL(MethodDesc method) mdType.FindVirtualFunctionTargetMethodOnObjectType(objectEquals).OwningType != mdType && ComparerIntrinsics.CanCompareValueTypeBits(mdType, objectEquals); } + else if (equatable.HasValue && equatable.Value) + { + // Value type that implements IEquatable of self. It is still bitwise-equatable + // if its fields are memcmp-comparable and its Equals is a plain field-wise comparison. + MethodDesc objectEquals = mdType.Context.GetWellKnownType(WellKnownType.Object).GetMethod("Equals"u8, null); + result = + ComparerIntrinsics.CanCompareValueTypeBits(mdType, objectEquals) && + ComparerIntrinsics.IsIEquatableEqualsFieldwise(mdType); + } } } break; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/BitwiseEquatableTests.cs b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/BitwiseEquatableTests.cs new file mode 100644 index 00000000000000..e51676fe8651e9 --- /dev/null +++ b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/BitwiseEquatableTests.cs @@ -0,0 +1,79 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Text; + +using Internal.IL; +using Internal.IL.Stubs; +using Internal.Text; +using Internal.TypeSystem; + +using Xunit; + +namespace ILCompiler.Compiler.Tests +{ + public class BitwiseEquatableTests + { + private readonly CompilerTypeSystemContext _context; + private readonly ModuleDesc _testModule; + private readonly MethodDesc _objectEquals; + + public BitwiseEquatableTests() + { + var target = new TargetDetails(TargetArchitecture.X64, TargetOS.Windows, TargetAbi.NativeAot); + _context = new CompilerTypeSystemContext(target, SharedGenericsMode.CanonicalReferenceTypes, DelegateFeature.All); + + _context.InputFilePaths = new Dictionary { + { "Test.CoreLib", @"Test.CoreLib.dll" }, + { "EquatableAssets", @"EquatableAssets.dll" }, + }; + _context.ReferenceFilePaths = new Dictionary(); + + _context.SetSystemModule(_context.GetModuleForSimpleName("Test.CoreLib")); + _testModule = _context.GetModuleForSimpleName("EquatableAssets"); + _objectEquals = _context.GetWellKnownType(WellKnownType.Object).GetMethod("Equals"u8, null); + } + + private MetadataType GetTestType(string name) + => (MetadataType)_testModule.GetType( + new Utf8Span(Encoding.UTF8.GetBytes("BitwiseEquatable")), + new Utf8Span(Encoding.UTF8.GetBytes(name))); + + [Theory] + [InlineData("OneField", true)] + [InlineData("TwoFields", true)] + [InlineData("MixedPrimitives", true)] + [InlineData("ForwardsToOp", true)] + [InlineData("FloatField", false)] + [InlineData("PartialCompare", false)] + [InlineData("OrCompare", false)] + [InlineData("NestedField", false)] + [InlineData("NotEquatable", false)] + public void TestIsBitwiseEquatable(string typeName, bool expected) + { + MetadataType type = GetTestType(typeName); + + // This mirrors the decision RuntimeHelpersIntrinsics.EmitIL makes for a value type + // that implements IEquatable of self. + bool result = ComparerIntrinsics.CanCompareValueTypeBits(type, _objectEquals) + && ComparerIntrinsics.IsIEquatableEqualsFieldwise(type); + + Assert.Equal(expected, result); + } + + [Theory] + [InlineData("PartialCompare")] + [InlineData("OrCompare")] + [InlineData("NestedField")] + public void TestFieldwiseScanRejectsNonEquivalentEquals(string typeName) + { + // These types are bit-comparable at the field level, so the scan of the actual + // Equals implementation is what proves they are not memcmp-equivalent. + MetadataType type = GetTestType(typeName); + + Assert.True(ComparerIntrinsics.CanCompareValueTypeBits(type, _objectEquals)); + Assert.False(ComparerIntrinsics.IsIEquatableEqualsFieldwise(type)); + } + } +} diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/BitwiseEquatable.cs b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/BitwiseEquatable.cs new file mode 100644 index 00000000000000..70f1bbe1d60c18 --- /dev/null +++ b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/BitwiseEquatable.cs @@ -0,0 +1,79 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +#pragma warning disable 649 // field is never assigned to +#pragma warning disable 660 // defines operator == but not Equals +#pragma warning disable 661 // defines operator == but not GetHashCode + +namespace BitwiseEquatable +{ + // Positive cases: value types whose IEquatable.Equals is a plain field-wise comparison. + + public struct OneField : IEquatable + { + public int A; + public bool Equals(OneField other) => A == other.A; + } + + public struct TwoFields : IEquatable + { + public int A; + public int B; + public bool Equals(TwoFields other) => A == other.A && B == other.B; + } + + public struct MixedPrimitives : IEquatable + { + public long A; + public int B; + public short C; + public short D; + public bool Equals(MixedPrimitives other) => A == other.A && B == other.B && C == other.C && D == other.D; + } + + public struct ForwardsToOp : IEquatable + { + public int A; + public int B; + public static bool operator ==(ForwardsToOp x, ForwardsToOp y) => x.A == y.A && x.B == y.B; + public static bool operator !=(ForwardsToOp x, ForwardsToOp y) => !(x == y); + public bool Equals(ForwardsToOp other) => this == other; + } + + // Negative cases. + + public struct FloatField : IEquatable + { + public float A; + public bool Equals(FloatField other) => A == other.A; + } + + public struct PartialCompare : IEquatable + { + public int A; + public int B; + public bool Equals(PartialCompare other) => A == other.A; + } + + public struct OrCompare : IEquatable + { + public int A; + public int B; + public bool Equals(OrCompare other) => A == other.A || B == other.B; + } + + public struct NestedField : IEquatable + { + public OneField A; + public int B; + public bool Equals(NestedField other) => A.Equals(other.A) && B == other.B; + } + + public struct NotEquatable + { + public int A; + public int B; + } +} diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/EquatableAssets.csproj b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/EquatableAssets.csproj new file mode 100644 index 00000000000000..61e31cdad44699 --- /dev/null +++ b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/EquatableAssets.csproj @@ -0,0 +1,21 @@ + + + Library + EquatableAssets + false + false + true + $(NetCoreAppToolCurrent) + + true + false + + true + + + + + + + diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/ILCompiler.Compiler.Tests.csproj b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/ILCompiler.Compiler.Tests.csproj index 69e9d87637f92d..cf4accbf0e8b1f 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/ILCompiler.Compiler.Tests.csproj +++ b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/ILCompiler.Compiler.Tests.csproj @@ -37,10 +37,16 @@ Content PreserveNewest + + false + Content + PreserveNewest + + diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index 7020945877cc67..4e54d5df520337 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -1301,6 +1301,7 @@ DEFINE_CLASS(ICOMPARABLEGENERIC, System, IComparable`1) DEFINE_METHOD(ICOMPARABLEGENERIC, COMPARE_TO, CompareTo, NoSig) DEFINE_CLASS(IEQUATABLEGENERIC, System, IEquatable`1) +DEFINE_METHOD(IEQUATABLEGENERIC, EQUALS, Equals, NoSig) DEFINE_CLASS_U(Reflection, LoaderAllocator, LoaderAllocatorObject) DEFINE_FIELD_U(m_slots, LoaderAllocatorObject, m_pSlots) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index b382c42437cda2..3f571ffcabbeb2 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -7287,23 +7287,294 @@ static bool getILIntrinsicImplementationForInterlocked(MethodDesc * ftn, return true; } +namespace +{ + // The subset of IL opcodes the field-wise Equals scanner understands. The scanner only accepts + // the exact shapes the C# compiler emits for a field-wise equality comparison, so a small, + // literal opcode table is all that is required. + enum ILByte : BYTE + { + IL_LDARG_0 = 0x02, + IL_LDARG_1 = 0x03, + IL_LDC_I4_0 = 0x16, + IL_CALL = 0x28, + IL_RET = 0x2A, + IL_BNE_UN_S = 0x33, + IL_LDOBJ = 0x71, + IL_LDFLD = 0x7B, + IL_PREFIX1 = 0xFE, // ceq is encoded as 0xFE 0x01 + IL_CEQ_2ND = 0x01, + }; + + mdToken ReadILToken(const BYTE* pIL) + { + LIMITED_METHOD_CONTRACT; + return (mdToken)(pIL[0] | (pIL[1] << 8) | (pIL[2] << 16) | (pIL[3] << 24)); + } + + // Resolves an in-module FieldDef token. Returns NULL for anything else (e.g. a MemberRef, + // which only arises for generic or cross-module references we deliberately don't handle yet). + FieldDesc* TryResolveInModuleFieldDef(Module* pModule, mdToken token) + { + STANDARD_VM_CONTRACT; + if (TypeFromToken(token) != mdtFieldDef) + return NULL; + return pModule->LookupFieldDef(token); + } + + // Resolves an in-module MethodDef token. Returns NULL for anything else. + MethodDesc* TryResolveInModuleMethodDef(Module* pModule, mdToken token) + { + STANDARD_VM_CONTRACT; + if (TypeFromToken(token) != mdtMethodDef) + return NULL; + return MemberLoader::GetMethodDescFromMethodDef(pModule, token, Instantiation(), Instantiation()); + } + + // Interface dispatch on a value type resolves to an unboxing stub, which carries no IL of its + // own. Unwrap it to the underlying instance method so the scanner can read its body. + MethodDesc* UnwrapStub(MethodDesc* pMD) + { + WRAPPER_NO_CONTRACT; + if (pMD != NULL && pMD->IsWrapperStub()) + return pMD->GetWrappedMethodDesc(); + return pMD; + } + + // Scans the body of a method that is expected to compare every instance field of + // 'valueTypeMT' pairwise, ANDing the results, and returns true only if the body is exactly + // such a comparison and every comparison is bit-for-bit equivalent to memcmp. + // + // The C# compiler lowers 'this.f0 == other.f0 && this.f1 == other.f1 && ...' to a sequence of + // per-field units sharing a single 'return false' tail: + // + // non-final field: ldarg.0; ldfld F; ldarg.1; ldfld F; bne.un.s FALSE + // final field: ldarg.0; ldfld F; ldarg.1; ldfld F; ceq; ret + // shared tail: FALSE: ldc.i4.0; ret + // + // The two operands are always argument 0 and argument 1 (which are 'this'/'other' for the + // instance Equals and the two parameters for op_Equality). Requiring every instance field to be + // compared exactly once, together with the caller's 'tightly packed' guarantee, is what makes + // the field-wise comparison equivalent to a full memcmp. Only primitive (integer-like) fields + // are handled; the tightly-packed guarantee already excludes any struct with a value-type field. + bool ScanFieldwiseEqualsBody(MethodDesc* pEqualsMD, MethodTable* valueTypeMT) + { + STANDARD_VM_CONTRACT; + + if (!pEqualsMD->MayHaveILHeader()) + return false; + + COR_ILMETHOD* pILMethod = pEqualsMD->GetILHeader(); + if (pILMethod == NULL) + return false; + + COR_ILMETHOD_DECODER header(pILMethod); + const BYTE* pIL = header.Code; + if (pIL == NULL) + return false; + + const unsigned codeSize = header.GetCodeSize(); + Module* pModule = pEqualsMD->GetModule(); + + // Track which instance fields have been compared so we can require full coverage. + const DWORD fieldCount = valueTypeMT->GetNumInstanceFields(); + if (fieldCount == 0) + return false; + + NewArrayHolder compared(new FieldDesc*[fieldCount]); + DWORD numCompared = 0; + + unsigned ip = 0; + int falseTarget = -1; // shared 'return false' offset, discovered from the first branch + bool sawFinalUnit = false; + + while (!sawFinalUnit) + { + // Left operand: ldarg.0; ldfld F. + if (ip + 6 > codeSize || pIL[ip] != IL_LDARG_0 || pIL[ip + 1] != IL_LDFLD) + return false; + mdToken leftFieldTok = ReadILToken(pIL + ip + 2); + ip += 6; + + // Right operand: ldarg.1; ldfld of the same field. + if (ip + 6 > codeSize || pIL[ip] != IL_LDARG_1 || pIL[ip + 1] != IL_LDFLD) + return false; + mdToken rightFieldTok = ReadILToken(pIL + ip + 2); + ip += 6; + + if (leftFieldTok != rightFieldTok) + return false; + + FieldDesc* pField = TryResolveInModuleFieldDef(pModule, leftFieldTok); + if (pField == NULL || + pField->IsStatic() || + pField->GetApproxEnclosingMethodTable() != valueTypeMT) + { + return false; + } + + // Each field must be compared exactly once. + for (DWORD i = 0; i < numCompared; i++) + { + if (compared[i] == pField) + return false; + } + compared[numCompared++] = pField; + + // Only integer-like fields are bit-for-bit equivalent to '=='. Float/double are excluded + // (NaN != NaN, +0.0 == -0.0), and anything else (including value-type fields) is rejected. + switch (pField->GetFieldType()) + { + case ELEMENT_TYPE_BOOLEAN: + case ELEMENT_TYPE_CHAR: + case ELEMENT_TYPE_I1: + case ELEMENT_TYPE_U1: + case ELEMENT_TYPE_I2: + case ELEMENT_TYPE_U2: + case ELEMENT_TYPE_I4: + case ELEMENT_TYPE_U4: + case ELEMENT_TYPE_I8: + case ELEMENT_TYPE_U8: + case ELEMENT_TYPE_I: + case ELEMENT_TYPE_U: + case ELEMENT_TYPE_PTR: + case ELEMENT_TYPE_FNPTR: + break; + default: + return false; + } + + if (ip < codeSize && pIL[ip] == IL_BNE_UN_S) + { + // Non-final field: branch to the shared 'return false'. + if (ip + 2 > codeSize) + return false; + int target = (int)(ip + 2) + (int)(signed char)pIL[ip + 1]; + if (falseTarget == -1) + falseTarget = target; + else if (falseTarget != target) + return false; + ip += 2; + } + else if (ip + 3 <= codeSize && pIL[ip] == IL_PREFIX1 && pIL[ip + 1] == IL_CEQ_2ND && pIL[ip + 2] == IL_RET) + { + // Final field: ceq; ret. + ip += 3; + sawFinalUnit = true; + } + else + { + return false; + } + } + + // If there were any branches, the shared tail must be exactly 'ldc.i4.0; ret' and every + // branch must target it. A single-field comparison has no branches and no tail. + if (falseTarget != -1) + { + if ((int)ip != falseTarget || + ip + 2 != codeSize || + pIL[ip] != IL_LDC_I4_0 || + pIL[ip + 1] != IL_RET) + { + return false; + } + } + else if (ip != codeSize) + { + return false; + } + + // Every instance field must have been compared. + return numCompared == fieldCount; + } + + // Determines whether 'valueTypeMT's IEquatable.Equals implementation is a plain field-wise + // comparison that is equivalent to memcmp. 'pEqualsMD' is that Equals method. + bool IsFieldwiseEqualsBitwiseEquivalent(MethodTable* valueTypeMT, MethodDesc* pEqualsMD) + { + STANDARD_VM_CONTRACT; + + // First pass restrictions to keep the scan simple and unquestionably safe: + // - Must be unmanaged so a byte-wise compare is meaningful. + // - No generics for now (keeps IL token resolution to in-module def tokens). + // - Must be tightly packed (no padding gaps and no overlapping fields), otherwise memcmp + // would compare bytes the field-wise Equals ignores. + // - No inline arrays. + if (valueTypeMT->ContainsGCPointers() || + valueTypeMT->HasInstantiation() || + valueTypeMT->IsNotTightlyPacked() || + valueTypeMT->GetClass()->IsInlineArray()) + { + return false; + } + + // Follow the extremely common 'Equals(T other) => this == other' forward into op_Equality: + // ldarg.0; ldobj T; ldarg.1; call op_Equality; ret + MethodDesc* pScanMD = pEqualsMD; + if (pEqualsMD->MayHaveILHeader()) + { + COR_ILMETHOD* pILMethod = pEqualsMD->GetILHeader(); + if (pILMethod != NULL) + { + COR_ILMETHOD_DECODER header(pILMethod); + const BYTE* pIL = header.Code; + const unsigned codeSize = header.GetCodeSize(); + + // 02 71 03 28 2A + if (pIL != NULL && codeSize == 13 && + pIL[0] == IL_LDARG_0 && + pIL[1] == IL_LDOBJ && ReadILToken(pIL + 2) == valueTypeMT->GetCl() && + pIL[6] == IL_LDARG_1 && + pIL[7] == IL_CALL && + pIL[12] == IL_RET) + { + MethodDesc* pOpEquality = TryResolveInModuleMethodDef(pEqualsMD->GetModule(), ReadILToken(pIL + 8)); + if (pOpEquality != NULL && + pOpEquality->IsStatic() && + pOpEquality->GetMethodTable() == valueTypeMT && + strcmp(pOpEquality->GetName(), "op_Equality") == 0) + { + pScanMD = pOpEquality; + } + } + } + } + + return ScanFieldwiseEqualsBody(pScanMD, valueTypeMT); + } +} + bool IsBitwiseEquatable(TypeHandle typeHandle, MethodTable * methodTable) { - if (!methodTable->IsValueType() || - !CanCompareBitsOrUseFastGetHashCode(methodTable)) + STANDARD_VM_CONTRACT; + + if (!methodTable->IsValueType()) { return false; } - // CanCompareBitsOrUseFastGetHashCode checks for an object.Equals override. - // We also need to check for an IEquatable implementation. Instantiation inst(&typeHandle, 1); - if (typeHandle.CanCastTo(TypeHandle(CoreLibBinder::GetClass(CLASS__IEQUATABLEGENERIC)).Instantiate(inst))) + TypeHandle iequatableOfSelf = TypeHandle(CoreLibBinder::GetClass(CLASS__IEQUATABLEGENERIC)).Instantiate(inst); + + if (!typeHandle.CanCastTo(iequatableOfSelf)) + { + // The type provides no IEquatable of its own, so bitwise equality is safe as long as the + // fields are bit-comparable and there is no object.Equals override with custom semantics. + return CanCompareBitsOrUseFastGetHashCode(methodTable); + } + + // The type provides IEquatable.Equals. It can only be treated as bitwise equatable if that + // implementation is a plain field-wise comparison equivalent to memcmp. Interface dispatch on a + // value type resolves to an unboxing stub, so unwrap it to the underlying instance method. + MethodDesc* pEqualsMD = UnwrapStub(methodTable->GetMethodDescForInterfaceMethod( + iequatableOfSelf, CoreLibBinder::GetMethod(METHOD__IEQUATABLEGENERIC__EQUALS), FALSE /* throwOnConflict */)); + if (pEqualsMD == NULL) { return false; } - return true; + return IsFieldwiseEqualsBitwiseEquivalent(methodTable, pEqualsMD); } static bool getILIntrinsicImplementationForRuntimeHelpers( From f2fd52aa2a78a46fd8d34d5e3e2e19113ec40d8c Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 14 Jul 2026 11:54:52 -0700 Subject: [PATCH 02/26] Fix IsNotTightlyPacked for multi-byte value-type fields InitializeFieldDescs accumulated only `1 << dwLog2FieldSize` into totalDeclaredFieldSize for by-value instance fields, where dwLog2FieldSize is forced to 0. Any struct containing a multi-byte value-type field was therefore always flagged NotTightlyPacked, needlessly pushing ValueType.Equals and GetHashCode onto the reflection slow path. Accumulate the real GetNumInstanceFieldBytes() for by-value fields so the flag accurately reflects whether the declared fields exactly cover the instance size at each level. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/methodtablebuilder.cpp | 34 ++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index c9984deef79a1b..647d59fa3f9ba5 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -1822,7 +1822,8 @@ MethodTableBuilder::BuildMethodTableThrowing( BuildMethodTableThrowException(IDS_CLASSLOAD_FIELDTOOLARGE); } - if (CheckIfSIMDAndUpdateSize()) + bool fIsSIMDType = CheckIfSIMDAndUpdateSize(); + if (fIsSIMDType) { totalDeclaredFieldSize = bmtFP->NumInstanceFieldBytes; } @@ -1866,7 +1867,28 @@ MethodTableBuilder::BuildMethodTableThrowing( if (IsValueClass()) { - if (bmtFP->NumInstanceFieldBytes != totalDeclaredFieldSize || HasOverlaidField()) + // A value type is "tightly packed" when a byte-wise compare of two instances equals comparing + // every field, i.e. there is no padding anywhere. That needs (1) the declared fields to exactly + // cover the instance size with no gaps or overlap ('totalDeclaredFieldSize' sums each field's + // full size, so it catches gaps at this level) and (2) every nested value-type field to itself + // be tightly packed, else the compare reads padding inside it. Propagating (2) makes the flag + // transitive, so callers need not recurse. SIMD types are exempt (size treated as fully covered). + bool fIsNotTightlyPacked = (bmtFP->NumInstanceFieldBytes != totalDeclaredFieldSize) || HasOverlaidField(); + + if (!fIsNotTightlyPacked && !fIsSIMDType && pByValueClassCache != NULL) + { + for (DWORD i = 0; i < bmtEnumFields->dwNumInstanceFields; i++) + { + MethodTable* pFieldMT = pByValueClassCache[i]; + if (pFieldMT != NULL && pFieldMT->IsNotTightlyPacked()) + { + fIsNotTightlyPacked = true; + break; + } + } + } + + if (fIsNotTightlyPacked) GetHalfBakedClass()->SetIsNotTightlyPacked(); #ifdef FEATURE_HFA @@ -4516,7 +4538,13 @@ VOID MethodTableBuilder::InitializeFieldDescs(FieldDesc *pFieldDescList, if (!fIsStatic) { pFD = &pFieldDescList[dwCurrentDeclaredField]; // lgtm [cpp/upcast-array-pointer-arithmetic] - The call of concern in FixupFieldDescForEnC, initializes this loop invariant to 1, so will never be > 1. - *totalDeclaredSize += (1 << dwLog2FieldSize); + + // Accumulate declared field sizes so the type can be flagged NotTightlyPacked when they + // don't exactly cover the instance size. A value-type field contributes its full instance + // size; dwLog2FieldSize is meaningless for value types (size isn't a power of two). + *totalDeclaredSize += fIsByValue + ? (*pByValueClassCache)[dwCurrentDeclaredField]->GetNumInstanceFieldBytes() + : (1 << dwLog2FieldSize); } else /* (dwMemberAttrs & mdStatic) */ { From 53346519d854b387293ec8fd75274a90693ccda5 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 14 Jul 2026 10:15:57 -0700 Subject: [PATCH 03/26] Recognize recursive value-type fields in IsBitwiseEquatable A nested value-type field compared through its own IEquatable.Equals is now accepted, recursing per level so a struct-of-structs whose Equals is a plain field-wise comparison is reported bitwise-equatable in both the VM and ILC. MethodTable::IsNotTightlyPacked() cannot gate this: the layout builder accounts a nested value-type field as a single byte, so it flags every value type containing another value type. The scan now computes tight-packing directly from the field offsets and sizes, matching the ILC ComparerIntrinsics.IsTightlyPacked, and the ILC IEquatable path is made self-contained on layout so both sides agree even when a nested type overrides object.Equals. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../TypeSystem/IL/Stubs/ComparerIntrinsics.cs | 174 ++++++++++-- .../IL/Stubs/RuntimeHelpersIntrinsics.cs | 9 +- .../BitwiseEquatableTests.cs | 13 +- .../EquatableAssets/BitwiseEquatable.cs | 76 ++++- src/coreclr/vm/jitinterface.cpp | 261 +++++++++++++----- 5 files changed, 417 insertions(+), 116 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs index dd3c93522e7d85..b83f02b161b5c4 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs @@ -326,20 +326,17 @@ public static bool CanCompareValueTypeBitsUntilOffset(MetadataType type, MethodD /// public static bool IsIEquatableEqualsFieldwise(MetadataType type) { - // Keep token resolution simple by only handling non-generic value types, matching the VM. - if (type.HasInstantiation) + // The type's layout must be tightly packed (no padding gaps and no overlapping fields) so + // that a byte-wise compare never inspects bytes the field-wise Equals ignores. This is + // checked at every level of the recursion, matching the CoreCLR VM. + if (!IsTightlyPacked(type)) return false; - MetadataType iequatableType = type.Context.SystemModule.GetKnownType("System"u8, "IEquatable`1"u8); - MethodDesc equalsInterfaceMethod = iequatableType.MakeInstantiatedType(type).GetMethod("Equals"u8, null); - if (equalsInterfaceMethod == null) + MethodDesc equalsImpl = GetIEquatableEqualsImplementation(type); + if (equalsImpl is not EcmaMethod ecmaImpl) return false; - MethodDesc equalsImpl = type.ResolveInterfaceMethodToVirtualMethodOnType(equalsInterfaceMethod); - if (equalsImpl is not EcmaMethod) - return false; - - MethodIL methodIL = EcmaMethodIL.Create((EcmaMethod)equalsImpl); + MethodIL methodIL = EcmaMethodIL.Create(ecmaImpl); // A common pattern forwards `bool Equals(T other) => this == other;` to a user-defined // `op_Equality`. Follow that single forward before scanning the field-wise comparison. @@ -349,6 +346,63 @@ public static bool IsIEquatableEqualsFieldwise(MetadataType type) return ScanFieldwiseEqualsBody(methodIL, type); } + private static bool IsTightlyPacked(MetadataType type) + { + // Mirrors the CoreCLR VM's MethodTable::IsNotTightlyPacked (negated): a byte-wise compare + // equals comparing every field only if there is no padding anywhere. That needs the declared + // fields to exactly cover the instance size (no gaps, no overlap) and every nested value-type + // field to itself be tightly packed. The nested check makes this transitive, like the VM flag. + if (type.ContainsGCPointers) + return false; + + if (type.IsInlineArray) + return false; + + if (type.IsGenericDefinition) + return false; + + OverlappingFieldTracker overlappingFieldTracker = new OverlappingFieldTracker(type); + int lastFieldEndOffset = 0; + + foreach (FieldDesc field in type.GetFields()) + { + if (field.IsStatic) + continue; + + lastFieldEndOffset = Math.Max(lastFieldEndOffset, field.Offset.AsInt + field.FieldType.GetElementSize().AsInt); + + if (!overlappingFieldTracker.TrackField(field)) + return false; + + TypeDesc fieldType = field.FieldType; + if (!fieldType.IsPrimitive && !fieldType.IsEnum && !fieldType.IsPointer && !fieldType.IsFunctionPointer) + { + // Not a leaf field, so (having excluded GC pointers above) it is a nested value type. + if (fieldType is not MetadataType nestedType || !IsTightlyPacked(nestedType)) + return false; + } + } + + if (overlappingFieldTracker.HasGapsBeforeOffset(lastFieldEndOffset)) + return false; + + return lastFieldEndOffset == type.InstanceFieldSize.AsInt; + } + + private static MethodDesc GetIEquatableEqualsImplementation(MetadataType type) + { + // Keep token resolution simple by only handling non-generic value types, matching the VM. + if (type.HasInstantiation) + return null; + + MetadataType iequatableType = type.Context.SystemModule.GetKnownType("System"u8, "IEquatable`1"u8); + MethodDesc equalsInterfaceMethod = iequatableType.MakeInstantiatedType(type).GetMethod("Equals"u8, null); + if (equalsInterfaceMethod == null) + return null; + + return type.ResolveInterfaceMethodToVirtualMethodOnType(equalsInterfaceMethod); + } + private static MethodDesc TryGetOpEqualityForward(MethodIL methodIL, MetadataType type) { // ldarg.0; ldobj T; ldarg.1; call op_Equality; ret @@ -398,12 +452,17 @@ private static bool ScanFieldwiseEqualsBody(MethodIL methodIL, MetadataType type while (!sawFinalCompare) { - // Each field comparison loads `ldarg.0; ldfld F; ldarg.1; ldfld F`. + // Each field comparison loads the field from both arguments. A primitive compared with + // `==` loads both by value (`ldfld`); a primitive compared with its own `Equals`, or a + // nested value-type field, loads the left side by address (`ldflda`) for the call. if (!reader.HasNext || reader.ReadILOpcode() != ILOpcode.ldarg_0) return false; - if (reader.ReadILOpcode() != ILOpcode.ldfld) + + ILOpcode leftLoad = reader.ReadILOpcode(); + if (leftLoad != ILOpcode.ldfld && leftLoad != ILOpcode.ldflda) return false; FieldDesc leftField = methodIL.GetObject(reader.ReadILToken()) as FieldDesc; + if (reader.ReadILOpcode() != ILOpcode.ldarg_1) return false; if (reader.ReadILOpcode() != ILOpcode.ldfld) @@ -417,29 +476,62 @@ private static bool ScanFieldwiseEqualsBody(MethodIL methodIL, MetadataType type if (!comparedFields.Add(leftField)) return false; - if (!IsBitwiseComparablePrimitive(leftField.FieldType)) - return false; - - ILOpcode compareOpcode = reader.ReadILOpcode(); - if (compareOpcode == ILOpcode.bne_un_s) + bool nested = leftLoad == ILOpcode.ldflda; + if (!nested) { - // Non-final field: `bne.un.s FALSE` jumps to the shared `return false` tail. - int target = reader.ReadBranchDestination(compareOpcode); - if (falseTarget == -1) - falseTarget = target; - else if (falseTarget != target) + if (!IsBitwiseComparablePrimitive(leftField.FieldType)) return false; - } - else if (compareOpcode == ILOpcode.ceq) - { - // Final field: `ceq; ret` produces the result directly. - if (reader.ReadILOpcode() != ILOpcode.ret) + + ILOpcode compareOpcode = reader.ReadILOpcode(); + if (compareOpcode == ILOpcode.bne_un_s) + { + // Non-final field: `bne.un.s FALSE` jumps to the shared `return false` tail. + int target = reader.ReadBranchDestination(compareOpcode); + if (falseTarget == -1) + falseTarget = target; + else if (falseTarget != target) + return false; + } + else if (compareOpcode == ILOpcode.ceq) + { + // Final field: `ceq; ret` produces the result directly. + if (reader.ReadILOpcode() != ILOpcode.ret) + return false; + sawFinalCompare = true; + } + else + { return false; - sawFinalCompare = true; + } } else { - return false; + // Call form: a primitive's own Equals, or a nested type's field-wise Equals. + if (reader.ReadILOpcode() != ILOpcode.call) + return false; + MethodDesc callee = methodIL.GetObject(reader.ReadILToken()) as MethodDesc; + if (!IsPrimitiveEqualsCall(callee, leftField.FieldType) && !IsNestedFieldwiseEquatable(callee, leftField.FieldType)) + return false; + + ILOpcode compareOpcode = reader.ReadILOpcode(); + if (compareOpcode == ILOpcode.brfalse_s) + { + // Non-final field: branch to the shared tail. + int target = reader.ReadBranchDestination(compareOpcode); + if (falseTarget == -1) + falseTarget = target; + else if (falseTarget != target) + return false; + } + else if (compareOpcode == ILOpcode.ret) + { + // Final field: ret. + sawFinalCompare = true; + } + else + { + return false; + } } } @@ -457,6 +549,30 @@ private static bool ScanFieldwiseEqualsBody(MethodIL methodIL, MetadataType type return !reader.HasNext && comparedFields.Count == instanceFieldCount; } + private static bool IsNestedFieldwiseEquatable(MethodDesc callee, TypeDesc fieldType) + { + // The nested field must be compared through the nested type's own IEquatable.Equals, and + // that Equals must itself be field-wise. Its layout is validated by CanCompareValueTypeBits. + if (callee == null || fieldType is not MetadataType nestedType || !nestedType.IsValueType) + return false; + + if (callee != GetIEquatableEqualsImplementation(nestedType)) + return false; + + return IsIEquatableEqualsFieldwise(nestedType); + } + + private static bool IsPrimitiveEqualsCall(MethodDesc callee, TypeDesc fieldType) + { + // A primitive field compared via 'x.Equals(y)' instead of 'x == y'; for these integer-like + // types both lower to the same bit-for-bit compare. Confirm the callee is its IEquatable.Equals. + if (callee == null || !IsBitwiseComparablePrimitive(fieldType)) + return false; + + return fieldType is MetadataType primitiveType + && callee == GetIEquatableEqualsImplementation(primitiveType); + } + private static bool IsBitwiseComparablePrimitive(TypeDesc fieldType) { if (fieldType.IsPrimitive || fieldType.IsEnum || fieldType.IsPointer || fieldType.IsFunctionPointer) diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/RuntimeHelpersIntrinsics.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/RuntimeHelpersIntrinsics.cs index 91c1e84dd323c2..1a4695fe353ca8 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/RuntimeHelpersIntrinsics.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/RuntimeHelpersIntrinsics.cs @@ -76,11 +76,10 @@ public static MethodIL EmitIL(MethodDesc method) else if (equatable.HasValue && equatable.Value) { // Value type that implements IEquatable of self. It is still bitwise-equatable - // if its fields are memcmp-comparable and its Equals is a plain field-wise comparison. - MethodDesc objectEquals = mdType.Context.GetWellKnownType(WellKnownType.Object).GetMethod("Equals"u8, null); - result = - ComparerIntrinsics.CanCompareValueTypeBits(mdType, objectEquals) && - ComparerIntrinsics.IsIEquatableEqualsFieldwise(mdType); + // if it is tightly packed and its Equals is a plain field-wise comparison. This is + // self-contained (it does its own layout check) so that, matching the CoreCLR VM, a + // nested value-type field is accepted even when its type also overrides object.Equals. + result = ComparerIntrinsics.IsIEquatableEqualsFieldwise(mdType); } } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/BitwiseEquatableTests.cs b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/BitwiseEquatableTests.cs index e51676fe8651e9..52a28109895f46 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/BitwiseEquatableTests.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/BitwiseEquatableTests.cs @@ -45,19 +45,23 @@ private MetadataType GetTestType(string name) [InlineData("TwoFields", true)] [InlineData("MixedPrimitives", true)] [InlineData("ForwardsToOp", true)] + [InlineData("NestedField", true)] + [InlineData("NestedFieldLast", true)] + [InlineData("AllNested", true)] [InlineData("FloatField", false)] [InlineData("PartialCompare", false)] [InlineData("OrCompare", false)] - [InlineData("NestedField", false)] + [InlineData("WrapsPartial", false)] + [InlineData("Padded", false)] + [InlineData("WrapsPadded", false)] [InlineData("NotEquatable", false)] public void TestIsBitwiseEquatable(string typeName, bool expected) { MetadataType type = GetTestType(typeName); // This mirrors the decision RuntimeHelpersIntrinsics.EmitIL makes for a value type - // that implements IEquatable of self. - bool result = ComparerIntrinsics.CanCompareValueTypeBits(type, _objectEquals) - && ComparerIntrinsics.IsIEquatableEqualsFieldwise(type); + // that implements IEquatable of self: the field-wise scan is self-contained. + bool result = ComparerIntrinsics.IsIEquatableEqualsFieldwise(type); Assert.Equal(expected, result); } @@ -65,7 +69,6 @@ public void TestIsBitwiseEquatable(string typeName, bool expected) [Theory] [InlineData("PartialCompare")] [InlineData("OrCompare")] - [InlineData("NestedField")] public void TestFieldwiseScanRejectsNonEquivalentEquals(string typeName) { // These types are bit-comparable at the field level, so the scan of the actual diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/BitwiseEquatable.cs b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/BitwiseEquatable.cs index 70f1bbe1d60c18..eb84a2c5e7134b 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/BitwiseEquatable.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/BitwiseEquatable.cs @@ -9,12 +9,16 @@ namespace BitwiseEquatable { - // Positive cases: value types whose IEquatable.Equals is a plain field-wise comparison. + // Positive cases: value types whose IEquatable.Equals is a plain field-wise comparison. These + // follow the standard pattern of also overriding object.Equals/GetHashCode; that override is + // irrelevant to bitwise equatability because the IEquatable.Equals is what gets used. public struct OneField : IEquatable { public int A; public bool Equals(OneField other) => A == other.A; + public override bool Equals(object obj) => obj is OneField other && Equals(other); + public override int GetHashCode() => A; } public struct TwoFields : IEquatable @@ -22,6 +26,8 @@ public struct TwoFields : IEquatable public int A; public int B; public bool Equals(TwoFields other) => A == other.A && B == other.B; + public override bool Equals(object obj) => obj is TwoFields other && Equals(other); + public override int GetHashCode() => A; } public struct MixedPrimitives : IEquatable @@ -31,6 +37,8 @@ public struct MixedPrimitives : IEquatable public short C; public short D; public bool Equals(MixedPrimitives other) => A == other.A && B == other.B && C == other.C && D == other.D; + public override bool Equals(object obj) => obj is MixedPrimitives other && Equals(other); + public override int GetHashCode() => B; } public struct ForwardsToOp : IEquatable @@ -40,6 +48,37 @@ public struct ForwardsToOp : IEquatable public static bool operator ==(ForwardsToOp x, ForwardsToOp y) => x.A == y.A && x.B == y.B; public static bool operator !=(ForwardsToOp x, ForwardsToOp y) => !(x == y); public bool Equals(ForwardsToOp other) => this == other; + public override bool Equals(object obj) => obj is ForwardsToOp other && Equals(other); + public override int GetHashCode() => A; + } + + // Recursive cases: a nested value-type field is compared through its own IEquatable.Equals. + + public struct NestedField : IEquatable + { + public OneField A; + public int B; + public bool Equals(NestedField other) => A.Equals(other.A) && B == other.B; + public override bool Equals(object obj) => obj is NestedField other && Equals(other); + public override int GetHashCode() => B; + } + + public struct NestedFieldLast : IEquatable + { + public int A; + public TwoFields B; + public bool Equals(NestedFieldLast other) => A == other.A && B.Equals(other.B); + public override bool Equals(object obj) => obj is NestedFieldLast other && Equals(other); + public override int GetHashCode() => A; + } + + public struct AllNested : IEquatable + { + public OneField A; + public TwoFields B; + public bool Equals(AllNested other) => A.Equals(other.A) && B.Equals(other.B); + public override bool Equals(object obj) => obj is AllNested other && Equals(other); + public override int GetHashCode() => 0; } // Negative cases. @@ -48,6 +87,8 @@ public struct FloatField : IEquatable { public float A; public bool Equals(FloatField other) => A == other.A; + public override bool Equals(object obj) => obj is FloatField other && Equals(other); + public override int GetHashCode() => 0; } public struct PartialCompare : IEquatable @@ -55,6 +96,8 @@ public struct PartialCompare : IEquatable public int A; public int B; public bool Equals(PartialCompare other) => A == other.A; + public override bool Equals(object obj) => obj is PartialCompare other && Equals(other); + public override int GetHashCode() => A; } public struct OrCompare : IEquatable @@ -62,13 +105,38 @@ public struct OrCompare : IEquatable public int A; public int B; public bool Equals(OrCompare other) => A == other.A || B == other.B; + public override bool Equals(object obj) => obj is OrCompare other && Equals(other); + public override int GetHashCode() => A; } - public struct NestedField : IEquatable + // Nested field whose own Equals is not a full field-wise comparison, so the outer type is not + // memcmp-equivalent even though its layout is bit-comparable. + public struct WrapsPartial : IEquatable { - public OneField A; + public PartialCompare A; public int B; - public bool Equals(NestedField other) => A.Equals(other.A) && B == other.B; + public bool Equals(WrapsPartial other) => A.Equals(other.A) && B == other.B; + public override bool Equals(object obj) => obj is WrapsPartial other && Equals(other); + public override int GetHashCode() => B; + } + + // Nested field that introduces internal padding, so a byte-wise compare would inspect bytes the + // field-wise Equals ignores. Both the nested type and the wrapper must be rejected. + public struct Padded : IEquatable + { + public byte A; + public long B; + public bool Equals(Padded other) => A == other.A && B == other.B; + public override bool Equals(object obj) => obj is Padded other && Equals(other); + public override int GetHashCode() => (int)B; + } + + public struct WrapsPadded : IEquatable + { + public Padded A; + public bool Equals(WrapsPadded other) => A.Equals(other.A); + public override bool Equals(object obj) => obj is WrapsPadded other && Equals(other); + public override int GetHashCode() => 0; } public struct NotEquatable diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 3f571ffcabbeb2..23feda2018f26e 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -7289,9 +7289,8 @@ static bool getILIntrinsicImplementationForInterlocked(MethodDesc * ftn, namespace { - // The subset of IL opcodes the field-wise Equals scanner understands. The scanner only accepts - // the exact shapes the C# compiler emits for a field-wise equality comparison, so a small, - // literal opcode table is all that is required. + // The IL opcodes the field-wise Equals scanner matches. It only accepts the exact shapes the C# + // compiler emits for a field-wise comparison, so a small literal table suffices. enum ILByte : BYTE { IL_LDARG_0 = 0x02, @@ -7299,9 +7298,11 @@ namespace IL_LDC_I4_0 = 0x16, IL_CALL = 0x28, IL_RET = 0x2A, + IL_BRFALSE_S = 0x2C, IL_BNE_UN_S = 0x33, IL_LDOBJ = 0x71, IL_LDFLD = 0x7B, + IL_LDFLDA = 0x7C, IL_PREFIX1 = 0xFE, // ceq is encoded as 0xFE 0x01 IL_CEQ_2ND = 0x01, }; @@ -7331,8 +7332,23 @@ namespace return MemberLoader::GetMethodDescFromMethodDef(pModule, token, Instantiation(), Instantiation()); } - // Interface dispatch on a value type resolves to an unboxing stub, which carries no IL of its - // own. Unwrap it to the underlying instance method so the scanner can read its body. + // Resolves a method token, including a cross-module MemberRef (a primitive's Equals lives in + // CoreLib). Callers only match non-generic methods, so a MethodSpec resolves to its generic + // definition -- which never matches -- instead of throwing. + MethodDesc* TryResolveMethodToken(Module* pModule, mdToken token) + { + STANDARD_VM_CONTRACT; + + mdToken kind = TypeFromToken(token); + if (kind != mdtMethodDef && kind != mdtMemberRef && kind != mdtMethodSpec) + return NULL; + + SigTypeContext typeContext; + return MemberLoader::GetMethodDescFromMemberDefOrRefOrSpec( + pModule, token, &typeContext, FALSE /* strictMetadataChecks */, FALSE /* allowInstParam */); + } + + // Unwraps an unboxing stub (interface dispatch on a value type) to the instance method that has IL. MethodDesc* UnwrapStub(MethodDesc* pMD) { WRAPPER_NO_CONTRACT; @@ -7341,22 +7357,104 @@ namespace return pMD; } - // Scans the body of a method that is expected to compare every instance field of - // 'valueTypeMT' pairwise, ANDing the results, and returns true only if the body is exactly - // such a comparison and every comparison is bit-for-bit equivalent to memcmp. - // - // The C# compiler lowers 'this.f0 == other.f0 && this.f1 == other.f1 && ...' to a sequence of - // per-field units sharing a single 'return false' tail: + // Resolves 'mt's IEquatable.Equals implementation (unboxing stub unwrapped), or NULL if 'mt' + // is not a value type that implements IEquatable of self. + MethodDesc* GetIEquatableEqualsImpl(MethodTable* mt) + { + STANDARD_VM_CONTRACT; + + if (mt == NULL || !mt->IsValueType()) + return NULL; + + TypeHandle th(mt); + Instantiation inst(&th, 1); + TypeHandle iequatableOfSelf = TypeHandle(CoreLibBinder::GetClass(CLASS__IEQUATABLEGENERIC)).Instantiate(inst); + + if (!th.CanCastTo(iequatableOfSelf)) + return NULL; + + return UnwrapStub(mt->GetMethodDescForInterfaceMethod( + iequatableOfSelf, CoreLibBinder::GetMethod(METHOD__IEQUATABLEGENERIC__EQUALS), FALSE /* throwOnConflict */)); + } + + // Forward declaration: the field-wise scanner recurses into nested value-type fields. + bool IsFieldwiseEqualsBitwiseEquivalent(MethodTable* valueTypeMT, MethodDesc* pEqualsMD); + + // Integer-like primitives whose '==' and Equals are both a bit-for-bit compare. Float/double are + // excluded: neither form is a memcmp (for '==' NaN != NaN and +0.0 == -0.0; Equals treats all NaNs + // and both signed zeros as equal). + bool IsBitwiseComparablePrimitive(CorElementType et) + { + LIMITED_METHOD_CONTRACT; + switch (et) + { + case ELEMENT_TYPE_BOOLEAN: + case ELEMENT_TYPE_CHAR: + case ELEMENT_TYPE_I1: + case ELEMENT_TYPE_U1: + case ELEMENT_TYPE_I2: + case ELEMENT_TYPE_U2: + case ELEMENT_TYPE_I4: + case ELEMENT_TYPE_U4: + case ELEMENT_TYPE_I8: + case ELEMENT_TYPE_U8: + case ELEMENT_TYPE_I: + case ELEMENT_TYPE_U: + case ELEMENT_TYPE_PTR: + case ELEMENT_TYPE_FNPTR: + return true; + default: + return false; + } + } + + // Accepts a primitive field compared via 'x.Equals(y)' instead of 'x == y'; for these integer-like + // types both lower to the same bit-for-bit compare. + bool IsPrimitiveEqualsCall(MethodDesc* pCallee, FieldDesc* pField) + { + STANDARD_VM_CONTRACT; + + if (pCallee == NULL || !IsBitwiseComparablePrimitive(pField->GetFieldType())) + return false; + + MethodTable* pFieldMT = pField->GetFieldTypeHandleThrowing().GetMethodTable(); + MethodDesc* pFieldEquals = GetIEquatableEqualsImpl(pFieldMT); + return pFieldEquals != NULL && pFieldEquals == UnwrapStub(pCallee); + } + + // Accepts a nested value-type field compared through its own IEquatable.Equals, but only when + // that Equals is itself a provable field-wise compare (its layout is covered by the recursion). + bool IsNestedFieldwiseEquatable(MethodDesc* pCallee, FieldDesc* pField) + { + STANDARD_VM_CONTRACT; + + if (pCallee == NULL || pField->GetFieldType() != ELEMENT_TYPE_VALUETYPE) + return false; + + MethodTable* pNestedMT = pField->GetFieldTypeHandleThrowing().GetMethodTable(); + if (pNestedMT == NULL) + return false; + + MethodDesc* pNestedEquals = GetIEquatableEqualsImpl(pNestedMT); + if (pNestedEquals == NULL || pNestedEquals != UnwrapStub(pCallee)) + return false; + + return IsFieldwiseEqualsBitwiseEquivalent(pNestedMT, pNestedEquals); + } + + // Returns true only if 'pEqualsMD' compares every instance field of 'valueTypeMT' exactly once and + // ANDs the results, bit-for-bit like memcmp. Combined with the caller's 'tightly packed' guarantee, + // that makes the whole comparison a memcmp. // - // non-final field: ldarg.0; ldfld F; ldarg.1; ldfld F; bne.un.s FALSE - // final field: ldarg.0; ldfld F; ldarg.1; ldfld F; ceq; ret - // shared tail: FALSE: ldc.i4.0; ret + // The C# compiler lowers 'this.f0 == other.f0 && ...' to per-field units sharing one 'return false' + // tail. Operands are always arg0/arg1. A primitive is compared inline; a nested value type through + // its own IEquatable.Equals, which must itself be field-wise (checked recursively): // - // The two operands are always argument 0 and argument 1 (which are 'this'/'other' for the - // instance Equals and the two parameters for op_Equality). Requiring every instance field to be - // compared exactly once, together with the caller's 'tightly packed' guarantee, is what makes - // the field-wise comparison equivalent to a full memcmp. Only primitive (integer-like) fields - // are handled; the tightly-packed guarantee already excludes any struct with a value-type field. + // primitive, non-final: ldarg.0; ldfld F; ldarg.1; ldfld F; bne.un.s FALSE + // primitive, final: ldarg.0; ldfld F; ldarg.1; ldfld F; ceq; ret + // nested, non-final: ldarg.0; ldflda F; ldarg.1; ldfld F; call F::Equals; brfalse.s FALSE + // nested, final: ldarg.0; ldflda F; ldarg.1; ldfld F; call F::Equals; ret + // shared tail: FALSE: ldc.i4.0; ret bool ScanFieldwiseEqualsBody(MethodDesc* pEqualsMD, MethodTable* valueTypeMT) { STANDARD_VM_CONTRACT; @@ -7390,13 +7488,16 @@ namespace while (!sawFinalUnit) { - // Left operand: ldarg.0; ldfld F. - if (ip + 6 > codeSize || pIL[ip] != IL_LDARG_0 || pIL[ip + 1] != IL_LDFLD) + // Left operand: ldarg.0; ldfld/ldflda F. + if (ip + 6 > codeSize || pIL[ip] != IL_LDARG_0) + return false; + BYTE leftLoad = pIL[ip + 1]; + if (leftLoad != IL_LDFLD && leftLoad != IL_LDFLDA) return false; mdToken leftFieldTok = ReadILToken(pIL + ip + 2); ip += 6; - // Right operand: ldarg.1; ldfld of the same field. + // Right operand: ldarg.1; ldfld F. if (ip + 6 > codeSize || pIL[ip] != IL_LDARG_1 || pIL[ip + 1] != IL_LDFLD) return false; mdToken rightFieldTok = ReadILToken(pIL + ip + 2); @@ -7421,55 +7522,72 @@ namespace } compared[numCompared++] = pField; - // Only integer-like fields are bit-for-bit equivalent to '=='. Float/double are excluded - // (NaN != NaN, +0.0 == -0.0), and anything else (including value-type fields) is rejected. - switch (pField->GetFieldType()) - { - case ELEMENT_TYPE_BOOLEAN: - case ELEMENT_TYPE_CHAR: - case ELEMENT_TYPE_I1: - case ELEMENT_TYPE_U1: - case ELEMENT_TYPE_I2: - case ELEMENT_TYPE_U2: - case ELEMENT_TYPE_I4: - case ELEMENT_TYPE_U4: - case ELEMENT_TYPE_I8: - case ELEMENT_TYPE_U8: - case ELEMENT_TYPE_I: - case ELEMENT_TYPE_U: - case ELEMENT_TYPE_PTR: - case ELEMENT_TYPE_FNPTR: - break; - default: - return false; - } - - if (ip < codeSize && pIL[ip] == IL_BNE_UN_S) + bool nested = leftLoad == IL_LDFLDA; + if (!nested) { - // Non-final field: branch to the shared 'return false'. - if (ip + 2 > codeSize) + // Inline '==': only integer-like primitives are memcmp-equivalent. + if (!IsBitwiseComparablePrimitive(pField->GetFieldType())) return false; - int target = (int)(ip + 2) + (int)(signed char)pIL[ip + 1]; - if (falseTarget == -1) - falseTarget = target; - else if (falseTarget != target) + + if (ip < codeSize && pIL[ip] == IL_BNE_UN_S) + { + // Non-final field: branch to the shared 'return false'. + if (ip + 2 > codeSize) + return false; + int target = (int)(ip + 2) + (int)(signed char)pIL[ip + 1]; + if (falseTarget == -1) + falseTarget = target; + else if (falseTarget != target) + return false; + ip += 2; + } + else if (ip + 3 <= codeSize && pIL[ip] == IL_PREFIX1 && pIL[ip + 1] == IL_CEQ_2ND && pIL[ip + 2] == IL_RET) + { + // Final field: ceq; ret. + ip += 3; + sawFinalUnit = true; + } + else + { return false; - ip += 2; - } - else if (ip + 3 <= codeSize && pIL[ip] == IL_PREFIX1 && pIL[ip + 1] == IL_CEQ_2ND && pIL[ip + 2] == IL_RET) - { - // Final field: ceq; ret. - ip += 3; - sawFinalUnit = true; + } } else { - return false; + // Call form: a primitive's own Equals, or a nested type's field-wise Equals. + if (ip + 5 > codeSize || pIL[ip] != IL_CALL) + return false; + MethodDesc* pCallee = TryResolveMethodToken(pModule, ReadILToken(pIL + ip + 1)); + if (!IsPrimitiveEqualsCall(pCallee, pField) && !IsNestedFieldwiseEquatable(pCallee, pField)) + return false; + ip += 5; + + if (ip < codeSize && pIL[ip] == IL_BRFALSE_S) + { + // Non-final field: branch to the shared tail. + if (ip + 2 > codeSize) + return false; + int target = (int)(ip + 2) + (int)(signed char)pIL[ip + 1]; + if (falseTarget == -1) + falseTarget = target; + else if (falseTarget != target) + return false; + ip += 2; + } + else if (ip < codeSize && pIL[ip] == IL_RET) + { + // Final field: ret. + ip += 1; + sawFinalUnit = true; + } + else + { + return false; + } } } - // If there were any branches, the shared tail must be exactly 'ldc.i4.0; ret' and every - // branch must target it. A single-field comparison has no branches and no tail. + // Any branches must target the shared 'ldc.i4.0; ret' tail; a single-field compare has none. if (falseTarget != -1) { if ((int)ip != falseTarget || @@ -7495,12 +7613,10 @@ namespace { STANDARD_VM_CONTRACT; - // First pass restrictions to keep the scan simple and unquestionably safe: - // - Must be unmanaged so a byte-wise compare is meaningful. - // - No generics for now (keeps IL token resolution to in-module def tokens). - // - Must be tightly packed (no padding gaps and no overlapping fields), otherwise memcmp - // would compare bytes the field-wise Equals ignores. - // - No inline arrays. + // First-pass restrictions that keep the scan simple and unquestionably safe: the type must be + // unmanaged (so a byte-wise compare is meaningful), non-generic (so IL tokens stay in-module), + // tightly packed (no padding anywhere -- the flag is transitive -- else memcmp inspects bytes + // Equals ignores), and not an inline array. if (valueTypeMT->ContainsGCPointers() || valueTypeMT->HasInstantiation() || valueTypeMT->IsNotTightlyPacked() || @@ -7559,14 +7675,13 @@ bool IsBitwiseEquatable(TypeHandle typeHandle, MethodTable * methodTable) if (!typeHandle.CanCastTo(iequatableOfSelf)) { - // The type provides no IEquatable of its own, so bitwise equality is safe as long as the - // fields are bit-comparable and there is no object.Equals override with custom semantics. + // No IEquatable of its own: bitwise equality is safe if the fields are bit-comparable and + // there is no custom object.Equals override. return CanCompareBitsOrUseFastGetHashCode(methodTable); } - // The type provides IEquatable.Equals. It can only be treated as bitwise equatable if that - // implementation is a plain field-wise comparison equivalent to memcmp. Interface dispatch on a - // value type resolves to an unboxing stub, so unwrap it to the underlying instance method. + // Has IEquatable.Equals: bitwise only if that Equals is a plain field-wise memcmp equivalent. + // UnwrapStub turns the value-type interface dispatch into the underlying instance method. MethodDesc* pEqualsMD = UnwrapStub(methodTable->GetMethodDescForInterfaceMethod( iequatableOfSelf, CoreLibBinder::GetMethod(METHOD__IEQUATABLEGENERIC__EQUALS), FALSE /* throwOnConflict */)); if (pEqualsMD == NULL) From 5efb9f50877ed334db2718904b3f42c33494c0a5 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 14 Jul 2026 14:37:11 -0700 Subject: [PATCH 04/26] Recognize record struct EqualityComparer.Default.Equals in IsBitwiseEquatable Roslyn-generated record structs compare each field via EqualityComparer.Default.Equals(field, other.field) rather than == or F.Equals. Recognize this shape in both the VM and ILC scanners, accepting it only when F is itself bitwise-equatable. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../TypeSystem/IL/Stubs/ComparerIntrinsics.cs | 105 +++++++++++---- src/coreclr/vm/corelib.h | 1 + src/coreclr/vm/jitinterface.cpp | 127 ++++++++++++++---- 3 files changed, 181 insertions(+), 52 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs index b83f02b161b5c4..53c72cad7a398e 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs @@ -433,7 +433,8 @@ private static MethodDesc TryGetOpEqualityForward(MethodIL methodIL, MetadataTyp private static bool ScanFieldwiseEqualsBody(MethodIL methodIL, MetadataType type) { // Verifies the body is a plain field-wise equality: every instance field is compared exactly once - // with `==` and the results are ANDed together, which is equivalent to a bitwise (memcmp) comparison. + // (via `==`, its own `Equals`, or `EqualityComparer.Default.Equals` for records) and the results + // are ANDed together, which is equivalent to a bitwise (memcmp) comparison. int instanceFieldCount = 0; foreach (FieldDesc field in type.GetFields()) { @@ -452,15 +453,26 @@ private static bool ScanFieldwiseEqualsBody(MethodIL methodIL, MetadataType type while (!sawFinalCompare) { - // Each field comparison loads the field from both arguments. A primitive compared with - // `==` loads both by value (`ldfld`); a primitive compared with its own `Equals`, or a - // nested value-type field, loads the left side by address (`ldflda`) for the call. + // Optional records lead-in: `call EqualityComparer::get_Default` before the operands. + MethodDesc getDefault = null; + bool records = false; + if (reader.PeekILOpcode() == ILOpcode.call) + { + reader.ReadILOpcode(); + getDefault = methodIL.GetObject(reader.ReadILToken()) as MethodDesc; + records = true; + } + + // Left operand: `ldarg.0; ldfld/ldflda F`. Records and inline `==` load by value; the + // `.Equals` call form loads the left side by address. if (!reader.HasNext || reader.ReadILOpcode() != ILOpcode.ldarg_0) return false; ILOpcode leftLoad = reader.ReadILOpcode(); if (leftLoad != ILOpcode.ldfld && leftLoad != ILOpcode.ldflda) return false; + if (records && leftLoad != ILOpcode.ldfld) + return false; FieldDesc leftField = methodIL.GetObject(reader.ReadILToken()) as FieldDesc; if (reader.ReadILOpcode() != ILOpcode.ldarg_1) @@ -476,9 +488,9 @@ private static bool ScanFieldwiseEqualsBody(MethodIL methodIL, MetadataType type if (!comparedFields.Add(leftField)) return false; - bool nested = leftLoad == ILOpcode.ldflda; - if (!nested) + if (!records && leftLoad == ILOpcode.ldfld) { + // Inline `==`: only integer-like primitives are memcmp-equivalent. if (!IsBitwiseComparablePrimitive(leftField.FieldType)) return false; @@ -503,35 +515,46 @@ private static bool ScanFieldwiseEqualsBody(MethodIL methodIL, MetadataType type { return false; } + + continue; + } + + if (records) + { + // `callvirt EqualityComparer::Equals(!0, !0)`. + if (reader.ReadILOpcode() != ILOpcode.callvirt) + return false; + MethodDesc equals = methodIL.GetObject(reader.ReadILToken()) as MethodDesc; + if (!IsEqualityComparerDefaultEquals(getDefault, equals, leftField.FieldType)) + return false; } else { - // Call form: a primitive's own Equals, or a nested type's field-wise Equals. + // `.Equals` call form: a primitive's own Equals, or a nested type's field-wise Equals. if (reader.ReadILOpcode() != ILOpcode.call) return false; MethodDesc callee = methodIL.GetObject(reader.ReadILToken()) as MethodDesc; if (!IsPrimitiveEqualsCall(callee, leftField.FieldType) && !IsNestedFieldwiseEquatable(callee, leftField.FieldType)) return false; + } - ILOpcode compareOpcode = reader.ReadILOpcode(); - if (compareOpcode == ILOpcode.brfalse_s) - { - // Non-final field: branch to the shared tail. - int target = reader.ReadBranchDestination(compareOpcode); - if (falseTarget == -1) - falseTarget = target; - else if (falseTarget != target) - return false; - } - else if (compareOpcode == ILOpcode.ret) - { - // Final field: ret. - sawFinalCompare = true; - } - else - { + // The Equals call already yields a bool: `brfalse.s` to the shared tail, or `ret` if final. + ILOpcode terminator = reader.ReadILOpcode(); + if (terminator == ILOpcode.brfalse_s) + { + int target = reader.ReadBranchDestination(terminator); + if (falseTarget == -1) + falseTarget = target; + else if (falseTarget != target) return false; - } + } + else if (terminator == ILOpcode.ret) + { + sawFinalCompare = true; + } + else + { + return false; } } @@ -573,6 +596,38 @@ private static bool IsPrimitiveEqualsCall(MethodDesc callee, TypeDesc fieldType) && callee == GetIEquatableEqualsImplementation(primitiveType); } + private static bool IsEqualityComparerDefaultEquals(MethodDesc getDefault, MethodDesc equals, TypeDesc fieldType) + { + // Records compare each field with EqualityComparer.Default.Equals(this.F, other.F). That is + // a memcmp only when F is itself bitwise-equatable: a bit-comparable primitive, or a nested + // value type whose own IEquatable.Equals is field-wise. + if (!IsEqualityComparerMethod(getDefault, fieldType, "get_Default"u8, isStatic: true) || + !IsEqualityComparerMethod(equals, fieldType, "Equals"u8, isStatic: false)) + { + return false; + } + + if (IsBitwiseComparablePrimitive(fieldType)) + return true; + + return fieldType is MetadataType nestedType && nestedType.IsValueType + && GetIEquatableEqualsImplementation(nestedType) != null + && IsIEquatableEqualsFieldwise(nestedType); + } + + private static bool IsEqualityComparerMethod(MethodDesc method, TypeDesc fieldType, ReadOnlySpan name, bool isStatic) + { + if (method == null || method.Signature.IsStatic != isStatic || method.Name != name) + return false; + + MetadataType equalityComparer = fieldType.Context.SystemModule.GetType("System.Collections.Generic"u8, "EqualityComparer`1"u8, throwIfNotFound: false); + TypeDesc owningType = method.OwningType; + return equalityComparer != null + && owningType.GetTypeDefinition() == equalityComparer + && owningType.Instantiation.Length == 1 + && owningType.Instantiation[0] == fieldType; + } + private static bool IsBitwiseComparablePrimitive(TypeDesc fieldType) { if (fieldType.IsPrimitive || fieldType.IsEnum || fieldType.IsPointer || fieldType.IsFunctionPointer) diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index 4e54d5df520337..fcdbe8c797a14b 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -1331,6 +1331,7 @@ DEFINE_METHOD(UTF8BUFFERMARSHALER, CONVERT_TO_MANAGED, ConvertToManaged, NoSig) // Classes referenced in EqualityComparer.Default optimization +DEFINE_CLASS(EQUALITY_COMPARER, CollectionsGeneric, EqualityComparer`1) DEFINE_CLASS(STRING_EQUALITYCOMPARER, CollectionsGeneric, StringEqualityComparer) DEFINE_CLASS(ENUM_EQUALITYCOMPARER, CollectionsGeneric, EnumEqualityComparer`1) DEFINE_CLASS(NULLABLE_EQUALITYCOMPARER, CollectionsGeneric, NullableEqualityComparer`1) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 23feda2018f26e..7a9db07eb46606 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -7300,6 +7300,7 @@ namespace IL_RET = 0x2A, IL_BRFALSE_S = 0x2C, IL_BNE_UN_S = 0x33, + IL_CALLVIRT = 0x6F, IL_LDOBJ = 0x71, IL_LDFLD = 0x7B, IL_LDFLDA = 0x7C, @@ -7442,18 +7443,66 @@ namespace return IsFieldwiseEqualsBitwiseEquivalent(pNestedMT, pNestedEquals); } + // True if 'pMD' is 'EqualityComparer::name' with the expected static-ness. Records call + // through this base type: 'get_Default' (static) and the abstract 'Equals' (instance). + bool IsEqualityComparerMethod(MethodDesc* pMD, TypeHandle fieldTh, const char* name, bool isStatic) + { + STANDARD_VM_CONTRACT; + + if (pMD == NULL) + return false; + + MethodTable* pMT = pMD->GetMethodTable(); + if (pMT == NULL || !pMT->HasSameTypeDefAs(CoreLibBinder::GetClass(CLASS__EQUALITY_COMPARER))) + return false; + + Instantiation inst = pMT->GetInstantiation(); + if (inst.GetNumArgs() != 1 || inst[0] != fieldTh) + return false; + + return (pMD->IsStatic() != FALSE) == isStatic && strcmp(pMD->GetName(), name) == 0; + } + + // Accepts a field compared via 'EqualityComparer.Default.Equals(this.F, other.F)' (what Roslyn + // emits for record structs), but only when Default.Equals is itself a memcmp: F must be a + // bit-comparable primitive or a nested value type that is itself provably field-wise. + bool IsEqualityComparerDefaultEquals(MethodDesc* pGetDefault, MethodDesc* pEquals, FieldDesc* pField) + { + STANDARD_VM_CONTRACT; + + TypeHandle fieldTh = pField->GetFieldTypeHandleThrowing(); + if (!IsEqualityComparerMethod(pGetDefault, fieldTh, "get_Default", true /* isStatic */) || + !IsEqualityComparerMethod(pEquals, fieldTh, "Equals", false /* isStatic */)) + { + return false; + } + + if (IsBitwiseComparablePrimitive(pField->GetFieldType())) + return true; + + if (pField->GetFieldType() != ELEMENT_TYPE_VALUETYPE) + return false; + + MethodTable* pNestedMT = fieldTh.GetMethodTable(); + MethodDesc* pNestedEquals = GetIEquatableEqualsImpl(pNestedMT); + return pNestedEquals != NULL && IsFieldwiseEqualsBitwiseEquivalent(pNestedMT, pNestedEquals); + } + // Returns true only if 'pEqualsMD' compares every instance field of 'valueTypeMT' exactly once and // ANDs the results, bit-for-bit like memcmp. Combined with the caller's 'tightly packed' guarantee, // that makes the whole comparison a memcmp. // // The C# compiler lowers 'this.f0 == other.f0 && ...' to per-field units sharing one 'return false' // tail. Operands are always arg0/arg1. A primitive is compared inline; a nested value type through - // its own IEquatable.Equals, which must itself be field-wise (checked recursively): + // its own IEquatable.Equals; a record struct field through EqualityComparer.Default.Equals. + // Every call-form callee must itself be field-wise (checked recursively): // // primitive, non-final: ldarg.0; ldfld F; ldarg.1; ldfld F; bne.un.s FALSE // primitive, final: ldarg.0; ldfld F; ldarg.1; ldfld F; ceq; ret // nested, non-final: ldarg.0; ldflda F; ldarg.1; ldfld F; call F::Equals; brfalse.s FALSE // nested, final: ldarg.0; ldflda F; ldarg.1; ldfld F; call F::Equals; ret + // record, non-final: call EqualityComparer::get_Default; ldarg.0; ldfld F; ldarg.1; ldfld F; callvirt Equals; brfalse.s FALSE + // record, final: call EqualityComparer::get_Default; ldarg.0; ldfld F; ldarg.1; ldfld F; callvirt Equals; ret // shared tail: FALSE: ldc.i4.0; ret bool ScanFieldwiseEqualsBody(MethodDesc* pEqualsMD, MethodTable* valueTypeMT) { @@ -7488,12 +7537,25 @@ namespace while (!sawFinalUnit) { - // Left operand: ldarg.0; ldfld/ldflda F. + // Optional records lead-in: 'call EqualityComparer::get_Default' before the operands. + mdToken getDefaultTok = mdTokenNil; + bool records = false; + if (ip + 5 <= codeSize && pIL[ip] == IL_CALL) + { + getDefaultTok = ReadILToken(pIL + ip + 1); + records = true; + ip += 5; + } + + // Left operand: ldarg.0; ldfld/ldflda F. Records and inline '==' load by value; the + // '.Equals' call form loads by address. if (ip + 6 > codeSize || pIL[ip] != IL_LDARG_0) return false; BYTE leftLoad = pIL[ip + 1]; if (leftLoad != IL_LDFLD && leftLoad != IL_LDFLDA) return false; + if (records && leftLoad != IL_LDFLD) + return false; mdToken leftFieldTok = ReadILToken(pIL + ip + 2); ip += 6; @@ -7522,8 +7584,7 @@ namespace } compared[numCompared++] = pField; - bool nested = leftLoad == IL_LDFLDA; - if (!nested) + if (!records && leftLoad == IL_LDFLD) { // Inline '==': only integer-like primitives are memcmp-equivalent. if (!IsBitwiseComparablePrimitive(pField->GetFieldType())) @@ -7551,39 +7612,51 @@ namespace { return false; } + + continue; + } + + if (records) + { + // callvirt EqualityComparer::Equals(!0, !0). + if (ip + 5 > codeSize || pIL[ip] != IL_CALLVIRT) + return false; + MethodDesc* pGetDefault = TryResolveMethodToken(pModule, getDefaultTok); + MethodDesc* pEquals = TryResolveMethodToken(pModule, ReadILToken(pIL + ip + 1)); + if (!IsEqualityComparerDefaultEquals(pGetDefault, pEquals, pField)) + return false; } else { - // Call form: a primitive's own Equals, or a nested type's field-wise Equals. + // '.Equals' call form: a primitive's own Equals, or a nested type's field-wise Equals. if (ip + 5 > codeSize || pIL[ip] != IL_CALL) return false; MethodDesc* pCallee = TryResolveMethodToken(pModule, ReadILToken(pIL + ip + 1)); if (!IsPrimitiveEqualsCall(pCallee, pField) && !IsNestedFieldwiseEquatable(pCallee, pField)) return false; - ip += 5; + } + ip += 5; - if (ip < codeSize && pIL[ip] == IL_BRFALSE_S) - { - // Non-final field: branch to the shared tail. - if (ip + 2 > codeSize) - return false; - int target = (int)(ip + 2) + (int)(signed char)pIL[ip + 1]; - if (falseTarget == -1) - falseTarget = target; - else if (falseTarget != target) - return false; - ip += 2; - } - else if (ip < codeSize && pIL[ip] == IL_RET) - { - // Final field: ret. - ip += 1; - sawFinalUnit = true; - } - else - { + // The Equals call already yields a bool: brfalse.s to the shared tail, or ret if final. + if (ip < codeSize && pIL[ip] == IL_BRFALSE_S) + { + if (ip + 2 > codeSize) return false; - } + int target = (int)(ip + 2) + (int)(signed char)pIL[ip + 1]; + if (falseTarget == -1) + falseTarget = target; + else if (falseTarget != target) + return false; + ip += 2; + } + else if (ip < codeSize && pIL[ip] == IL_RET) + { + ip += 1; + sawFinalUnit = true; + } + else + { + return false; } } From ec0e64e36c8c582bbeb97ee35879cca8d3d338bf Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 14 Jul 2026 16:18:38 -0700 Subject: [PATCH 05/26] Keep inline array types uncached in CanCompareBitsOrUseFastGetHashCode ValueType.Equals/GetHashCode throw NotSupportedException for inline arrays, but only via the QCALL entry point and only while the type's CanCompareBits flag is unchecked. The internal helper previously cached 'false' for inline arrays alongside GC/not-tightly-packed types. That was latent until the IsNotTightlyPacked fix let a struct wrapping an inline array be tightly packed, so CanCompareBitsOrUseFastGetHashCode now recurses into the inline array field and caches its flag, permanently suppressing the throw for that type. Return false without caching for inline arrays so the managed fast path keeps routing through the throwing QCALL. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/comutilnative.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/comutilnative.cpp b/src/coreclr/vm/comutilnative.cpp index 4412dfaec4eadf..52d26ddbe8dcf5 100644 --- a/src/coreclr/vm/comutilnative.cpp +++ b/src/coreclr/vm/comutilnative.cpp @@ -1775,9 +1775,17 @@ BOOL CanCompareBitsOrUseFastGetHashCode(MethodTable* mt) return mt->CanCompareBitsOrUseFastGetHashCode(); } + if (mt->GetClass()->IsInlineArray()) + { + // Inline arrays must always throw from ValueType.Equals/GetHashCode, which only happens on the + // QCALL entry point. Return false without caching so the managed fast path keeps routing there + // instead of reading a cached 'false' (e.g. primed by an enclosing type's field recursion) that + // would silently skip the throw. + return FALSE; + } + if (mt->ContainsGCPointers() - || mt->IsNotTightlyPacked() - || mt->GetClass()->IsInlineArray()) + || mt->IsNotTightlyPacked()) { mt->SetHasCheckedCanCompareBitsOrUseFastGetHashCode(); return FALSE; From dbc47aaa5ddabc61eeb66267a97778c0303cc66d Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 14 Jul 2026 16:18:45 -0700 Subject: [PATCH 06/26] Address review feedback on token decode and comments Cast bytes to uint32_t before shifting in ReadILToken so decoding a token with the high bit set is well-defined, and correct a stale ILC comment that referenced CanCompareValueTypeBits instead of the field-wise scan. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs | 2 +- src/coreclr/vm/jitinterface.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs index 53c72cad7a398e..98c78b6e3c50b6 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs @@ -575,7 +575,7 @@ private static bool ScanFieldwiseEqualsBody(MethodIL methodIL, MetadataType type private static bool IsNestedFieldwiseEquatable(MethodDesc callee, TypeDesc fieldType) { // The nested field must be compared through the nested type's own IEquatable.Equals, and - // that Equals must itself be field-wise. Its layout is validated by CanCompareValueTypeBits. + // that Equals must itself be field-wise (its layout is validated by IsIEquatableEqualsFieldwise). if (callee == null || fieldType is not MetadataType nestedType || !nestedType.IsValueType) return false; diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 7a9db07eb46606..b61011159c6140 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -7311,7 +7311,7 @@ namespace mdToken ReadILToken(const BYTE* pIL) { LIMITED_METHOD_CONTRACT; - return (mdToken)(pIL[0] | (pIL[1] << 8) | (pIL[2] << 16) | (pIL[3] << 24)); + return (mdToken)((uint32_t)pIL[0] | ((uint32_t)pIL[1] << 8) | ((uint32_t)pIL[2] << 16) | ((uint32_t)pIL[3] << 24)); } // Resolves an in-module FieldDef token. Returns NULL for anything else (e.g. a MemberRef, From 4b35aa78b211361ab625e6463320ebe71ff93195 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 14 Jul 2026 16:53:42 -0700 Subject: [PATCH 07/26] Guard the ILC field-wise scan against a truncated IL stream Check ILReader.HasNext before PeekILOpcode at the top of the scan loop so a malformed or truncated body returns false instead of indexing past the IL. Also drop the unnecessary CS660/CS661 suppressions in the test asset (the only operator == also overrides Equals/GetHashCode). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs | 3 +++ .../EquatableAssets/BitwiseEquatable.cs | 2 -- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs index 98c78b6e3c50b6..35e7ce56f209e7 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs @@ -453,6 +453,9 @@ private static bool ScanFieldwiseEqualsBody(MethodIL methodIL, MetadataType type while (!sawFinalCompare) { + if (!reader.HasNext) + return false; + // Optional records lead-in: `call EqualityComparer::get_Default` before the operands. MethodDesc getDefault = null; bool records = false; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/BitwiseEquatable.cs b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/BitwiseEquatable.cs index eb84a2c5e7134b..9ea245c440fc9e 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/BitwiseEquatable.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/BitwiseEquatable.cs @@ -4,8 +4,6 @@ using System; #pragma warning disable 649 // field is never assigned to -#pragma warning disable 660 // defines operator == but not Equals -#pragma warning disable 661 // defines operator == but not GetHashCode namespace BitwiseEquatable { From 06fbe3900266e989e60141b325c1721b9289a435 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 14 Jul 2026 17:11:25 -0700 Subject: [PATCH 08/26] Add CoreCLR test coverage for IsBitwiseEquatable Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../BitwiseEquatable/BitwiseEquatable.cs | 246 ++++++++++++++++++ .../BitwiseEquatable/BitwiseEquatable.csproj | 5 + 2 files changed, 251 insertions(+) create mode 100644 src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs create mode 100644 src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.csproj diff --git a/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs new file mode 100644 index 00000000000000..ee1a89dc019024 --- /dev/null +++ b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs @@ -0,0 +1,246 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Reflection; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; + +using Xunit; + +#pragma warning disable CS0649 // field is never assigned to + +namespace BitwiseEquatableTests +{ + public static class BitwiseEquatable + { + private static readonly MethodInfo s_isBitwiseEquatable = + typeof(RuntimeHelpers).GetMethod("IsBitwiseEquatable", BindingFlags.Static | BindingFlags.NonPublic)!; + + private static bool IsBitwiseEquatable(Type t) => + (bool)s_isBitwiseEquatable.MakeGenericMethod(t).Invoke(null, null)!; + + [Theory] + // Primitives: '==' and Equals lower to the same bit-for-bit compare. + [InlineData(typeof(int), true)] + [InlineData(typeof(Int128), true)] + [InlineData(typeof(UInt128), true)] + // A SIMD/Unsafe-backed body is not a recognized field-wise shape. + [InlineData(typeof(Guid), false)] + // Plain field-wise IEquatable.Equals. + [InlineData(typeof(Point), true)] + [InlineData(typeof(ThreeFields), true)] + [InlineData(typeof(OneField), true)] + // 'Equals(other) => this == other' forwarding into a field-wise op_Equality. + [InlineData(typeof(ForwardsToOp), true)] + // Nested value-type fields compared through their own field-wise IEquatable.Equals. + [InlineData(typeof(Nested), true)] + [InlineData(typeof(NestedLast), true)] + [InlineData(typeof(AllNested), true)] + // Nested type's Equals ignores a field, or is internally padded. + [InlineData(typeof(WrapsPartial), false)] + [InlineData(typeof(WrapsPadded), false)] + // No IEquatable at all: legacy path still accepts safe blittable fields. + [InlineData(typeof(PlainNoEquatable), true)] + // float/double are never bitwise (NaN and signed-zero semantics differ from memcmp). + [InlineData(typeof(HasFloat), false)] + // Equals ignores a field, does custom logic, or forwards to a non-op_Equality helper. + [InlineData(typeof(IgnoresField), false)] + [InlineData(typeof(CustomLogic), false)] + [InlineData(typeof(CallsHelper), false)] + // Explicit padding means memcmp inspects bytes Equals does not. + [InlineData(typeof(WithPadding), false)] + // Overrides object.Equals only; no IEquatable. + [InlineData(typeof(OverriddenOnly), false)] + // Primitive fields compared via '.Equals' rather than '=='. + [InlineData(typeof(PrimEquals), true)] + [InlineData(typeof(MixedEquals), true)] + [InlineData(typeof(FloatEquals), false)] + // Record structs: Roslyn emits EqualityComparer.Default.Equals(this.F, other.F) per field. + [InlineData(typeof(RecTwo), true)] + [InlineData(typeof(RecNested), true)] + [InlineData(typeof(RecMixed), true)] + [InlineData(typeof(RecPadded), false)] + [InlineData(typeof(RecFloat), false)] + public static void IsBitwiseEquatable_MatchesExpected(Type type, bool expected) + { + Assert.Equal(expected, IsBitwiseEquatable(type)); + } + + // The following structs have no Equals/GetHashCode override, so ValueType.Equals/GetHashCode go + // through CanCompareBitsOrUseFastGetHashCode. The IsNotTightlyPacked fix moved a struct with a + // multi-byte value-type field off the reflection slow path onto the memcmp fast path; either way + // the result must match the obvious value semantics. + + [Fact] + public static void TightlyPacked_EqualsAndHash_AreConsistent() + { + var a = new PlainOuter { X = new PlainInner { A = 1, B = 2 }, C = 3 }; + var b = new PlainOuter { X = new PlainInner { A = 1, B = 2 }, C = 3 }; + var c = new PlainOuter { X = new PlainInner { A = 1, B = 9 }, C = 3 }; + + Assert.True(a.Equals(b)); + Assert.False(a.Equals(c)); + Assert.Equal(a.GetHashCode(), b.GetHashCode()); + } + } + + public struct PlainInner { public int A; public int B; } + public struct PlainOuter { public PlainInner X; public int C; } + + public record struct RecTwo(int X, int Y); + public record struct RecNested(RecTwo P, long Z); + public record struct RecMixed(long A, int B, short C, short D); + public record struct RecPadded(int X, byte Y); + public record struct RecFloat(float X, int Y); + + public readonly struct Point : IEquatable + { + public readonly int X; public readonly int Y; + public bool Equals(Point o) => X == o.X && Y == o.Y; + public override bool Equals(object o) => o is Point p && Equals(p); + public override int GetHashCode() => 0; + } + + public readonly struct ThreeFields : IEquatable + { + public readonly int A; public readonly int B; public readonly int C; + public bool Equals(ThreeFields o) => A == o.A && B == o.B && C == o.C; + public override bool Equals(object o) => o is ThreeFields p && Equals(p); + public override int GetHashCode() => 0; + } + + public readonly struct OneField : IEquatable + { + public readonly long V; + public bool Equals(OneField o) => V == o.V; + public override bool Equals(object o) => o is OneField p && Equals(p); + public override int GetHashCode() => 0; + } + + public readonly struct ForwardsToOp : IEquatable + { + public readonly int Lo; public readonly int Hi; + public bool Equals(ForwardsToOp o) => this == o; + public static bool operator ==(ForwardsToOp a, ForwardsToOp b) => a.Lo == b.Lo && a.Hi == b.Hi; + public static bool operator !=(ForwardsToOp a, ForwardsToOp b) => !(a == b); + public override bool Equals(object o) => o is ForwardsToOp p && Equals(p); + public override int GetHashCode() => 0; + } + + public readonly struct Nested : IEquatable + { + public readonly Point P; public readonly int Z; + public bool Equals(Nested o) => P.Equals(o.P) && Z == o.Z; + public override bool Equals(object o) => o is Nested p && Equals(p); + public override int GetHashCode() => 0; + } + + public readonly struct NestedLast : IEquatable + { + public readonly int Z; public readonly Point P; + public bool Equals(NestedLast o) => Z == o.Z && P.Equals(o.P); + public override bool Equals(object o) => o is NestedLast p && Equals(p); + public override int GetHashCode() => 0; + } + + public readonly struct AllNested : IEquatable + { + public readonly Point A; public readonly OneField B; + public bool Equals(AllNested o) => A.Equals(o.A) && B.Equals(o.B); + public override bool Equals(object o) => o is AllNested p && Equals(p); + public override int GetHashCode() => 0; + } + + public readonly struct WrapsPartial : IEquatable + { + public readonly IgnoresField A; public readonly int Z; + public bool Equals(WrapsPartial o) => A.Equals(o.A) && Z == o.Z; + public override bool Equals(object o) => o is WrapsPartial p && Equals(p); + public override int GetHashCode() => 0; + } + + public readonly struct WrapsPadded : IEquatable + { + public readonly WithPadding A; public readonly int Z; + public bool Equals(WrapsPadded o) => A.Equals(o.A) && Z == o.Z; + public override bool Equals(object o) => o is WrapsPadded p && Equals(p); + public override int GetHashCode() => 0; + } + + public struct PlainNoEquatable { public int A; public int B; } + + public readonly struct HasFloat : IEquatable + { + public readonly int A; public readonly float F; + public bool Equals(HasFloat o) => A == o.A && F == o.F; + public override bool Equals(object o) => o is HasFloat p && Equals(p); + public override int GetHashCode() => 0; + } + + public readonly struct IgnoresField : IEquatable + { + public readonly int A; public readonly int B; + public bool Equals(IgnoresField o) => A == o.A; + public override bool Equals(object o) => o is IgnoresField p && Equals(p); + public override int GetHashCode() => 0; + } + + public readonly struct CustomLogic : IEquatable + { + public readonly int A; + public bool Equals(CustomLogic o) => (A & 0xF) == (o.A & 0xF); + public override bool Equals(object o) => o is CustomLogic p && Equals(p); + public override int GetHashCode() => 0; + } + + public readonly struct CallsHelper : IEquatable + { + public readonly int A; public readonly int B; + public bool Equals(CallsHelper o) => Cmp(this, o); + private static bool Cmp(CallsHelper a, CallsHelper b) => a.A == b.A && a.B == b.B; + public override bool Equals(object o) => o is CallsHelper p && Equals(p); + public override int GetHashCode() => 0; + } + + [StructLayout(LayoutKind.Explicit, Size = 16)] + public readonly struct WithPadding : IEquatable + { + [FieldOffset(0)] public readonly byte A; + [FieldOffset(8)] public readonly int B; + public bool Equals(WithPadding o) => A == o.A && B == o.B; + public override bool Equals(object o) => o is WithPadding p && Equals(p); + public override int GetHashCode() => 0; + } + + public struct OverriddenOnly + { + public int A; + public override bool Equals(object o) => o is OverriddenOnly p && p.A == A; + public override int GetHashCode() => A; + } + + public readonly struct PrimEquals : IEquatable + { + public readonly byte A; public readonly sbyte B; public readonly short C; public readonly int D; public readonly long E; + public bool Equals(PrimEquals o) => A.Equals(o.A) && B.Equals(o.B) && C.Equals(o.C) && D.Equals(o.D) && E.Equals(o.E); + public override bool Equals(object o) => o is PrimEquals p && Equals(p); + public override int GetHashCode() => 0; + } + + public readonly struct MixedEquals : IEquatable + { + public readonly int A; public readonly int B; + public bool Equals(MixedEquals o) => A == o.A && B.Equals(o.B); + public override bool Equals(object o) => o is MixedEquals p && Equals(p); + public override int GetHashCode() => 0; + } + + public readonly struct FloatEquals : IEquatable + { + public readonly int A; public readonly float F; + public bool Equals(FloatEquals o) => A.Equals(o.A) && F.Equals(o.F); + public override bool Equals(object o) => o is FloatEquals p && Equals(p); + public override int GetHashCode() => 0; + } +} diff --git a/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.csproj b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.csproj new file mode 100644 index 00000000000000..c5d04fb92dd7dc --- /dev/null +++ b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.csproj @@ -0,0 +1,5 @@ + + + + + From 2c79ee35740d134f9a62d9b47076a1b28f8b8722 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 14 Jul 2026 17:46:16 -0700 Subject: [PATCH 09/26] Force optimized IL and correct Guid case in the CoreCLR test The field-wise scan matches Roslyn's optimized IL shape, so the test must be built optimized (Debug otherwise defaults Optimize to false). Guid is a known bitwise-equatable type special-cased by the runtime, so it reports true. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs | 5 +++-- .../classloader/BitwiseEquatable/BitwiseEquatable.csproj | 5 +++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs index ee1a89dc019024..c27826e8f125af 100644 --- a/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs +++ b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs @@ -25,8 +25,9 @@ private static bool IsBitwiseEquatable(Type t) => [InlineData(typeof(int), true)] [InlineData(typeof(Int128), true)] [InlineData(typeof(UInt128), true)] - // A SIMD/Unsafe-backed body is not a recognized field-wise shape. - [InlineData(typeof(Guid), false)] + // A SIMD/Unsafe-backed body isn't a recognized field-wise shape, but Guid is a known + // bitwise-equatable type special-cased by the runtime (matching NativeAOT), so it stays true. + [InlineData(typeof(Guid), true)] // Plain field-wise IEquatable.Equals. [InlineData(typeof(Point), true)] [InlineData(typeof(ThreeFields), true)] diff --git a/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.csproj b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.csproj index c5d04fb92dd7dc..885c4b556f36b0 100644 --- a/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.csproj +++ b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.csproj @@ -1,4 +1,9 @@ + + + true + From 0f7aa7ee7feaffe3dbe28cf3cb9ae62f95d4bf11 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 14 Jul 2026 17:57:41 -0700 Subject: [PATCH 10/26] Gate the CoreCLR IsBitwiseEquatable test off Mono Mono's IsBitwiseEquatable intrinsic only recognizes non-float primitive types, so the Guid, Int128, struct, and record cases would fail there. Mark the project unsupported on Mono until it matches CoreCLR semantics. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../classloader/BitwiseEquatable/BitwiseEquatable.csproj | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.csproj b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.csproj index 885c4b556f36b0..e84fc8869ba4ea 100644 --- a/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.csproj +++ b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.csproj @@ -3,6 +3,9 @@ true + + true From 5a57cd2427bb486caa18a03eddd261f038aa466a Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 14 Jul 2026 20:53:00 -0700 Subject: [PATCH 11/26] Treat enum fields as bitwise-comparable in the VM Equals scanner FieldDesc::GetFieldType() reports ELEMENT_TYPE_VALUETYPE for enum fields, so the scanner rejected structs whose field-wise Equals compares enums -- diverging from the ILC scanner, which already accepts enums. Resolve the enum's integer-backed underlying type explicitly across all three field shapes (inline ==, primitive .Equals, and record EqualityComparer.Equals). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/jitinterface.cpp | 29 ++++++++++++++++--- .../BitwiseEquatable/BitwiseEquatable.cs | 24 +++++++++++++++ 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index b61011159c6140..7d7fbaa6e62ef0 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -7409,13 +7409,34 @@ namespace } } + // A field is memcmp-comparable if it is a bit-comparable primitive or an enum (always integer-backed). + // FieldDesc::GetFieldType() reports ELEMENT_TYPE_VALUETYPE for enums, so resolve the underlying type + // explicitly -- this keeps the VM in parity with the managed IsBitwiseComparablePrimitive. + bool IsBitwiseComparableField(FieldDesc* pField) + { + STANDARD_VM_CONTRACT; + + CorElementType et = pField->GetFieldType(); + if (IsBitwiseComparablePrimitive(et)) + return true; + + if (et == ELEMENT_TYPE_VALUETYPE) + { + MethodTable* pFieldMT = pField->GetFieldTypeHandleThrowing().GetMethodTable(); + if (pFieldMT != NULL && pFieldMT->IsEnum()) + return IsBitwiseComparablePrimitive(pFieldMT->GetInternalCorElementType()); + } + + return false; + } + // Accepts a primitive field compared via 'x.Equals(y)' instead of 'x == y'; for these integer-like // types both lower to the same bit-for-bit compare. bool IsPrimitiveEqualsCall(MethodDesc* pCallee, FieldDesc* pField) { STANDARD_VM_CONTRACT; - if (pCallee == NULL || !IsBitwiseComparablePrimitive(pField->GetFieldType())) + if (pCallee == NULL || !IsBitwiseComparableField(pField)) return false; MethodTable* pFieldMT = pField->GetFieldTypeHandleThrowing().GetMethodTable(); @@ -7477,7 +7498,7 @@ namespace return false; } - if (IsBitwiseComparablePrimitive(pField->GetFieldType())) + if (IsBitwiseComparableField(pField)) return true; if (pField->GetFieldType() != ELEMENT_TYPE_VALUETYPE) @@ -7586,8 +7607,8 @@ namespace if (!records && leftLoad == IL_LDFLD) { - // Inline '==': only integer-like primitives are memcmp-equivalent. - if (!IsBitwiseComparablePrimitive(pField->GetFieldType())) + // Inline '==': only integer-like primitives (and enums) are memcmp-equivalent. + if (!IsBitwiseComparableField(pField)) return false; if (ip < codeSize && pIL[ip] == IL_BNE_UN_S) diff --git a/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs index c27826e8f125af..3539e1078c72aa 100644 --- a/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs +++ b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs @@ -63,6 +63,10 @@ private static bool IsBitwiseEquatable(Type t) => [InlineData(typeof(RecMixed), true)] [InlineData(typeof(RecPadded), false)] [InlineData(typeof(RecFloat), false)] + // Enum fields are integer-backed, so they compare bitwise like their underlying primitive. + [InlineData(typeof(EnumPair), true)] + [InlineData(typeof(EnumAndInt), true)] + [InlineData(typeof(RecEnum), true)] public static void IsBitwiseEquatable_MatchesExpected(Type type, bool expected) { Assert.Equal(expected, IsBitwiseEquatable(type)); @@ -94,6 +98,26 @@ public record struct RecNested(RecTwo P, long Z); public record struct RecMixed(long A, int B, short C, short D); public record struct RecPadded(int X, byte Y); public record struct RecFloat(float X, int Y); + public record struct RecEnum(ColorInt A, ColorInt B); + + // Int-backed enum: two fields pack to 8 bytes with no padding. + public enum ColorInt { A, B, C } + + public readonly struct EnumPair : IEquatable + { + public readonly ColorInt First; public readonly ColorInt Second; + public bool Equals(EnumPair o) => First == o.First && Second == o.Second; + public override bool Equals(object o) => o is EnumPair p && Equals(p); + public override int GetHashCode() => 0; + } + + public readonly struct EnumAndInt : IEquatable + { + public readonly int X; public readonly ColorInt E; + public bool Equals(EnumAndInt o) => X == o.X && E == o.E; + public override bool Equals(object o) => o is EnumAndInt p && Equals(p); + public override int GetHashCode() => 0; + } public readonly struct Point : IEquatable { From 16b9789bb1b3339327ea598e4a9a05c29c54ac4e Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 14 Jul 2026 20:53:07 -0700 Subject: [PATCH 12/26] Guard the ILC field-wise scan against malformed IL ILReader throws InvalidProgramException on a truncated stream, so the field-wise scan could throw instead of conservatively returning false. Catch it at the scan boundary -- more robust than per-read HasNext checks, which don't cover the multi-byte token reads. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../TypeSystem/IL/Stubs/ComparerIntrinsics.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs index 35e7ce56f209e7..0b6a9a4a51a33b 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs @@ -431,6 +431,19 @@ private static MethodDesc TryGetOpEqualityForward(MethodIL methodIL, MetadataTyp } private static bool ScanFieldwiseEqualsBody(MethodIL methodIL, MetadataType type) + { + try + { + return ScanFieldwiseEqualsBodyCore(methodIL, type); + } + catch (TypeSystemException.InvalidProgramException) + { + // Malformed or truncated IL: stay conservative and treat it as not field-wise. + return false; + } + } + + private static bool ScanFieldwiseEqualsBodyCore(MethodIL methodIL, MetadataType type) { // Verifies the body is a plain field-wise equality: every instance field is compared exactly once // (via `==`, its own `Equals`, or `EqualityComparer.Default.Equals` for records) and the results From aeeebda87f420226a43d533b90ba55318a908499 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 14 Jul 2026 21:07:08 -0700 Subject: [PATCH 13/26] Use MemberData so the test builds in the merged runner Merged test assemblies reference each test via an extern alias. The generated runner emits [InlineData] typeof arguments without that alias, so typeof() of a test-local type failed to resolve (CS0246) in the merged Loader runner. Move the cases to a [MemberData] source, which keeps those typeofs in this assembly. Verified by building the full Loader tree. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../BitwiseEquatable/BitwiseEquatable.cs | 102 ++++++++++-------- 1 file changed, 56 insertions(+), 46 deletions(-) diff --git a/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs index 3539e1078c72aa..9a0b88cdafebd1 100644 --- a/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs +++ b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -20,53 +21,62 @@ public static class BitwiseEquatable private static bool IsBitwiseEquatable(Type t) => (bool)s_isBitwiseEquatable.MakeGenericMethod(t).Invoke(null, null)!; + // typeof() of a test-local type can't be used in [InlineData] here: this project is compiled + // into a merged test assembly that references each test via an extern alias, and the generated + // runner can't alias the inlined typeof argument. [MemberData] keeps those typeofs in this + // assembly, so the cases stay data-driven. + public static IEnumerable Cases() + { + // Primitives: '==' and Equals lower to the same bit-for-bit compare. + yield return new object[] { typeof(int), true }; + yield return new object[] { typeof(Int128), true }; + yield return new object[] { typeof(UInt128), true }; + // A SIMD/Unsafe-backed body isn't a recognized field-wise shape, but Guid is a known + // bitwise-equatable type special-cased by the runtime (matching NativeAOT), so it stays true. + yield return new object[] { typeof(Guid), true }; + // Plain field-wise IEquatable.Equals. + yield return new object[] { typeof(Point), true }; + yield return new object[] { typeof(ThreeFields), true }; + yield return new object[] { typeof(OneField), true }; + // 'Equals(other) => this == other' forwarding into a field-wise op_Equality. + yield return new object[] { typeof(ForwardsToOp), true }; + // Nested value-type fields compared through their own field-wise IEquatable.Equals. + yield return new object[] { typeof(Nested), true }; + yield return new object[] { typeof(NestedLast), true }; + yield return new object[] { typeof(AllNested), true }; + // Nested type's Equals ignores a field, or is internally padded. + yield return new object[] { typeof(WrapsPartial), false }; + yield return new object[] { typeof(WrapsPadded), false }; + // No IEquatable at all: legacy path still accepts safe blittable fields. + yield return new object[] { typeof(PlainNoEquatable), true }; + // float/double are never bitwise (NaN and signed-zero semantics differ from memcmp). + yield return new object[] { typeof(HasFloat), false }; + // Equals ignores a field, does custom logic, or forwards to a non-op_Equality helper. + yield return new object[] { typeof(IgnoresField), false }; + yield return new object[] { typeof(CustomLogic), false }; + yield return new object[] { typeof(CallsHelper), false }; + // Explicit padding means memcmp inspects bytes Equals does not. + yield return new object[] { typeof(WithPadding), false }; + // Overrides object.Equals only; no IEquatable. + yield return new object[] { typeof(OverriddenOnly), false }; + // Primitive fields compared via '.Equals' rather than '=='. + yield return new object[] { typeof(PrimEquals), true }; + yield return new object[] { typeof(MixedEquals), true }; + yield return new object[] { typeof(FloatEquals), false }; + // Record structs: Roslyn emits EqualityComparer.Default.Equals(this.F, other.F) per field. + yield return new object[] { typeof(RecTwo), true }; + yield return new object[] { typeof(RecNested), true }; + yield return new object[] { typeof(RecMixed), true }; + yield return new object[] { typeof(RecPadded), false }; + yield return new object[] { typeof(RecFloat), false }; + // Enum fields are integer-backed, so they compare bitwise like their underlying primitive. + yield return new object[] { typeof(EnumPair), true }; + yield return new object[] { typeof(EnumAndInt), true }; + yield return new object[] { typeof(RecEnum), true }; + } + [Theory] - // Primitives: '==' and Equals lower to the same bit-for-bit compare. - [InlineData(typeof(int), true)] - [InlineData(typeof(Int128), true)] - [InlineData(typeof(UInt128), true)] - // A SIMD/Unsafe-backed body isn't a recognized field-wise shape, but Guid is a known - // bitwise-equatable type special-cased by the runtime (matching NativeAOT), so it stays true. - [InlineData(typeof(Guid), true)] - // Plain field-wise IEquatable.Equals. - [InlineData(typeof(Point), true)] - [InlineData(typeof(ThreeFields), true)] - [InlineData(typeof(OneField), true)] - // 'Equals(other) => this == other' forwarding into a field-wise op_Equality. - [InlineData(typeof(ForwardsToOp), true)] - // Nested value-type fields compared through their own field-wise IEquatable.Equals. - [InlineData(typeof(Nested), true)] - [InlineData(typeof(NestedLast), true)] - [InlineData(typeof(AllNested), true)] - // Nested type's Equals ignores a field, or is internally padded. - [InlineData(typeof(WrapsPartial), false)] - [InlineData(typeof(WrapsPadded), false)] - // No IEquatable at all: legacy path still accepts safe blittable fields. - [InlineData(typeof(PlainNoEquatable), true)] - // float/double are never bitwise (NaN and signed-zero semantics differ from memcmp). - [InlineData(typeof(HasFloat), false)] - // Equals ignores a field, does custom logic, or forwards to a non-op_Equality helper. - [InlineData(typeof(IgnoresField), false)] - [InlineData(typeof(CustomLogic), false)] - [InlineData(typeof(CallsHelper), false)] - // Explicit padding means memcmp inspects bytes Equals does not. - [InlineData(typeof(WithPadding), false)] - // Overrides object.Equals only; no IEquatable. - [InlineData(typeof(OverriddenOnly), false)] - // Primitive fields compared via '.Equals' rather than '=='. - [InlineData(typeof(PrimEquals), true)] - [InlineData(typeof(MixedEquals), true)] - [InlineData(typeof(FloatEquals), false)] - // Record structs: Roslyn emits EqualityComparer.Default.Equals(this.F, other.F) per field. - [InlineData(typeof(RecTwo), true)] - [InlineData(typeof(RecNested), true)] - [InlineData(typeof(RecMixed), true)] - [InlineData(typeof(RecPadded), false)] - [InlineData(typeof(RecFloat), false)] - // Enum fields are integer-backed, so they compare bitwise like their underlying primitive. - [InlineData(typeof(EnumPair), true)] - [InlineData(typeof(EnumAndInt), true)] - [InlineData(typeof(RecEnum), true)] + [MemberData(nameof(Cases))] public static void IsBitwiseEquatable_MatchesExpected(Type type, bool expected) { Assert.Equal(expected, IsBitwiseEquatable(type)); From 165ff669c9e23c0d86bba5b2f79ac0dcf4575af0 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Wed, 15 Jul 2026 16:57:56 -0700 Subject: [PATCH 14/26] Exercise IsBitwiseEquatable via UnsafeAccessor and drop the ILC unit test The behavioral CoreCLR test now calls the internal RuntimeHelpers.IsBitwiseEquatable through an UnsafeAccessor instead of reflection, so it runs under NativeAOT and exercises the intrinsic as the compiler expands it. This lets the ILCompiler.Compiler.Tests copy be removed, which would otherwise silently diverge as the scanner gains coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../BitwiseEquatableTests.cs | 82 ---------- .../EquatableAssets/BitwiseEquatable.cs | 145 ------------------ .../EquatableAssets/EquatableAssets.csproj | 21 --- .../ILCompiler.Compiler.Tests.csproj | 6 - .../BitwiseEquatable/BitwiseEquatable.cs | 88 +++++------ 5 files changed, 39 insertions(+), 303 deletions(-) delete mode 100644 src/coreclr/tools/aot/ILCompiler.Compiler.Tests/BitwiseEquatableTests.cs delete mode 100644 src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/BitwiseEquatable.cs delete mode 100644 src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/EquatableAssets.csproj diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/BitwiseEquatableTests.cs b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/BitwiseEquatableTests.cs deleted file mode 100644 index 52a28109895f46..00000000000000 --- a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/BitwiseEquatableTests.cs +++ /dev/null @@ -1,82 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Collections.Generic; -using System.Text; - -using Internal.IL; -using Internal.IL.Stubs; -using Internal.Text; -using Internal.TypeSystem; - -using Xunit; - -namespace ILCompiler.Compiler.Tests -{ - public class BitwiseEquatableTests - { - private readonly CompilerTypeSystemContext _context; - private readonly ModuleDesc _testModule; - private readonly MethodDesc _objectEquals; - - public BitwiseEquatableTests() - { - var target = new TargetDetails(TargetArchitecture.X64, TargetOS.Windows, TargetAbi.NativeAot); - _context = new CompilerTypeSystemContext(target, SharedGenericsMode.CanonicalReferenceTypes, DelegateFeature.All); - - _context.InputFilePaths = new Dictionary { - { "Test.CoreLib", @"Test.CoreLib.dll" }, - { "EquatableAssets", @"EquatableAssets.dll" }, - }; - _context.ReferenceFilePaths = new Dictionary(); - - _context.SetSystemModule(_context.GetModuleForSimpleName("Test.CoreLib")); - _testModule = _context.GetModuleForSimpleName("EquatableAssets"); - _objectEquals = _context.GetWellKnownType(WellKnownType.Object).GetMethod("Equals"u8, null); - } - - private MetadataType GetTestType(string name) - => (MetadataType)_testModule.GetType( - new Utf8Span(Encoding.UTF8.GetBytes("BitwiseEquatable")), - new Utf8Span(Encoding.UTF8.GetBytes(name))); - - [Theory] - [InlineData("OneField", true)] - [InlineData("TwoFields", true)] - [InlineData("MixedPrimitives", true)] - [InlineData("ForwardsToOp", true)] - [InlineData("NestedField", true)] - [InlineData("NestedFieldLast", true)] - [InlineData("AllNested", true)] - [InlineData("FloatField", false)] - [InlineData("PartialCompare", false)] - [InlineData("OrCompare", false)] - [InlineData("WrapsPartial", false)] - [InlineData("Padded", false)] - [InlineData("WrapsPadded", false)] - [InlineData("NotEquatable", false)] - public void TestIsBitwiseEquatable(string typeName, bool expected) - { - MetadataType type = GetTestType(typeName); - - // This mirrors the decision RuntimeHelpersIntrinsics.EmitIL makes for a value type - // that implements IEquatable of self: the field-wise scan is self-contained. - bool result = ComparerIntrinsics.IsIEquatableEqualsFieldwise(type); - - Assert.Equal(expected, result); - } - - [Theory] - [InlineData("PartialCompare")] - [InlineData("OrCompare")] - public void TestFieldwiseScanRejectsNonEquivalentEquals(string typeName) - { - // These types are bit-comparable at the field level, so the scan of the actual - // Equals implementation is what proves they are not memcmp-equivalent. - MetadataType type = GetTestType(typeName); - - Assert.True(ComparerIntrinsics.CanCompareValueTypeBits(type, _objectEquals)); - Assert.False(ComparerIntrinsics.IsIEquatableEqualsFieldwise(type)); - } - } -} diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/BitwiseEquatable.cs b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/BitwiseEquatable.cs deleted file mode 100644 index 9ea245c440fc9e..00000000000000 --- a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/BitwiseEquatable.cs +++ /dev/null @@ -1,145 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; - -#pragma warning disable 649 // field is never assigned to - -namespace BitwiseEquatable -{ - // Positive cases: value types whose IEquatable.Equals is a plain field-wise comparison. These - // follow the standard pattern of also overriding object.Equals/GetHashCode; that override is - // irrelevant to bitwise equatability because the IEquatable.Equals is what gets used. - - public struct OneField : IEquatable - { - public int A; - public bool Equals(OneField other) => A == other.A; - public override bool Equals(object obj) => obj is OneField other && Equals(other); - public override int GetHashCode() => A; - } - - public struct TwoFields : IEquatable - { - public int A; - public int B; - public bool Equals(TwoFields other) => A == other.A && B == other.B; - public override bool Equals(object obj) => obj is TwoFields other && Equals(other); - public override int GetHashCode() => A; - } - - public struct MixedPrimitives : IEquatable - { - public long A; - public int B; - public short C; - public short D; - public bool Equals(MixedPrimitives other) => A == other.A && B == other.B && C == other.C && D == other.D; - public override bool Equals(object obj) => obj is MixedPrimitives other && Equals(other); - public override int GetHashCode() => B; - } - - public struct ForwardsToOp : IEquatable - { - public int A; - public int B; - public static bool operator ==(ForwardsToOp x, ForwardsToOp y) => x.A == y.A && x.B == y.B; - public static bool operator !=(ForwardsToOp x, ForwardsToOp y) => !(x == y); - public bool Equals(ForwardsToOp other) => this == other; - public override bool Equals(object obj) => obj is ForwardsToOp other && Equals(other); - public override int GetHashCode() => A; - } - - // Recursive cases: a nested value-type field is compared through its own IEquatable.Equals. - - public struct NestedField : IEquatable - { - public OneField A; - public int B; - public bool Equals(NestedField other) => A.Equals(other.A) && B == other.B; - public override bool Equals(object obj) => obj is NestedField other && Equals(other); - public override int GetHashCode() => B; - } - - public struct NestedFieldLast : IEquatable - { - public int A; - public TwoFields B; - public bool Equals(NestedFieldLast other) => A == other.A && B.Equals(other.B); - public override bool Equals(object obj) => obj is NestedFieldLast other && Equals(other); - public override int GetHashCode() => A; - } - - public struct AllNested : IEquatable - { - public OneField A; - public TwoFields B; - public bool Equals(AllNested other) => A.Equals(other.A) && B.Equals(other.B); - public override bool Equals(object obj) => obj is AllNested other && Equals(other); - public override int GetHashCode() => 0; - } - - // Negative cases. - - public struct FloatField : IEquatable - { - public float A; - public bool Equals(FloatField other) => A == other.A; - public override bool Equals(object obj) => obj is FloatField other && Equals(other); - public override int GetHashCode() => 0; - } - - public struct PartialCompare : IEquatable - { - public int A; - public int B; - public bool Equals(PartialCompare other) => A == other.A; - public override bool Equals(object obj) => obj is PartialCompare other && Equals(other); - public override int GetHashCode() => A; - } - - public struct OrCompare : IEquatable - { - public int A; - public int B; - public bool Equals(OrCompare other) => A == other.A || B == other.B; - public override bool Equals(object obj) => obj is OrCompare other && Equals(other); - public override int GetHashCode() => A; - } - - // Nested field whose own Equals is not a full field-wise comparison, so the outer type is not - // memcmp-equivalent even though its layout is bit-comparable. - public struct WrapsPartial : IEquatable - { - public PartialCompare A; - public int B; - public bool Equals(WrapsPartial other) => A.Equals(other.A) && B == other.B; - public override bool Equals(object obj) => obj is WrapsPartial other && Equals(other); - public override int GetHashCode() => B; - } - - // Nested field that introduces internal padding, so a byte-wise compare would inspect bytes the - // field-wise Equals ignores. Both the nested type and the wrapper must be rejected. - public struct Padded : IEquatable - { - public byte A; - public long B; - public bool Equals(Padded other) => A == other.A && B == other.B; - public override bool Equals(object obj) => obj is Padded other && Equals(other); - public override int GetHashCode() => (int)B; - } - - public struct WrapsPadded : IEquatable - { - public Padded A; - public bool Equals(WrapsPadded other) => A.Equals(other.A); - public override bool Equals(object obj) => obj is WrapsPadded other && Equals(other); - public override int GetHashCode() => 0; - } - - public struct NotEquatable - { - public int A; - public int B; - } -} diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/EquatableAssets.csproj b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/EquatableAssets.csproj deleted file mode 100644 index 61e31cdad44699..00000000000000 --- a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/EquatableAssets/EquatableAssets.csproj +++ /dev/null @@ -1,21 +0,0 @@ - - - Library - EquatableAssets - false - false - true - $(NetCoreAppToolCurrent) - - true - false - - true - - - - - - - diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/ILCompiler.Compiler.Tests.csproj b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/ILCompiler.Compiler.Tests.csproj index cf4accbf0e8b1f..69e9d87637f92d 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/ILCompiler.Compiler.Tests.csproj +++ b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/ILCompiler.Compiler.Tests.csproj @@ -37,16 +37,10 @@ Content PreserveNewest - - false - Content - PreserveNewest - - diff --git a/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs index 9a0b88cdafebd1..9129162ced66bc 100644 --- a/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs +++ b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Collections.Generic; -using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -15,71 +13,63 @@ namespace BitwiseEquatableTests { public static class BitwiseEquatable { - private static readonly MethodInfo s_isBitwiseEquatable = - typeof(RuntimeHelpers).GetMethod("IsBitwiseEquatable", BindingFlags.Static | BindingFlags.NonPublic)!; + // Call the internal RuntimeHelpers.IsBitwiseEquatable directly via UnsafeAccessor so the + // intrinsic is exercised as the JIT/AOT compiler expands it, rather than through reflection + // (which wouldn't hit the intrinsic path and doesn't work under NativeAOT). + [UnsafeAccessor(UnsafeAccessorKind.StaticMethod, Name = "IsBitwiseEquatable")] + private static extern bool IsBitwiseEquatable([UnsafeAccessorType("System.Runtime.CompilerServices.RuntimeHelpers")] object o); - private static bool IsBitwiseEquatable(Type t) => - (bool)s_isBitwiseEquatable.MakeGenericMethod(t).Invoke(null, null)!; + private static void Check(bool expected) => Assert.Equal(expected, IsBitwiseEquatable(null!)); - // typeof() of a test-local type can't be used in [InlineData] here: this project is compiled - // into a merged test assembly that references each test via an extern alias, and the generated - // runner can't alias the inlined typeof argument. [MemberData] keeps those typeofs in this - // assembly, so the cases stay data-driven. - public static IEnumerable Cases() + [Fact] + public static void IsBitwiseEquatable_MatchesExpected() { // Primitives: '==' and Equals lower to the same bit-for-bit compare. - yield return new object[] { typeof(int), true }; - yield return new object[] { typeof(Int128), true }; - yield return new object[] { typeof(UInt128), true }; + Check(true); + Check(true); + Check(true); // A SIMD/Unsafe-backed body isn't a recognized field-wise shape, but Guid is a known // bitwise-equatable type special-cased by the runtime (matching NativeAOT), so it stays true. - yield return new object[] { typeof(Guid), true }; + Check(true); // Plain field-wise IEquatable.Equals. - yield return new object[] { typeof(Point), true }; - yield return new object[] { typeof(ThreeFields), true }; - yield return new object[] { typeof(OneField), true }; + Check(true); + Check(true); + Check(true); // 'Equals(other) => this == other' forwarding into a field-wise op_Equality. - yield return new object[] { typeof(ForwardsToOp), true }; + Check(true); // Nested value-type fields compared through their own field-wise IEquatable.Equals. - yield return new object[] { typeof(Nested), true }; - yield return new object[] { typeof(NestedLast), true }; - yield return new object[] { typeof(AllNested), true }; + Check(true); + Check(true); + Check(true); // Nested type's Equals ignores a field, or is internally padded. - yield return new object[] { typeof(WrapsPartial), false }; - yield return new object[] { typeof(WrapsPadded), false }; + Check(false); + Check(false); // No IEquatable at all: legacy path still accepts safe blittable fields. - yield return new object[] { typeof(PlainNoEquatable), true }; + Check(true); // float/double are never bitwise (NaN and signed-zero semantics differ from memcmp). - yield return new object[] { typeof(HasFloat), false }; + Check(false); // Equals ignores a field, does custom logic, or forwards to a non-op_Equality helper. - yield return new object[] { typeof(IgnoresField), false }; - yield return new object[] { typeof(CustomLogic), false }; - yield return new object[] { typeof(CallsHelper), false }; + Check(false); + Check(false); + Check(false); // Explicit padding means memcmp inspects bytes Equals does not. - yield return new object[] { typeof(WithPadding), false }; + Check(false); // Overrides object.Equals only; no IEquatable. - yield return new object[] { typeof(OverriddenOnly), false }; + Check(false); // Primitive fields compared via '.Equals' rather than '=='. - yield return new object[] { typeof(PrimEquals), true }; - yield return new object[] { typeof(MixedEquals), true }; - yield return new object[] { typeof(FloatEquals), false }; + Check(true); + Check(true); + Check(false); // Record structs: Roslyn emits EqualityComparer.Default.Equals(this.F, other.F) per field. - yield return new object[] { typeof(RecTwo), true }; - yield return new object[] { typeof(RecNested), true }; - yield return new object[] { typeof(RecMixed), true }; - yield return new object[] { typeof(RecPadded), false }; - yield return new object[] { typeof(RecFloat), false }; + Check(true); + Check(true); + Check(true); + Check(false); + Check(false); // Enum fields are integer-backed, so they compare bitwise like their underlying primitive. - yield return new object[] { typeof(EnumPair), true }; - yield return new object[] { typeof(EnumAndInt), true }; - yield return new object[] { typeof(RecEnum), true }; - } - - [Theory] - [MemberData(nameof(Cases))] - public static void IsBitwiseEquatable_MatchesExpected(Type type, bool expected) - { - Assert.Equal(expected, IsBitwiseEquatable(type)); + Check(true); + Check(true); + Check(true); } // The following structs have no Equals/GetHashCode override, so ValueType.Equals/GetHashCode go From eee09120135d7270ce6afa3aaac262e8836b00c7 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 16 Jul 2026 05:30:44 -0700 Subject: [PATCH 15/26] Recognize long-form branches and unify the IsBitwiseEquatable decision The field-wise Equals scanner only matched short-form bne.un.s/brfalse.s branches, so a type with enough fields to force Roslyn into long-form bne.un/brfalse (e.g. Guid's 11 fields) was not recognized. Teach both the VM and ILC scanners to accept either form, and route the ILC intrinsic through a single shared ComparerIntrinsics.IsBitwiseEquatable so the runtime and compiler share one determination. Guid is now proven by the scanner, so drop its hardcoded special-case. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../TypeSystem/IL/Stubs/ComparerIntrinsics.cs | 42 ++++- .../IL/Stubs/RuntimeHelpersIntrinsics.cs | 72 +------- src/coreclr/vm/corelib.h | 2 - src/coreclr/vm/jitinterface.cpp | 169 +++++++++--------- 4 files changed, 127 insertions(+), 158 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs index 0b6a9a4a51a33b..63708d7b48f993 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs @@ -318,6 +318,39 @@ public static bool CanCompareValueTypeBitsUntilOffset(MetadataType type, MethodD return result; } + /// + /// Determines whether is bitwise-equatable: an unmanaged, tightly-packed + /// type whose equality is provably a bitwise (memcmp) comparison. This is the single authority + /// behind and + /// mirrors the CoreCLR VM's determination. + /// + public static bool IsBitwiseEquatable(TypeDesc type) + { + // Integer-like primitives, enums, native ints, and pointers are memcmp-comparable. + if (IsBitwiseComparablePrimitive(type)) + return true; + + if (type is not MetadataType mdType || !mdType.IsValueType) + return false; + + bool? equatable = ImplementsIEquatable(mdType.GetTypeDefinition()); + if (!equatable.HasValue) + return false; + + if (equatable.Value) + { + // Value type that implements IEquatable of self: bitwise-equatable when it is tightly + // packed and its Equals is a plain field-wise comparison. + return IsIEquatableEqualsFieldwise(mdType); + } + + // Value type that can use memcmp and that doesn't override object.Equals or implement + // IEquatable.Equals. + MethodDesc objectEquals = mdType.Context.GetWellKnownType(WellKnownType.Object).GetMethod("Equals"u8, null); + return mdType.FindVirtualFunctionTargetMethodOnObjectType(objectEquals).OwningType != mdType + && CanCompareValueTypeBits(mdType, objectEquals); + } + /// /// Determines whether a value type's implementation of self is a /// plain field-wise comparison that is equivalent to a bitwise (memcmp) comparison. This lets a type @@ -511,9 +544,10 @@ private static bool ScanFieldwiseEqualsBodyCore(MethodIL methodIL, MetadataType return false; ILOpcode compareOpcode = reader.ReadILOpcode(); - if (compareOpcode == ILOpcode.bne_un_s) + if (compareOpcode == ILOpcode.bne_un_s || compareOpcode == ILOpcode.bne_un) { - // Non-final field: `bne.un.s FALSE` jumps to the shared `return false` tail. + // Non-final field: `bne.un[.s] FALSE` jumps to the shared `return false` tail. A + // body larger than a signed-byte range uses the long form. int target = reader.ReadBranchDestination(compareOpcode); if (falseTarget == -1) falseTarget = target; @@ -554,9 +588,9 @@ private static bool ScanFieldwiseEqualsBodyCore(MethodIL methodIL, MetadataType return false; } - // The Equals call already yields a bool: `brfalse.s` to the shared tail, or `ret` if final. + // The Equals call already yields a bool: `brfalse[.s]` to the shared tail, or `ret` if final. ILOpcode terminator = reader.ReadILOpcode(); - if (terminator == ILOpcode.brfalse_s) + if (terminator == ILOpcode.brfalse_s || terminator == ILOpcode.brfalse) { int target = reader.ReadBranchDestination(terminator); if (falseTarget == -1) diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/RuntimeHelpersIntrinsics.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/RuntimeHelpersIntrinsics.cs index 1a4695fe353ca8..473a4d9f3f15aa 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/RuntimeHelpersIntrinsics.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/RuntimeHelpersIntrinsics.cs @@ -3,7 +3,6 @@ using System; -using Internal.Text; using Internal.TypeSystem; using Debug = System.Diagnostics.Debug; @@ -32,59 +31,8 @@ public static MethodIL EmitIL(MethodDesc method) bool result; if (method.Name == "IsBitwiseEquatable"u8) { - // Ideally we could detect automatically whether a type is trivially equatable - // (i.e., its operator == could be implemented via memcmp). But for now we'll - // do the simple thing and hardcode the list of types we know fulfill this contract. - // n.b. This doesn't imply that the type's CompareTo method can be memcmp-implemented, - // as a method like CompareTo may need to take a type's signedness into account. - switch (elementType.UnderlyingType.Category) - { - case TypeFlags.Boolean: - case TypeFlags.Byte: - case TypeFlags.SByte: - case TypeFlags.Char: - case TypeFlags.UInt16: - case TypeFlags.Int16: - case TypeFlags.UInt32: - case TypeFlags.Int32: - case TypeFlags.UInt64: - case TypeFlags.Int64: - case TypeFlags.IntPtr: - case TypeFlags.UIntPtr: - result = true; - break; - default: - result = false; - if (elementType is MetadataType mdType) - { - if (IsKnownBitwiseEquatableType(mdType)) - { - result = true; - } - else if (mdType.IsValueType) - { - bool? equatable = ComparerIntrinsics.ImplementsIEquatable(mdType.GetTypeDefinition()); - - if (equatable.HasValue && !equatable.Value) - { - // Value type that can use memcmp and that doesn't override object.Equals or implement IEquatable.Equals. - MethodDesc objectEquals = mdType.Context.GetWellKnownType(WellKnownType.Object).GetMethod("Equals"u8, null); - result = - mdType.FindVirtualFunctionTargetMethodOnObjectType(objectEquals).OwningType != mdType && - ComparerIntrinsics.CanCompareValueTypeBits(mdType, objectEquals); - } - else if (equatable.HasValue && equatable.Value) - { - // Value type that implements IEquatable of self. It is still bitwise-equatable - // if it is tightly packed and its Equals is a plain field-wise comparison. This is - // self-contained (it does its own layout check) so that, matching the CoreCLR VM, a - // nested value-type field is accepted even when its type also overrides object.Equals. - result = ComparerIntrinsics.IsIEquatableEqualsFieldwise(mdType); - } - } - } - break; - } + // The runtime and the ILC share a single determination of what is bitwise-equatable. + result = ComparerIntrinsics.IsBitwiseEquatable(elementType); } else { @@ -95,21 +43,5 @@ public static MethodIL EmitIL(MethodDesc method) return new ILStubMethodIL(method, new byte[] { (byte)opcode, (byte)ILOpcode.ret }, Array.Empty(), Array.Empty()); } - - private static bool IsKnownBitwiseEquatableType(MetadataType type) - { - if (type.Module != type.Context.SystemModule) - { - return false; - } - - Utf8Span ns = type.Namespace; - if (ns == "System"u8) - { - Utf8Span name = type.Name; - return name == "Guid"u8 || name == "Int128"u8 || name == "UInt128"u8; - } - return ns == "System.Text"u8 && type.Name == "Rune"u8; - } } } diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index fcdbe8c797a14b..c5ae735c51fa92 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -273,8 +273,6 @@ DEFINE_FIELD(ENC_HELPER, OBJECT_REFERENCE, _objectReference) DEFINE_CLASS(ENCODING, Text, Encoding) -DEFINE_CLASS(RUNE, Text, Rune) - DEFINE_CLASS(ENUM, System, Enum) DEFINE_CLASS(ENVIRONMENT, System, Environment) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 7d7fbaa6e62ef0..2760b47a5923cc 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -7289,31 +7289,41 @@ static bool getILIntrinsicImplementationForInterlocked(MethodDesc * ftn, namespace { - // The IL opcodes the field-wise Equals scanner matches. It only accepts the exact shapes the C# - // compiler emits for a field-wise comparison, so a small literal table suffices. - enum ILByte : BYTE - { - IL_LDARG_0 = 0x02, - IL_LDARG_1 = 0x03, - IL_LDC_I4_0 = 0x16, - IL_CALL = 0x28, - IL_RET = 0x2A, - IL_BRFALSE_S = 0x2C, - IL_BNE_UN_S = 0x33, - IL_CALLVIRT = 0x6F, - IL_LDOBJ = 0x71, - IL_LDFLD = 0x7B, - IL_LDFLDA = 0x7C, - IL_PREFIX1 = 0xFE, // ceq is encoded as 0xFE 0x01 - IL_CEQ_2ND = 0x01, - }; - mdToken ReadILToken(const BYTE* pIL) { LIMITED_METHOD_CONTRACT; return (mdToken)((uint32_t)pIL[0] | ((uint32_t)pIL[1] << 8) | ((uint32_t)pIL[2] << 16) | ((uint32_t)pIL[3] << 24)); } + // Reads a conditional branch that Roslyn emits in either short form (1-byte signed offset) or long + // form (4-byte signed offset); a body larger than a signed-byte range forces the long form (e.g. the + // field-wise Equals of a type with many fields). On a match, sets 'target' to the absolute + // destination, advances 'ip' past the instruction, and returns true. + bool TryReadBranch(const BYTE* pIL, unsigned codeSize, unsigned& ip, BYTE shortOp, BYTE longOp, int& target) + { + LIMITED_METHOD_CONTRACT; + + if (ip < codeSize && pIL[ip] == shortOp) + { + if (ip + 2 > codeSize) + return false; + target = (int)(ip + 2) + (int)(signed char)pIL[ip + 1]; + ip += 2; + return true; + } + + if (ip < codeSize && pIL[ip] == longOp) + { + if (ip + 5 > codeSize) + return false; + target = (int)(ip + 5) + (int)ReadILToken(pIL + ip + 1); + ip += 5; + return true; + } + + return false; + } + // Resolves an in-module FieldDef token. Returns NULL for anything else (e.g. a MemberRef, // which only arises for generic or cross-module references we deliberately don't handle yet). FieldDesc* TryResolveInModuleFieldDef(Module* pModule, mdToken token) @@ -7561,7 +7571,7 @@ namespace // Optional records lead-in: 'call EqualityComparer::get_Default' before the operands. mdToken getDefaultTok = mdTokenNil; bool records = false; - if (ip + 5 <= codeSize && pIL[ip] == IL_CALL) + if (ip + 5 <= codeSize && pIL[ip] == CEE_CALL) { getDefaultTok = ReadILToken(pIL + ip + 1); records = true; @@ -7570,18 +7580,18 @@ namespace // Left operand: ldarg.0; ldfld/ldflda F. Records and inline '==' load by value; the // '.Equals' call form loads by address. - if (ip + 6 > codeSize || pIL[ip] != IL_LDARG_0) + if (ip + 6 > codeSize || pIL[ip] != CEE_LDARG_0) return false; BYTE leftLoad = pIL[ip + 1]; - if (leftLoad != IL_LDFLD && leftLoad != IL_LDFLDA) + if (leftLoad != CEE_LDFLD && leftLoad != CEE_LDFLDA) return false; - if (records && leftLoad != IL_LDFLD) + if (records && leftLoad != CEE_LDFLD) return false; mdToken leftFieldTok = ReadILToken(pIL + ip + 2); ip += 6; // Right operand: ldarg.1; ldfld F. - if (ip + 6 > codeSize || pIL[ip] != IL_LDARG_1 || pIL[ip + 1] != IL_LDFLD) + if (ip + 6 > codeSize || pIL[ip] != CEE_LDARG_1 || pIL[ip + 1] != CEE_LDFLD) return false; mdToken rightFieldTok = ReadILToken(pIL + ip + 2); ip += 6; @@ -7605,25 +7615,22 @@ namespace } compared[numCompared++] = pField; - if (!records && leftLoad == IL_LDFLD) + if (!records && leftLoad == CEE_LDFLD) { // Inline '==': only integer-like primitives (and enums) are memcmp-equivalent. if (!IsBitwiseComparableField(pField)) return false; - if (ip < codeSize && pIL[ip] == IL_BNE_UN_S) + int target; + if (TryReadBranch(pIL, codeSize, ip, CEE_BNE_UN_S, CEE_BNE_UN, target)) { // Non-final field: branch to the shared 'return false'. - if (ip + 2 > codeSize) - return false; - int target = (int)(ip + 2) + (int)(signed char)pIL[ip + 1]; if (falseTarget == -1) falseTarget = target; else if (falseTarget != target) return false; - ip += 2; } - else if (ip + 3 <= codeSize && pIL[ip] == IL_PREFIX1 && pIL[ip + 1] == IL_CEQ_2ND && pIL[ip + 2] == IL_RET) + else if (ip + 3 <= codeSize && pIL[ip] == CEE_PREFIX1 && pIL[ip + 1] == (CEE_CEQ & 0xFF) && pIL[ip + 2] == CEE_RET) { // Final field: ceq; ret. ip += 3; @@ -7640,7 +7647,7 @@ namespace if (records) { // callvirt EqualityComparer::Equals(!0, !0). - if (ip + 5 > codeSize || pIL[ip] != IL_CALLVIRT) + if (ip + 5 > codeSize || pIL[ip] != CEE_CALLVIRT) return false; MethodDesc* pGetDefault = TryResolveMethodToken(pModule, getDefaultTok); MethodDesc* pEquals = TryResolveMethodToken(pModule, ReadILToken(pIL + ip + 1)); @@ -7650,7 +7657,7 @@ namespace else { // '.Equals' call form: a primitive's own Equals, or a nested type's field-wise Equals. - if (ip + 5 > codeSize || pIL[ip] != IL_CALL) + if (ip + 5 > codeSize || pIL[ip] != CEE_CALL) return false; MethodDesc* pCallee = TryResolveMethodToken(pModule, ReadILToken(pIL + ip + 1)); if (!IsPrimitiveEqualsCall(pCallee, pField) && !IsNestedFieldwiseEquatable(pCallee, pField)) @@ -7658,19 +7665,16 @@ namespace } ip += 5; - // The Equals call already yields a bool: brfalse.s to the shared tail, or ret if final. - if (ip < codeSize && pIL[ip] == IL_BRFALSE_S) + // The Equals call already yields a bool: brfalse to the shared tail, or ret if final. + int target; + if (TryReadBranch(pIL, codeSize, ip, CEE_BRFALSE_S, CEE_BRFALSE, target)) { - if (ip + 2 > codeSize) - return false; - int target = (int)(ip + 2) + (int)(signed char)pIL[ip + 1]; if (falseTarget == -1) falseTarget = target; else if (falseTarget != target) return false; - ip += 2; } - else if (ip < codeSize && pIL[ip] == IL_RET) + else if (ip < codeSize && pIL[ip] == CEE_RET) { ip += 1; sawFinalUnit = true; @@ -7686,8 +7690,8 @@ namespace { if ((int)ip != falseTarget || ip + 2 != codeSize || - pIL[ip] != IL_LDC_I4_0 || - pIL[ip + 1] != IL_RET) + pIL[ip] != CEE_LDC_I4_0 || + pIL[ip + 1] != CEE_RET) { return false; } @@ -7733,11 +7737,11 @@ namespace // 02 71 03 28 2A if (pIL != NULL && codeSize == 13 && - pIL[0] == IL_LDARG_0 && - pIL[1] == IL_LDOBJ && ReadILToken(pIL + 2) == valueTypeMT->GetCl() && - pIL[6] == IL_LDARG_1 && - pIL[7] == IL_CALL && - pIL[12] == IL_RET) + pIL[0] == CEE_LDARG_0 && + pIL[1] == CEE_LDOBJ && ReadILToken(pIL + 2) == valueTypeMT->GetCl() && + pIL[6] == CEE_LDARG_1 && + pIL[7] == CEE_CALL && + pIL[12] == CEE_RET) { MethodDesc* pOpEquality = TryResolveInModuleMethodDef(pEqualsMD->GetModule(), ReadILToken(pIL + 8)); if (pOpEquality != NULL && @@ -7764,26 +7768,39 @@ bool IsBitwiseEquatable(TypeHandle typeHandle, MethodTable * methodTable) return false; } - Instantiation inst(&typeHandle, 1); - TypeHandle iequatableOfSelf = TypeHandle(CoreLibBinder::GetClass(CLASS__IEQUATABLEGENERIC)).Instantiate(inst); - - if (!typeHandle.CanCastTo(iequatableOfSelf)) + // Scanning resolves field/method tokens and can force type loads, any of which may throw on bad or + // incomplete metadata. Constant folding must be conservative, so trap and fold to 'false' on failure. + bool result = false; + EX_TRY { - // No IEquatable of its own: bitwise equality is safe if the fields are bit-comparable and - // there is no custom object.Equals override. - return CanCompareBitsOrUseFastGetHashCode(methodTable); - } + Instantiation inst(&typeHandle, 1); + TypeHandle iequatableOfSelf = TypeHandle(CoreLibBinder::GetClass(CLASS__IEQUATABLEGENERIC)).Instantiate(inst); - // Has IEquatable.Equals: bitwise only if that Equals is a plain field-wise memcmp equivalent. - // UnwrapStub turns the value-type interface dispatch into the underlying instance method. - MethodDesc* pEqualsMD = UnwrapStub(methodTable->GetMethodDescForInterfaceMethod( - iequatableOfSelf, CoreLibBinder::GetMethod(METHOD__IEQUATABLEGENERIC__EQUALS), FALSE /* throwOnConflict */)); - if (pEqualsMD == NULL) + if (!typeHandle.CanCastTo(iequatableOfSelf)) + { + // No IEquatable of its own: bitwise equality is safe if the fields are bit-comparable and + // there is no custom object.Equals override. + result = CanCompareBitsOrUseFastGetHashCode(methodTable); + } + else + { + // Has IEquatable.Equals: bitwise only if that Equals is a plain field-wise memcmp equivalent. + // UnwrapStub turns the value-type interface dispatch into the underlying instance method. + MethodDesc* pEqualsMD = UnwrapStub(methodTable->GetMethodDescForInterfaceMethod( + iequatableOfSelf, CoreLibBinder::GetMethod(METHOD__IEQUATABLEGENERIC__EQUALS), FALSE /* throwOnConflict */)); + if (pEqualsMD != NULL) + { + result = IsFieldwiseEqualsBitwiseEquivalent(methodTable, pEqualsMD); + } + } + } + EX_CATCH { - return false; + result = false; } + EX_END_CATCH - return IsFieldwiseEqualsBitwiseEquivalent(methodTable, pEqualsMD); + return result; } static bool getILIntrinsicImplementationForRuntimeHelpers( @@ -7812,28 +7829,16 @@ static bool getILIntrinsicImplementationForRuntimeHelpers( // Ideally we could detect automatically whether a type is trivially equatable // (i.e., its operator == could be implemented via memcmp). The best we can do - // for now is hardcode a list of known supported types and then also include anything - // that doesn't provide its own object.Equals override / IEquatable implementation. + // for now is check a few known-good shapes and then also include anything the + // field-wise scanner proves memcmp-equivalent. // n.b. This doesn't imply that the type's CompareTo method can be memcmp-implemented, // as a method like CompareTo may need to take a type's signedness into account. - - if (methodTable == CoreLibBinder::GetClass(CLASS__BOOLEAN) - || methodTable == CoreLibBinder::GetClass(CLASS__BYTE) - || methodTable == CoreLibBinder::GetClass(CLASS__SBYTE) - || methodTable == CoreLibBinder::GetClass(CLASS__CHAR) - || methodTable == CoreLibBinder::GetClass(CLASS__INT16) - || methodTable == CoreLibBinder::GetClass(CLASS__UINT16) - || methodTable == CoreLibBinder::GetClass(CLASS__INT32) - || methodTable == CoreLibBinder::GetClass(CLASS__UINT32) - || methodTable == CoreLibBinder::GetClass(CLASS__INT64) - || methodTable == CoreLibBinder::GetClass(CLASS__UINT64) - || methodTable == CoreLibBinder::GetClass(CLASS__INT128) - || methodTable == CoreLibBinder::GetClass(CLASS__UINT128) - || methodTable == CoreLibBinder::GetClass(CLASS__INTPTR) - || methodTable == CoreLibBinder::GetClass(CLASS__UINTPTR) - || methodTable == CoreLibBinder::GetClass(CLASS__GUID) - || methodTable == CoreLibBinder::GetClass(CLASS__RUNE) - || methodTable->IsEnum() + // + // Integer-like primitives, native ints, and enums are memcmp-comparable but their Equals + // isn't field-wise (one side is the raw primitive arg), so they're matched by element type. + // Everything else -- including Guid, Rune, Int128, and UInt128 -- is proven by IsBitwiseEquatable, + // which scans the type's IEquatable.Equals for a field-wise shape. + if (IsBitwiseComparablePrimitive(methodTable->GetInternalCorElementType()) || IsBitwiseEquatable(typeHandle, methodTable)) { methInfo->ILCode = const_cast(returnTrue); From 0826af2c41463ea77506aafecaa67467f1dbb725 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 16 Jul 2026 05:30:56 -0700 Subject: [PATCH 16/26] Make Guid.Equals a field-wise comparison Replace the vectorized EqualsCore with a plain field-wise op_Equality that Equals and op_Inequality forward to. This lets RuntimeHelpers.IsBitwiseEquatable prove Guid via the scanner; the JIT vectorizes the field-wise body and the flag lights up the memcmp fast paths in the BCL. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../System.Private.CoreLib/src/System/Guid.cs | 31 +++++-------------- 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Guid.cs b/src/libraries/System.Private.CoreLib/src/System/Guid.cs index e807450ceeb633..df2a71b8c9cbdb 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Guid.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Guid.cs @@ -1081,28 +1081,9 @@ public override int GetHashCode() // Returns true if and only if the guid represented // by o is the same as this instance. - public override bool Equals([NotNullWhen(true)] object? o) => o is Guid g && EqualsCore(this, g); + public override bool Equals([NotNullWhen(true)] object? o) => o is Guid g && this == g; - public bool Equals(Guid g) => EqualsCore(this, g); - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static bool EqualsCore(in Guid left, in Guid right) - { - if (Vector128.IsHardwareAccelerated) - { - return Unsafe.BitCast>(left) == Unsafe.BitCast>(right); - } - - ref int rA = ref Unsafe.AsRef(in left._a); - ref int rB = ref Unsafe.AsRef(in right._a); - - // Compare each element - - return rA == rB - && Unsafe.Add(ref rA, 1) == Unsafe.Add(ref rB, 1) - && Unsafe.Add(ref rA, 2) == Unsafe.Add(ref rB, 2) - && Unsafe.Add(ref rA, 3) == Unsafe.Add(ref rB, 3); - } + public bool Equals(Guid g) => this == g; private static int GetResult(uint me, uint them) => me < them ? -1 : 1; @@ -1179,9 +1160,13 @@ public int CompareTo(Guid value) return 0; } - public static bool operator ==(Guid a, Guid b) => EqualsCore(a, b); + // Field-wise so the runtime can prove Guid is bitwise-equatable (see RuntimeHelpers.IsBitwiseEquatable); + // the JIT vectorizes this and the flag lights up the memcmp fast paths in the BCL. + public static bool operator ==(Guid a, Guid b) => + a._a == b._a && a._b == b._b && a._c == b._c && a._d == b._d && a._e == b._e && a._f == b._f && + a._g == b._g && a._h == b._h && a._i == b._i && a._j == b._j && a._k == b._k; - public static bool operator !=(Guid a, Guid b) => !EqualsCore(a, b); + public static bool operator !=(Guid a, Guid b) => !(a == b); [MethodImpl(MethodImplOptions.AggressiveInlining)] private static unsafe int HexsToChars(TChar* guidChars, int a, int b) where TChar : unmanaged, IUtfChar From 8dc6c2412235a68ca52ad91faa6097634ed14813 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 16 Jul 2026 05:54:44 -0700 Subject: [PATCH 17/26] Funnel bitwise-equatable struct equality through Equals For our own bitwise-equatable value types, keep the field-wise comparison in Equals and have == and != defer to it, rather than the historical shape where Equals forwards to a field-wise ==. This makes Equals the single canonical comparison that every entry point funnels through, which is the stable hook a future JIT optimization can key off to vectorize the compare on types flagged IsBitwiseEquatable. Applies to Guid, Int128, UInt128, and Rune. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../System.Private.CoreLib/src/System/Guid.cs | 16 ++++++++-------- .../System.Private.CoreLib/src/System/Int128.cs | 6 +++--- .../src/System/Text/Rune.cs | 6 +++--- .../System.Private.CoreLib/src/System/UInt128.cs | 6 +++--- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Guid.cs b/src/libraries/System.Private.CoreLib/src/System/Guid.cs index df2a71b8c9cbdb..9a590e9fe7ae2a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Guid.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Guid.cs @@ -1081,9 +1081,13 @@ public override int GetHashCode() // Returns true if and only if the guid represented // by o is the same as this instance. - public override bool Equals([NotNullWhen(true)] object? o) => o is Guid g && this == g; + public override bool Equals([NotNullWhen(true)] object? o) => o is Guid g && Equals(g); - public bool Equals(Guid g) => this == g; + // Field-wise so the runtime can prove Guid is bitwise-equatable (see RuntimeHelpers.IsBitwiseEquatable). + // Equality funnels through Equals; == and != defer to it so this stays the single canonical comparison. + public bool Equals(Guid g) => + _a == g._a && _b == g._b && _c == g._c && _d == g._d && _e == g._e && _f == g._f && + _g == g._g && _h == g._h && _i == g._i && _j == g._j && _k == g._k; private static int GetResult(uint me, uint them) => me < them ? -1 : 1; @@ -1160,13 +1164,9 @@ public int CompareTo(Guid value) return 0; } - // Field-wise so the runtime can prove Guid is bitwise-equatable (see RuntimeHelpers.IsBitwiseEquatable); - // the JIT vectorizes this and the flag lights up the memcmp fast paths in the BCL. - public static bool operator ==(Guid a, Guid b) => - a._a == b._a && a._b == b._b && a._c == b._c && a._d == b._d && a._e == b._e && a._f == b._f && - a._g == b._g && a._h == b._h && a._i == b._i && a._j == b._j && a._k == b._k; + public static bool operator ==(Guid a, Guid b) => a.Equals(b); - public static bool operator !=(Guid a, Guid b) => !(a == b); + public static bool operator !=(Guid a, Guid b) => !a.Equals(b); [MethodImpl(MethodImplOptions.AggressiveInlining)] private static unsafe int HexsToChars(TChar* guidChars, int a, int b) where TChar : unmanaged, IUtfChar diff --git a/src/libraries/System.Private.CoreLib/src/System/Int128.cs b/src/libraries/System.Private.CoreLib/src/System/Int128.cs index ab04caa25ecb47..85a9a1b6f9e394 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int128.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int128.cs @@ -88,7 +88,7 @@ public override bool Equals([NotNullWhen(true)] object? obj) /// public bool Equals(Int128 other) { - return this == other; + return (_lower == other._lower) && (_upper == other._upper); } /// @@ -1088,10 +1088,10 @@ public static Int128 Log2(Int128 value) // /// - public static bool operator ==(Int128 left, Int128 right) => (left._lower == right._lower) && (left._upper == right._upper); + public static bool operator ==(Int128 left, Int128 right) => left.Equals(right); /// - public static bool operator !=(Int128 left, Int128 right) => (left._lower != right._lower) || (left._upper != right._upper); + public static bool operator !=(Int128 left, Int128 right) => !left.Equals(right); // // IIncrementOperators diff --git a/src/libraries/System.Private.CoreLib/src/System/Text/Rune.cs b/src/libraries/System.Private.CoreLib/src/System/Text/Rune.cs index 61673fc1da8d51..0da17fbc04a60a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Text/Rune.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Text/Rune.cs @@ -131,9 +131,9 @@ private Rune(uint scalarValue, bool _) _value = scalarValue; } - public static bool operator ==(Rune left, Rune right) => left._value == right._value; + public static bool operator ==(Rune left, Rune right) => left.Equals(right); - public static bool operator !=(Rune left, Rune right) => left._value != right._value; + public static bool operator !=(Rune left, Rune right) => !left.Equals(right); public static bool operator <(Rune left, Rune right) => left._value < right._value; @@ -787,7 +787,7 @@ public int EncodeToUtf8(Span destination) public override bool Equals([NotNullWhen(true)] object? obj) => (obj is Rune other) && Equals(other); - public bool Equals(Rune other) => this == other; + public bool Equals(Rune other) => _value == other._value; /// /// Returns a value that indicates whether the current instance and a specified rune are equal using the specified comparison option. diff --git a/src/libraries/System.Private.CoreLib/src/System/UInt128.cs b/src/libraries/System.Private.CoreLib/src/System/UInt128.cs index b66f13fc38bdea..81f37e0189cac5 100644 --- a/src/libraries/System.Private.CoreLib/src/System/UInt128.cs +++ b/src/libraries/System.Private.CoreLib/src/System/UInt128.cs @@ -90,7 +90,7 @@ public override bool Equals([NotNullWhen(true)] object? obj) /// public bool Equals(UInt128 other) { - return this == other; + return (_lower == other._lower) && (_upper == other._upper); } /// @@ -1326,10 +1326,10 @@ public static UInt128 Log2(UInt128 value) // /// - public static bool operator ==(UInt128 left, UInt128 right) => (left._lower == right._lower) && (left._upper == right._upper); + public static bool operator ==(UInt128 left, UInt128 right) => left.Equals(right); /// - public static bool operator !=(UInt128 left, UInt128 right) => (left._lower != right._lower) || (left._upper != right._upper); + public static bool operator !=(UInt128 left, UInt128 right) => !left.Equals(right); // // IIncrementOperators From 3e13ed0142b31796dc3950ae4e2eb3800f874071 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 16 Jul 2026 06:07:21 -0700 Subject: [PATCH 18/26] Cover long-form branches in the BitwiseEquatable test Add an 11-field GuidShape case (which forces Roslyn into long-form branches) and correct the stale Guid comment now that Guid is proven by the scanner rather than special-cased. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../BitwiseEquatable/BitwiseEquatable.cs | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs index 9129162ced66bc..04e644fe52a4e1 100644 --- a/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs +++ b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs @@ -24,13 +24,15 @@ public static class BitwiseEquatable [Fact] public static void IsBitwiseEquatable_MatchesExpected() { - // Primitives: '==' and Equals lower to the same bit-for-bit compare. + // Primitive: '==' and Equals lower to the same bit-for-bit compare. Check(true); + // Int128/UInt128: field-wise IEquatable.Equals over two ulong halves. Check(true); Check(true); - // A SIMD/Unsafe-backed body isn't a recognized field-wise shape, but Guid is a known - // bitwise-equatable type special-cased by the runtime (matching NativeAOT), so it stays true. + // Guid is proven field-wise by the scanner (its Equals compares all 11 fields). GuidShape + // below covers the same many-field shape (which forces long-form branches) independently. Check(true); + Check(true); // Plain field-wise IEquatable.Equals. Check(true); Check(true); @@ -143,6 +145,19 @@ public enum ColorInt { A, B, C } public override int GetHashCode() => 0; } + // 11 fields (int, 2x short, 8x byte = 16 bytes, tightly packed) shaped like Guid. Enough fields + // that Roslyn emits long-form branches in the field-wise Equals; the scanner must accept those. + public readonly struct GuidShape : IEquatable + { + public readonly int A; public readonly short B; public readonly short C; + public readonly byte D, E, F, G, H, I, J, K; + public bool Equals(GuidShape o) => + A == o.A && B == o.B && C == o.C && D == o.D && E == o.E && F == o.F && + G == o.G && H == o.H && I == o.I && J == o.J && K == o.K; + public override bool Equals(object o) => o is GuidShape g && Equals(g); + public override int GetHashCode() => 0; + } + public readonly struct ForwardsToOp : IEquatable { public readonly int Lo; public readonly int Hi; From d4be11d18121004b5283a0a642165ca5d2097c44 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 16 Jul 2026 06:35:05 -0700 Subject: [PATCH 19/26] Rethrow terminal exceptions from the IsBitwiseEquatable scan The EX_CATCH conservatively folds to false on metadata/type-load failures, but must not swallow terminal exceptions such as stack overflow. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/jitinterface.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 2760b47a5923cc..1f7475c1e3ac60 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -7797,6 +7797,7 @@ bool IsBitwiseEquatable(TypeHandle typeHandle, MethodTable * methodTable) EX_CATCH { result = false; + RethrowTerminalExceptions(); } EX_END_CATCH From 9b362925c61aa4af27ccaf96c5160a7d90340018 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 16 Jul 2026 07:35:41 -0700 Subject: [PATCH 20/26] Document the tearing consideration on IsBitwiseEquatable Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs index 584f6cd3e15965..ff8700017c107b 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs @@ -352,6 +352,13 @@ public static object GetUninitializedObject( /// true if given type is bitwise equatable (memcmp can be used for equality checking) /// /// Only use the result of this for Equals() comparison, not for CompareTo() comparison. + /// + /// A bitwise comparison may read the value using accesses wider than an individual field. Under an + /// unsynchronized concurrent mutation -- already a data race with undefined behavior -- this can observe + /// a torn value within a single field that a strictly field-wise comparison would not. A torn read cannot + /// fabricate an invalid managed reference; only the already-undefined total comparison result is affected. This is + /// consistent with existing bitwise-based APIs such as . + /// /// [Intrinsic] internal static bool IsBitwiseEquatable() From 2fb72c990565555151213c2f39e79d43f6193a96 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 16 Jul 2026 07:43:25 -0700 Subject: [PATCH 21/26] Apply suggestions from code review Co-authored-by: Jan Kotas --- .../Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs | 2 +- .../tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs index ff8700017c107b..f8460dd4c2e8f7 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs @@ -357,7 +357,7 @@ public static object GetUninitializedObject( /// unsynchronized concurrent mutation -- already a data race with undefined behavior -- this can observe /// a torn value within a single field that a strictly field-wise comparison would not. A torn read cannot /// fabricate an invalid managed reference; only the already-undefined total comparison result is affected. This is - /// consistent with existing bitwise-based APIs such as . + /// acceptable for bitwise-based APIs such as . /// /// [Intrinsic] diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs index 63708d7b48f993..0f016069f9233b 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs @@ -321,8 +321,7 @@ public static bool CanCompareValueTypeBitsUntilOffset(MetadataType type, MethodD /// /// Determines whether is bitwise-equatable: an unmanaged, tightly-packed /// type whose equality is provably a bitwise (memcmp) comparison. This is the single authority - /// behind and - /// mirrors the CoreCLR VM's determination. + /// behind . /// public static bool IsBitwiseEquatable(TypeDesc type) { @@ -384,7 +383,7 @@ private static bool IsTightlyPacked(MetadataType type) // Mirrors the CoreCLR VM's MethodTable::IsNotTightlyPacked (negated): a byte-wise compare // equals comparing every field only if there is no padding anywhere. That needs the declared // fields to exactly cover the instance size (no gaps, no overlap) and every nested value-type - // field to itself be tightly packed. The nested check makes this transitive, like the VM flag. + // field to itself be tightly packed. The nested check makes this transitive. if (type.ContainsGCPointers) return false; @@ -424,7 +423,7 @@ private static bool IsTightlyPacked(MetadataType type) private static MethodDesc GetIEquatableEqualsImplementation(MetadataType type) { - // Keep token resolution simple by only handling non-generic value types, matching the VM. + // Keep token resolution simple by only handling non-generic value types. if (type.HasInstantiation) return null; From 627879fb953f0af873d8602f3c7a757097206203 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 16 Jul 2026 07:47:32 -0700 Subject: [PATCH 22/26] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/coreclr/vm/jitinterface.cpp | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 1f7475c1e3ac60..d9a0431afbd8ae 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -7344,8 +7344,8 @@ namespace } // Resolves a method token, including a cross-module MemberRef (a primitive's Equals lives in - // CoreLib). Callers only match non-generic methods, so a MethodSpec resolves to its generic - // definition -- which never matches -- instead of throwing. + // CoreLib). Returns NULL if the token kind is unexpected or resolution fails. + // Callers only match non-generic methods, so a MethodSpec resolves to its generic definition. MethodDesc* TryResolveMethodToken(Module* pModule, mdToken token) { STANDARD_VM_CONTRACT; @@ -7355,8 +7355,21 @@ namespace return NULL; SigTypeContext typeContext; - return MemberLoader::GetMethodDescFromMemberDefOrRefOrSpec( - pModule, token, &typeContext, FALSE /* strictMetadataChecks */, FALSE /* allowInstParam */); + MethodDesc* pMD = NULL; + + EX_TRY + { + pMD = MemberLoader::GetMethodDescFromMemberDefOrRefOrSpec( + pModule, token, &typeContext, FALSE /* strictMetadataChecks */, FALSE /* allowInstParam */); + } + EX_CATCH + { + pMD = NULL; + RethrowTerminalExceptions(); + } + EX_END_CATCH + + return pMD; } // Unwraps an unboxing stub (interface dispatch on a value type) to the instance method that has IL. From 82db3033d2806edce822f5ed240ee2d374a1b20d Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 16 Jul 2026 08:27:54 -0700 Subject: [PATCH 23/26] Bail out early on generic value types in the field-wise scan Reject HasInstantiation up-front, before any layout inspection, in both the VM and ILC scanners. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../TypeSystem/IL/Stubs/ComparerIntrinsics.cs | 5 +++++ src/coreclr/vm/jitinterface.cpp | 13 +++++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs index 0f016069f9233b..949e57ef99726c 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs @@ -358,6 +358,11 @@ public static bool IsBitwiseEquatable(TypeDesc type) /// public static bool IsIEquatableEqualsFieldwise(MetadataType type) { + // Generic value types are out of scope: keep the field-wise scan to non-generic types. + // Bail before any layout inspection. + if (type.HasInstantiation) + return false; + // The type's layout must be tightly packed (no padding gaps and no overlapping fields) so // that a byte-wise compare never inspects bytes the field-wise Equals ignores. This is // checked at every level of the recursion, matching the CoreCLR VM. diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index d9a0431afbd8ae..e96ce7fba76c21 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -7724,12 +7724,17 @@ namespace { STANDARD_VM_CONTRACT; + // Generic value types are out of scope: their Equals bodies can carry TypeSpec/MethodSpec + // tokens needing a type context to resolve. Bail before any layout inspection. + if (valueTypeMT->HasInstantiation()) + { + return false; + } + // First-pass restrictions that keep the scan simple and unquestionably safe: the type must be - // unmanaged (so a byte-wise compare is meaningful), non-generic (so IL tokens stay in-module), - // tightly packed (no padding anywhere -- the flag is transitive -- else memcmp inspects bytes - // Equals ignores), and not an inline array. + // unmanaged (so a byte-wise compare is meaningful), tightly packed (no padding anywhere -- the + // flag is transitive -- else memcmp inspects bytes Equals ignores), and not an inline array. if (valueTypeMT->ContainsGCPointers() || - valueTypeMT->HasInstantiation() || valueTypeMT->IsNotTightlyPacked() || valueTypeMT->GetClass()->IsInlineArray()) { From 89c6fc406f62c68cb29ebc72ffb6707eb353c198 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 16 Jul 2026 08:31:21 -0700 Subject: [PATCH 24/26] Guard the field-wise scan against writing past the field array Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/jitinterface.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index e96ce7fba76c21..9fd2c6e4b40beb 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -7626,6 +7626,9 @@ namespace if (compared[i] == pField) return false; } + + if (numCompared >= fieldCount) + return false; compared[numCompared++] = pField; if (!records && leftLoad == CEE_LDFLD) From ac384b391f49d75ba634253f80f5e7bd3e2250c9 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Mon, 20 Jul 2026 08:30:23 -0700 Subject: [PATCH 25/26] Address review feedback on the field-wise equality scan - Track scanned Equals bodies as ReJIT inlining dependencies and blanket-disable the fold for EnC modules - Flip the MethodTable accessor to positive IsTightlyPacked (EEClass flag stays negative) - Mirror the VM by rejecting GC pointers in the caller, not the ILC tightly-packed predicate - Identify EqualityComparer`1 by name, namespace, and module instead of a by-name lookup - Key the EqualityComparer form off getDefault presence and drop the records framing Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../TypeSystem/IL/Stubs/ComparerIntrinsics.cs | 56 +++--- src/coreclr/vm/comutilnative.cpp | 2 +- src/coreclr/vm/jitinterface.cpp | 179 +++++++++++------- src/coreclr/vm/methodtable.h | 2 +- src/coreclr/vm/methodtable.inl | 4 +- src/coreclr/vm/methodtablebuilder.cpp | 2 +- 6 files changed, 143 insertions(+), 102 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs index 949e57ef99726c..d5e28582acf280 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs @@ -363,9 +363,11 @@ public static bool IsIEquatableEqualsFieldwise(MetadataType type) if (type.HasInstantiation) return false; - // The type's layout must be tightly packed (no padding gaps and no overlapping fields) so - // that a byte-wise compare never inspects bytes the field-wise Equals ignores. This is - // checked at every level of the recursion, matching the CoreCLR VM. + // Unmanaged (so a byte-wise compare is meaningful) and tightly packed (no padding anywhere the + // compare would inspect) -- matching the CoreCLR VM, which checks these separately. + if (type.ContainsGCPointers) + return false; + if (!IsTightlyPacked(type)) return false; @@ -385,13 +387,10 @@ public static bool IsIEquatableEqualsFieldwise(MetadataType type) private static bool IsTightlyPacked(MetadataType type) { - // Mirrors the CoreCLR VM's MethodTable::IsNotTightlyPacked (negated): a byte-wise compare - // equals comparing every field only if there is no padding anywhere. That needs the declared - // fields to exactly cover the instance size (no gaps, no overlap) and every nested value-type - // field to itself be tightly packed. The nested check makes this transitive. - if (type.ContainsGCPointers) - return false; - + // Mirrors the CoreCLR VM's MethodTable::IsTightlyPacked: a byte-wise compare equals comparing + // every field only if there is no padding anywhere. That needs the declared fields to exactly + // cover the instance size (no gaps, no overlap) and every nested value-type field to itself be + // tightly packed. The nested check makes this transitive. if (type.IsInlineArray) return false; @@ -412,9 +411,10 @@ private static bool IsTightlyPacked(MetadataType type) return false; TypeDesc fieldType = field.FieldType; - if (!fieldType.IsPrimitive && !fieldType.IsEnum && !fieldType.IsPointer && !fieldType.IsFunctionPointer) + if (fieldType.IsValueType && !fieldType.IsPrimitive && !fieldType.IsEnum) { - // Not a leaf field, so (having excluded GC pointers above) it is a nested value type. + // Nested value type: recurse for transitive packing. Primitives, pointers, and + // references are leaves whose element size already accounts for their footprint. if (fieldType is not MetadataType nestedType || !IsTightlyPacked(nestedType)) return false; } @@ -483,8 +483,8 @@ private static bool ScanFieldwiseEqualsBody(MethodIL methodIL, MetadataType type private static bool ScanFieldwiseEqualsBodyCore(MethodIL methodIL, MetadataType type) { // Verifies the body is a plain field-wise equality: every instance field is compared exactly once - // (via `==`, its own `Equals`, or `EqualityComparer.Default.Equals` for records) and the results - // are ANDed together, which is equivalent to a bitwise (memcmp) comparison. + // (via `==`, its own `Equals`, or `EqualityComparer.Default.Equals`) and the results are ANDed + // together, which is equivalent to a bitwise (memcmp) comparison. int instanceFieldCount = 0; foreach (FieldDesc field in type.GetFields()) { @@ -506,25 +506,24 @@ private static bool ScanFieldwiseEqualsBodyCore(MethodIL methodIL, MetadataType if (!reader.HasNext) return false; - // Optional records lead-in: `call EqualityComparer::get_Default` before the operands. + // Optional EqualityComparer.Default lead-in: `call EqualityComparer::get_Default` + // before the operands. MethodDesc getDefault = null; - bool records = false; if (reader.PeekILOpcode() == ILOpcode.call) { reader.ReadILOpcode(); getDefault = methodIL.GetObject(reader.ReadILToken()) as MethodDesc; - records = true; } - // Left operand: `ldarg.0; ldfld/ldflda F`. Records and inline `==` load by value; the - // `.Equals` call form loads the left side by address. + // Left operand: `ldarg.0; ldfld/ldflda F`. The EqualityComparer and inline `==` forms load by + // value; the `.Equals` call form loads the left side by address. if (!reader.HasNext || reader.ReadILOpcode() != ILOpcode.ldarg_0) return false; ILOpcode leftLoad = reader.ReadILOpcode(); if (leftLoad != ILOpcode.ldfld && leftLoad != ILOpcode.ldflda) return false; - if (records && leftLoad != ILOpcode.ldfld) + if (getDefault != null && leftLoad != ILOpcode.ldfld) return false; FieldDesc leftField = methodIL.GetObject(reader.ReadILToken()) as FieldDesc; @@ -541,7 +540,7 @@ private static bool ScanFieldwiseEqualsBodyCore(MethodIL methodIL, MetadataType if (!comparedFields.Add(leftField)) return false; - if (!records && leftLoad == ILOpcode.ldfld) + if (getDefault == null && leftLoad == ILOpcode.ldfld) { // Inline `==`: only integer-like primitives are memcmp-equivalent. if (!IsBitwiseComparablePrimitive(leftField.FieldType)) @@ -573,7 +572,7 @@ private static bool ScanFieldwiseEqualsBodyCore(MethodIL methodIL, MetadataType continue; } - if (records) + if (getDefault != null) { // `callvirt EqualityComparer::Equals(!0, !0)`. if (reader.ReadILOpcode() != ILOpcode.callvirt) @@ -652,9 +651,9 @@ private static bool IsPrimitiveEqualsCall(MethodDesc callee, TypeDesc fieldType) private static bool IsEqualityComparerDefaultEquals(MethodDesc getDefault, MethodDesc equals, TypeDesc fieldType) { - // Records compare each field with EqualityComparer.Default.Equals(this.F, other.F). That is - // a memcmp only when F is itself bitwise-equatable: a bit-comparable primitive, or a nested - // value type whose own IEquatable.Equals is field-wise. + // A field compared with EqualityComparer.Default.Equals(this.F, other.F). That is a memcmp + // only when F is itself bitwise-equatable: a bit-comparable primitive, or a nested value type + // whose own IEquatable.Equals is field-wise. if (!IsEqualityComparerMethod(getDefault, fieldType, "get_Default"u8, isStatic: true) || !IsEqualityComparerMethod(equals, fieldType, "Equals"u8, isStatic: false)) { @@ -674,10 +673,11 @@ private static bool IsEqualityComparerMethod(MethodDesc method, TypeDesc fieldTy if (method == null || method.Signature.IsStatic != isStatic || method.Name != name) return false; - MetadataType equalityComparer = fieldType.Context.SystemModule.GetType("System.Collections.Generic"u8, "EqualityComparer`1"u8, throwIfNotFound: false); TypeDesc owningType = method.OwningType; - return equalityComparer != null - && owningType.GetTypeDefinition() == equalityComparer + return owningType.GetTypeDefinition() is MetadataType definition + && definition.Module == fieldType.Context.SystemModule + && definition.Name == "EqualityComparer`1"u8 + && definition.Namespace == "System.Collections.Generic"u8 && owningType.Instantiation.Length == 1 && owningType.Instantiation[0] == fieldType; } diff --git a/src/coreclr/vm/comutilnative.cpp b/src/coreclr/vm/comutilnative.cpp index 52d26ddbe8dcf5..c017d433b02f3d 100644 --- a/src/coreclr/vm/comutilnative.cpp +++ b/src/coreclr/vm/comutilnative.cpp @@ -1785,7 +1785,7 @@ BOOL CanCompareBitsOrUseFastGetHashCode(MethodTable* mt) } if (mt->ContainsGCPointers() - || mt->IsNotTightlyPacked()) + || !mt->IsTightlyPacked()) { mt->SetHasCheckedCanCompareBitsOrUseFastGetHashCode(); return FALSE; diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 9fd2c6e4b40beb..80b74c79f4a054 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -7402,7 +7402,8 @@ namespace } // Forward declaration: the field-wise scanner recurses into nested value-type fields. - bool IsFieldwiseEqualsBitwiseEquivalent(MethodTable* valueTypeMT, MethodDesc* pEqualsMD); + // 'scannedMethods' collects every Equals body relied on so the caller can register a ReJIT dependency. + bool IsFieldwiseEqualsBitwiseEquivalent(MethodTable* valueTypeMT, MethodDesc* pEqualsMD, StackSArray& scannedMethods); // Integer-like primitives whose '==' and Equals are both a bit-for-bit compare. Float/double are // excluded: neither form is a memcmp (for '==' NaN != NaN and +0.0 == -0.0; Equals treats all NaNs @@ -7469,7 +7470,7 @@ namespace // Accepts a nested value-type field compared through its own IEquatable.Equals, but only when // that Equals is itself a provable field-wise compare (its layout is covered by the recursion). - bool IsNestedFieldwiseEquatable(MethodDesc* pCallee, FieldDesc* pField) + bool IsNestedFieldwiseEquatable(MethodDesc* pCallee, FieldDesc* pField, StackSArray& scannedMethods) { STANDARD_VM_CONTRACT; @@ -7484,11 +7485,11 @@ namespace if (pNestedEquals == NULL || pNestedEquals != UnwrapStub(pCallee)) return false; - return IsFieldwiseEqualsBitwiseEquivalent(pNestedMT, pNestedEquals); + return IsFieldwiseEqualsBitwiseEquivalent(pNestedMT, pNestedEquals, scannedMethods); } - // True if 'pMD' is 'EqualityComparer::name' with the expected static-ness. Records call - // through this base type: 'get_Default' (static) and the abstract 'Equals' (instance). + // True if 'pMD' is 'EqualityComparer::name' with the expected static-ness. The lead-in reaches + // Default through this base type: 'get_Default' (static) and the abstract 'Equals' (instance). bool IsEqualityComparerMethod(MethodDesc* pMD, TypeHandle fieldTh, const char* name, bool isStatic) { STANDARD_VM_CONTRACT; @@ -7507,10 +7508,10 @@ namespace return (pMD->IsStatic() != FALSE) == isStatic && strcmp(pMD->GetName(), name) == 0; } - // Accepts a field compared via 'EqualityComparer.Default.Equals(this.F, other.F)' (what Roslyn - // emits for record structs), but only when Default.Equals is itself a memcmp: F must be a - // bit-comparable primitive or a nested value type that is itself provably field-wise. - bool IsEqualityComparerDefaultEquals(MethodDesc* pGetDefault, MethodDesc* pEquals, FieldDesc* pField) + // Accepts a field compared via 'EqualityComparer.Default.Equals(this.F, other.F)', but only when + // Default.Equals is itself a memcmp: F must be a bit-comparable primitive or a nested value type that + // is itself provably field-wise. + bool IsEqualityComparerDefaultEquals(MethodDesc* pGetDefault, MethodDesc* pEquals, FieldDesc* pField, StackSArray& scannedMethods) { STANDARD_VM_CONTRACT; @@ -7529,7 +7530,7 @@ namespace MethodTable* pNestedMT = fieldTh.GetMethodTable(); MethodDesc* pNestedEquals = GetIEquatableEqualsImpl(pNestedMT); - return pNestedEquals != NULL && IsFieldwiseEqualsBitwiseEquivalent(pNestedMT, pNestedEquals); + return pNestedEquals != NULL && IsFieldwiseEqualsBitwiseEquivalent(pNestedMT, pNestedEquals, scannedMethods); } // Returns true only if 'pEqualsMD' compares every instance field of 'valueTypeMT' exactly once and @@ -7538,17 +7539,17 @@ namespace // // The C# compiler lowers 'this.f0 == other.f0 && ...' to per-field units sharing one 'return false' // tail. Operands are always arg0/arg1. A primitive is compared inline; a nested value type through - // its own IEquatable.Equals; a record struct field through EqualityComparer.Default.Equals. - // Every call-form callee must itself be field-wise (checked recursively): + // its own IEquatable.Equals; a field may also go through EqualityComparer.Default.Equals. Every + // call-form callee must itself be field-wise (checked recursively): // // primitive, non-final: ldarg.0; ldfld F; ldarg.1; ldfld F; bne.un.s FALSE // primitive, final: ldarg.0; ldfld F; ldarg.1; ldfld F; ceq; ret // nested, non-final: ldarg.0; ldflda F; ldarg.1; ldfld F; call F::Equals; brfalse.s FALSE // nested, final: ldarg.0; ldflda F; ldarg.1; ldfld F; call F::Equals; ret - // record, non-final: call EqualityComparer::get_Default; ldarg.0; ldfld F; ldarg.1; ldfld F; callvirt Equals; brfalse.s FALSE - // record, final: call EqualityComparer::get_Default; ldarg.0; ldfld F; ldarg.1; ldfld F; callvirt Equals; ret + // eqcmp, non-final: call EqualityComparer::get_Default; ldarg.0; ldfld F; ldarg.1; ldfld F; callvirt Equals; brfalse.s FALSE + // eqcmp, final: call EqualityComparer::get_Default; ldarg.0; ldfld F; ldarg.1; ldfld F; callvirt Equals; ret // shared tail: FALSE: ldc.i4.0; ret - bool ScanFieldwiseEqualsBody(MethodDesc* pEqualsMD, MethodTable* valueTypeMT) + bool ScanFieldwiseEqualsBody(MethodDesc* pEqualsMD, MethodTable* valueTypeMT, StackSArray& scannedMethods) { STANDARD_VM_CONTRACT; @@ -7581,24 +7582,23 @@ namespace while (!sawFinalUnit) { - // Optional records lead-in: 'call EqualityComparer::get_Default' before the operands. + // Optional EqualityComparer.Default lead-in: 'call EqualityComparer::get_Default' before + // the operands. mdToken getDefaultTok = mdTokenNil; - bool records = false; if (ip + 5 <= codeSize && pIL[ip] == CEE_CALL) { getDefaultTok = ReadILToken(pIL + ip + 1); - records = true; ip += 5; } - // Left operand: ldarg.0; ldfld/ldflda F. Records and inline '==' load by value; the - // '.Equals' call form loads by address. + // Left operand: ldarg.0; ldfld/ldflda F. The EqualityComparer and inline '==' forms load by + // value; the '.Equals' call form loads by address. if (ip + 6 > codeSize || pIL[ip] != CEE_LDARG_0) return false; BYTE leftLoad = pIL[ip + 1]; if (leftLoad != CEE_LDFLD && leftLoad != CEE_LDFLDA) return false; - if (records && leftLoad != CEE_LDFLD) + if (getDefaultTok != mdTokenNil && leftLoad != CEE_LDFLD) return false; mdToken leftFieldTok = ReadILToken(pIL + ip + 2); ip += 6; @@ -7631,7 +7631,7 @@ namespace return false; compared[numCompared++] = pField; - if (!records && leftLoad == CEE_LDFLD) + if (getDefaultTok == mdTokenNil && leftLoad == CEE_LDFLD) { // Inline '==': only integer-like primitives (and enums) are memcmp-equivalent. if (!IsBitwiseComparableField(pField)) @@ -7660,14 +7660,14 @@ namespace continue; } - if (records) + if (getDefaultTok != mdTokenNil) { // callvirt EqualityComparer::Equals(!0, !0). if (ip + 5 > codeSize || pIL[ip] != CEE_CALLVIRT) return false; MethodDesc* pGetDefault = TryResolveMethodToken(pModule, getDefaultTok); MethodDesc* pEquals = TryResolveMethodToken(pModule, ReadILToken(pIL + ip + 1)); - if (!IsEqualityComparerDefaultEquals(pGetDefault, pEquals, pField)) + if (!IsEqualityComparerDefaultEquals(pGetDefault, pEquals, pField, scannedMethods)) return false; } else @@ -7676,7 +7676,7 @@ namespace if (ip + 5 > codeSize || pIL[ip] != CEE_CALL) return false; MethodDesc* pCallee = TryResolveMethodToken(pModule, ReadILToken(pIL + ip + 1)); - if (!IsPrimitiveEqualsCall(pCallee, pField) && !IsNestedFieldwiseEquatable(pCallee, pField)) + if (!IsPrimitiveEqualsCall(pCallee, pField) && !IsNestedFieldwiseEquatable(pCallee, pField, scannedMethods)) return false; } ip += 5; @@ -7722,8 +7722,9 @@ namespace } // Determines whether 'valueTypeMT's IEquatable.Equals implementation is a plain field-wise - // comparison that is equivalent to memcmp. 'pEqualsMD' is that Equals method. - bool IsFieldwiseEqualsBitwiseEquivalent(MethodTable* valueTypeMT, MethodDesc* pEqualsMD) + // comparison that is equivalent to memcmp. 'pEqualsMD' is that Equals method. Every Equals body the + // decision relies on is appended to 'scannedMethods' so the caller can register a ReJIT dependency. + bool IsFieldwiseEqualsBitwiseEquivalent(MethodTable* valueTypeMT, MethodDesc* pEqualsMD, StackSArray& scannedMethods) { STANDARD_VM_CONTRACT; @@ -7734,11 +7735,17 @@ namespace return false; } + // EnC can replace the Equals IL after the fold, so don't trust the scan for an editable module. + if (pEqualsMD->GetModule()->IsEditAndContinueEnabled()) + { + return false; + } + // First-pass restrictions that keep the scan simple and unquestionably safe: the type must be // unmanaged (so a byte-wise compare is meaningful), tightly packed (no padding anywhere -- the // flag is transitive -- else memcmp inspects bytes Equals ignores), and not an inline array. if (valueTypeMT->ContainsGCPointers() || - valueTypeMT->IsNotTightlyPacked() || + !valueTypeMT->IsTightlyPacked() || valueTypeMT->GetClass()->IsInlineArray()) { return false; @@ -7776,11 +7783,18 @@ namespace } } - return ScanFieldwiseEqualsBody(pScanMD, valueTypeMT); + if (!ScanFieldwiseEqualsBody(pScanMD, valueTypeMT, scannedMethods)) + return false; + + // Record every body the fold relied on (the forwarder and the scanned op_Equality). + scannedMethods.Append(pEqualsMD); + if (pScanMD != pEqualsMD) + scannedMethods.Append(pScanMD); + return true; } } -bool IsBitwiseEquatable(TypeHandle typeHandle, MethodTable * methodTable) +bool IsBitwiseEquatable(TypeHandle typeHandle, MethodTable * methodTable, StackSArray& scannedMethods) { STANDARD_VM_CONTRACT; @@ -7811,7 +7825,7 @@ bool IsBitwiseEquatable(TypeHandle typeHandle, MethodTable * methodTable) iequatableOfSelf, CoreLibBinder::GetMethod(METHOD__IEQUATABLEGENERIC__EQUALS), FALSE /* throwOnConflict */)); if (pEqualsMD != NULL) { - result = IsFieldwiseEqualsBitwiseEquivalent(methodTable, pEqualsMD); + result = IsFieldwiseEqualsBitwiseEquivalent(methodTable, pEqualsMD, scannedMethods); } } } @@ -7825,10 +7839,15 @@ bool IsBitwiseEquatable(TypeHandle typeHandle, MethodTable * methodTable) return result; } +#if defined FEATURE_REJIT && !defined(DACCESS_COMPILE) +static void TrackInliningForRejit(MethodDesc* pCaller, MethodDesc* pCallee); +#endif // defined FEATURE_REJIT && !defined(DACCESS_COMPILE) + static bool getILIntrinsicImplementationForRuntimeHelpers( MethodInfoWorkerContext& cxt, CORINFO_METHOD_INFO* methInfo, - SigPointer* localSig) + SigPointer* localSig, + MethodDesc* pMethodBeingCompiled) { STANDARD_VM_CONTRACT; @@ -7860,10 +7879,23 @@ static bool getILIntrinsicImplementationForRuntimeHelpers( // isn't field-wise (one side is the raw primitive arg), so they're matched by element type. // Everything else -- including Guid, Rune, Int128, and UInt128 -- is proven by IsBitwiseEquatable, // which scans the type's IEquatable.Equals for a field-wise shape. + StackSArray scannedMethods; if (IsBitwiseComparablePrimitive(methodTable->GetInternalCorElementType()) - || IsBitwiseEquatable(typeHandle, methodTable)) + || IsBitwiseEquatable(typeHandle, methodTable, scannedMethods)) { methInfo->ILCode = const_cast(returnTrue); + +#if defined FEATURE_REJIT && !defined(DACCESS_COMPILE) + // The fold "inlined" each scanned Equals body, so a profiler ReJIT of one must rejit the + // method the fold is baked into. No bodies are recorded for the primitive/element-type path. + if (pMethodBeingCompiled != NULL) + { + for (COUNT_T i = 0; i < scannedMethods.GetCount(); i++) + { + TrackInliningForRejit(pMethodBeingCompiled, scannedMethods[i]); + } + } +#endif // defined FEATURE_REJIT && !defined(DACCESS_COMPILE) } else { @@ -8068,7 +8100,7 @@ COR_ILMETHOD_DECODER* CEEInfo::getMethodInfoWorker( } else if (CoreLibBinder::IsClass(pMT, CLASS__RUNTIME_HELPERS)) { - fILIntrinsic = getILIntrinsicImplementationForRuntimeHelpers(cxt, methInfo, &localSig); + fILIntrinsic = getILIntrinsicImplementationForRuntimeHelpers(cxt, methInfo, &localSig, m_pMethodBeingCompiled); } else if (CoreLibBinder::IsClass(pMT, CLASS__ACTIVATOR)) { @@ -8548,6 +8580,49 @@ void CEEInfo::beginInlining(CORINFO_METHOD_HANDLE inlinerHnd, // do nothing } +#if defined FEATURE_REJIT && !defined(DACCESS_COMPILE) +// Records that 'pCaller' incorporated 'pCallee's IL -- a real inline, or the field-wise Equals scan that +// folds RuntimeHelpers.IsBitwiseEquatable -- so that a profiler ReJIT of 'pCallee' also rejits 'pCaller'. +static void TrackInliningForRejit(MethodDesc* pCaller, MethodDesc* pCallee) +{ + STANDARD_VM_CONTRACT; + + pCallee->GetModule()->AddInlining(pCaller, pCallee); + + if (CORProfilerEnableRejit()) + { + ModuleID modId = 0; + mdMethodDef methodDef = mdMethodDefNil; + BOOL shouldCallReJIT = FALSE; + + { + // If ReJIT is enabled, there is a chance that a race happened where the profiler + // requested a ReJIT on a method, but before the ReJIT occurred an inlining happened. + // If we end up reporting an inlining on a method with non-default IL it means the race + // happened and we need to manually request ReJIT for it since it was missed. + CodeVersionManager* pCodeVersionManager = pCallee->GetCodeVersionManager(); + CodeVersionManager::LockHolder codeVersioningLockHolder; + ILCodeVersion ilVersion = pCodeVersionManager->GetActiveILCodeVersion(pCallee); + if (ilVersion.GetRejitState() != RejitFlags::kStateActive || !ilVersion.HasDefaultIL()) + { + shouldCallReJIT = TRUE; + modId = reinterpret_cast(pCaller->GetModule()); + methodDef = pCaller->GetMemberDef(); + // Do Not call RequestReJIT inside this scope, calling RequestReJIT while holding the CodeVersionManager lock + // will cause deadlocks with other threads calling RequestReJIT since it tries to obtain the CodeVersionManager lock + } + } + + if (shouldCallReJIT) + { + _ASSERTE(modId != 0); + _ASSERTE(methodDef != mdMethodDefNil); + ReJitManager::RequestReJIT(1, &modId, &methodDef, static_cast(0)); + } + } +} +#endif // defined FEATURE_REJIT && !defined(DACCESS_COMPILE) + void CEEInfo::reportInliningDecision (CORINFO_METHOD_HANDLE inlinerHnd, CORINFO_METHOD_HANDLE inlineeHnd, CorInfoInline inlineResult, @@ -8673,41 +8748,7 @@ void CEEInfo::reportInliningDecision (CORINFO_METHOD_HANDLE inlinerHnd, { // We don't want to track the chain of methods, so intentionally use m_pMethodBeingCompiled // to just track the methods that pCallee is eventually inlined in - MethodDesc *pCallee = GetMethod(inlineeHnd); - MethodDesc *pCaller = m_pMethodBeingCompiled; - pCallee->GetModule()->AddInlining(pCaller, pCallee); - - if (CORProfilerEnableRejit()) - { - ModuleID modId = 0; - mdMethodDef methodDef = mdMethodDefNil; - BOOL shouldCallReJIT = FALSE; - - { - // If ReJIT is enabled, there is a chance that a race happened where the profiler - // requested a ReJIT on a method, but before the ReJIT occurred an inlining happened. - // If we end up reporting an inlining on a method with non-default IL it means the race - // happened and we need to manually request ReJIT for it since it was missed. - CodeVersionManager* pCodeVersionManager = pCallee->GetCodeVersionManager(); - CodeVersionManager::LockHolder codeVersioningLockHolder; - ILCodeVersion ilVersion = pCodeVersionManager->GetActiveILCodeVersion(pCallee); - if (ilVersion.GetRejitState() != RejitFlags::kStateActive || !ilVersion.HasDefaultIL()) - { - shouldCallReJIT = TRUE; - modId = reinterpret_cast(pCaller->GetModule()); - methodDef = pCaller->GetMemberDef(); - // Do Not call RequestReJIT inside this scope, calling RequestReJIT while holding the CodeVersionManager lock - // will cause deadlocks with other threads calling RequestReJIT since it tries to obtain the CodeVersionManager lock - } - } - - if (shouldCallReJIT) - { - _ASSERTE(modId != 0); - _ASSERTE(methodDef != mdMethodDefNil); - ReJitManager::RequestReJIT(1, &modId, &methodDef, static_cast(0)); - } - } + TrackInliningForRejit(m_pMethodBeingCompiled, GetMethod(inlineeHnd)); } #endif // defined FEATURE_REJIT && !defined(DACCESS_COMPILE) diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index ba51bfaa9ae311..23f4d141dfe632 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -1996,7 +1996,7 @@ class MethodTable OBJECTHANDLE GetLoaderAllocatorObjectHandle(); NOINLINE BYTE *GetLoaderAllocatorObjectForGC(); - BOOL IsNotTightlyPacked(); + BOOL IsTightlyPacked(); BOOL IsAllGCPointers(); diff --git a/src/coreclr/vm/methodtable.inl b/src/coreclr/vm/methodtable.inl index 5bd6ddf711b680..c735a73166111e 100644 --- a/src/coreclr/vm/methodtable.inl +++ b/src/coreclr/vm/methodtable.inl @@ -230,10 +230,10 @@ inline DWORD MethodTable::GetAttrClass() } //========================================================================================== -inline BOOL MethodTable::IsNotTightlyPacked() +inline BOOL MethodTable::IsTightlyPacked() { WRAPPER_NO_CONTRACT; - return GetClass()->IsNotTightlyPacked(); + return !GetClass()->IsNotTightlyPacked(); } //========================================================================================== diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 647d59fa3f9ba5..1ca9cbeef87ce8 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -1880,7 +1880,7 @@ MethodTableBuilder::BuildMethodTableThrowing( for (DWORD i = 0; i < bmtEnumFields->dwNumInstanceFields; i++) { MethodTable* pFieldMT = pByValueClassCache[i]; - if (pFieldMT != NULL && pFieldMT->IsNotTightlyPacked()) + if (pFieldMT != NULL && !pFieldMT->IsTightlyPacked()) { fIsNotTightlyPacked = true; break; From 1df9e78dcb8301fe0b6585e29d771333b9fe694c Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Mon, 20 Jul 2026 09:22:52 -0700 Subject: [PATCH 26/26] Support generic value types in the field-wise equality scan Thread the exact instantiation context through field and token resolution in both the VM and ILC scanners so an unmanaged, tightly-packed generic value type folds like its non-generic counterparts. Shared __Canon instantiations fall out via the pre-existing GC-pointer rejection. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../TypeSystem/IL/Stubs/ComparerIntrinsics.cs | 40 +++-- src/coreclr/vm/jitinterface.cpp | 150 +++++++++++------- .../BitwiseEquatable/BitwiseEquatable.cs | 41 +++++ 3 files changed, 163 insertions(+), 68 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs index d5e28582acf280..7760d7d1f39fb2 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs @@ -332,7 +332,7 @@ public static bool IsBitwiseEquatable(TypeDesc type) if (type is not MetadataType mdType || !mdType.IsValueType) return false; - bool? equatable = ImplementsIEquatable(mdType.GetTypeDefinition()); + bool? equatable = ImplementsIEquatable(mdType); if (!equatable.HasValue) return false; @@ -358,11 +358,6 @@ public static bool IsBitwiseEquatable(TypeDesc type) /// public static bool IsIEquatableEqualsFieldwise(MetadataType type) { - // Generic value types are out of scope: keep the field-wise scan to non-generic types. - // Bail before any layout inspection. - if (type.HasInstantiation) - return false; - // Unmanaged (so a byte-wise compare is meaningful) and tightly packed (no padding anywhere the // compare would inspect) -- matching the CoreCLR VM, which checks these separately. if (type.ContainsGCPointers) @@ -372,19 +367,40 @@ public static bool IsIEquatableEqualsFieldwise(MetadataType type) return false; MethodDesc equalsImpl = GetIEquatableEqualsImplementation(type); - if (equalsImpl is not EcmaMethod ecmaImpl) + if (equalsImpl == null) return false; - MethodIL methodIL = EcmaMethodIL.Create(ecmaImpl); + MethodIL methodIL = GetScannableMethodIL(equalsImpl); + if (methodIL == null) + return false; // A common pattern forwards `bool Equals(T other) => this == other;` to a user-defined // `op_Equality`. Follow that single forward before scanning the field-wise comparison. - if (TryGetOpEqualityForward(methodIL, type) is EcmaMethod forwarded) - methodIL = EcmaMethodIL.Create(forwarded); + if (TryGetOpEqualityForward(methodIL, type) is MethodDesc forwarded) + { + methodIL = GetScannableMethodIL(forwarded); + if (methodIL == null) + return false; + } return ScanFieldwiseEqualsBody(methodIL, type); } + // Builds the IL to scan for a method that may live on an instantiated type. The IL is defined on the + // typical (open) method; wrapping it in an InstantiatedMethodIL makes token lookups resolve fields + // and methods in the exact instantiation. Returns null if the method has no ECMA-backed body. + private static MethodIL GetScannableMethodIL(MethodDesc method) + { + if (method.GetTypicalMethodDefinition() is not EcmaMethod typicalMethod) + return null; + + MethodIL typicalIL = EcmaMethodIL.Create(typicalMethod); + if (typicalIL == null) + return null; + + return method == typicalMethod ? typicalIL : new InstantiatedMethodIL(method, typicalIL); + } + private static bool IsTightlyPacked(MetadataType type) { // Mirrors the CoreCLR VM's MethodTable::IsTightlyPacked: a byte-wise compare equals comparing @@ -428,10 +444,6 @@ private static bool IsTightlyPacked(MetadataType type) private static MethodDesc GetIEquatableEqualsImplementation(MetadataType type) { - // Keep token resolution simple by only handling non-generic value types. - if (type.HasInstantiation) - return null; - MetadataType iequatableType = type.Context.SystemModule.GetKnownType("System"u8, "IEquatable`1"u8); MethodDesc equalsInterfaceMethod = iequatableType.MakeInstantiatedType(type).GetMethod("Equals"u8, null); if (equalsInterfaceMethod == null) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 80b74c79f4a054..7d359485ef808a 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -7324,29 +7324,38 @@ namespace return false; } - // Resolves an in-module FieldDef token. Returns NULL for anything else (e.g. a MemberRef, - // which only arises for generic or cross-module references we deliberately don't handle yet). - FieldDesc* TryResolveInModuleFieldDef(Module* pModule, mdToken token) + // Resolves a field token (FieldDef, or a MemberRef over a generic TypeSpec) using 'pContext'. Returns + // NULL for any other kind or on failure. + FieldDesc* TryResolveFieldToken(Module* pModule, mdToken token, const SigTypeContext* pContext) { STANDARD_VM_CONTRACT; - if (TypeFromToken(token) != mdtFieldDef) - return NULL; - return pModule->LookupFieldDef(token); - } - // Resolves an in-module MethodDef token. Returns NULL for anything else. - MethodDesc* TryResolveInModuleMethodDef(Module* pModule, mdToken token) - { - STANDARD_VM_CONTRACT; - if (TypeFromToken(token) != mdtMethodDef) + mdToken kind = TypeFromToken(token); + if (kind != mdtFieldDef && kind != mdtMemberRef) return NULL; - return MemberLoader::GetMethodDescFromMethodDef(pModule, token, Instantiation(), Instantiation()); + + FieldDesc* pField = NULL; + + EX_TRY + { + pField = MemberLoader::GetFieldDescFromMemberDefOrRef( + pModule, token, pContext, FALSE /* strictMetadataChecks */); + } + EX_CATCH + { + pField = NULL; + RethrowTerminalExceptions(); + } + EX_END_CATCH + + return pField; } // Resolves a method token, including a cross-module MemberRef (a primitive's Equals lives in - // CoreLib). Returns NULL if the token kind is unexpected or resolution fails. - // Callers only match non-generic methods, so a MethodSpec resolves to its generic definition. - MethodDesc* TryResolveMethodToken(Module* pModule, mdToken token) + // CoreLib) or a MethodSpec/MemberRef that mentions the enclosing instantiation. 'pContext' supplies + // that instantiation so tokens over a generic parameter resolve to the concrete argument. Returns + // NULL if the token kind is unexpected or resolution fails. + MethodDesc* TryResolveMethodToken(Module* pModule, mdToken token, const SigTypeContext* pContext) { STANDARD_VM_CONTRACT; @@ -7354,13 +7363,12 @@ namespace if (kind != mdtMethodDef && kind != mdtMemberRef && kind != mdtMethodSpec) return NULL; - SigTypeContext typeContext; MethodDesc* pMD = NULL; EX_TRY { pMD = MemberLoader::GetMethodDescFromMemberDefOrRefOrSpec( - pModule, token, &typeContext, FALSE /* strictMetadataChecks */, FALSE /* allowInstParam */); + pModule, token, pContext, FALSE /* strictMetadataChecks */, FALSE /* allowInstParam */); } EX_CATCH { @@ -7372,6 +7380,34 @@ namespace return pMD; } + // Resolves a type token (TypeDef/TypeRef/TypeSpec) in 'pContext', or the null handle if the kind is + // unexpected or resolution fails. A generic type appears as a TypeSpec, so a raw token compare is not + // enough. + TypeHandle TryResolveTypeToken(Module* pModule, mdToken token, const SigTypeContext* pContext) + { + STANDARD_VM_CONTRACT; + + mdToken kind = TypeFromToken(token); + if (kind != mdtTypeDef && kind != mdtTypeRef && kind != mdtTypeSpec) + return TypeHandle(); + + TypeHandle th; + + EX_TRY + { + th = ClassLoader::LoadTypeDefOrRefOrSpecThrowing( + pModule, token, pContext, ClassLoader::ReturnNullIfNotFound, ClassLoader::FailIfUninstDefOrRef); + } + EX_CATCH + { + th = TypeHandle(); + RethrowTerminalExceptions(); + } + EX_END_CATCH + + return th; + } + // Unwraps an unboxing stub (interface dispatch on a value type) to the instance method that has IL. MethodDesc* UnwrapStub(MethodDesc* pMD) { @@ -7434,19 +7470,22 @@ namespace } // A field is memcmp-comparable if it is a bit-comparable primitive or an enum (always integer-backed). - // FieldDesc::GetFieldType() reports ELEMENT_TYPE_VALUETYPE for enums, so resolve the underlying type - // explicitly -- this keeps the VM in parity with the managed IsBitwiseComparablePrimitive. - bool IsBitwiseComparableField(FieldDesc* pField) + // 'fieldTh' is the field's exact type (resolved against the owning instantiation), so a generic field + // like 'T' is inspected as its concrete argument. + bool IsBitwiseComparableType(TypeHandle fieldTh) { STANDARD_VM_CONTRACT; - CorElementType et = pField->GetFieldType(); + if (fieldTh.IsNull()) + return false; + + CorElementType et = fieldTh.GetSignatureCorElementType(); if (IsBitwiseComparablePrimitive(et)) return true; if (et == ELEMENT_TYPE_VALUETYPE) { - MethodTable* pFieldMT = pField->GetFieldTypeHandleThrowing().GetMethodTable(); + MethodTable* pFieldMT = fieldTh.GetMethodTable(); if (pFieldMT != NULL && pFieldMT->IsEnum()) return IsBitwiseComparablePrimitive(pFieldMT->GetInternalCorElementType()); } @@ -7456,28 +7495,27 @@ namespace // Accepts a primitive field compared via 'x.Equals(y)' instead of 'x == y'; for these integer-like // types both lower to the same bit-for-bit compare. - bool IsPrimitiveEqualsCall(MethodDesc* pCallee, FieldDesc* pField) + bool IsPrimitiveEqualsCall(MethodDesc* pCallee, TypeHandle fieldTh) { STANDARD_VM_CONTRACT; - if (pCallee == NULL || !IsBitwiseComparableField(pField)) + if (pCallee == NULL || !IsBitwiseComparableType(fieldTh)) return false; - MethodTable* pFieldMT = pField->GetFieldTypeHandleThrowing().GetMethodTable(); - MethodDesc* pFieldEquals = GetIEquatableEqualsImpl(pFieldMT); + MethodDesc* pFieldEquals = GetIEquatableEqualsImpl(fieldTh.GetMethodTable()); return pFieldEquals != NULL && pFieldEquals == UnwrapStub(pCallee); } // Accepts a nested value-type field compared through its own IEquatable.Equals, but only when // that Equals is itself a provable field-wise compare (its layout is covered by the recursion). - bool IsNestedFieldwiseEquatable(MethodDesc* pCallee, FieldDesc* pField, StackSArray& scannedMethods) + bool IsNestedFieldwiseEquatable(MethodDesc* pCallee, TypeHandle fieldTh, StackSArray& scannedMethods) { STANDARD_VM_CONTRACT; - if (pCallee == NULL || pField->GetFieldType() != ELEMENT_TYPE_VALUETYPE) + if (pCallee == NULL || fieldTh.GetSignatureCorElementType() != ELEMENT_TYPE_VALUETYPE) return false; - MethodTable* pNestedMT = pField->GetFieldTypeHandleThrowing().GetMethodTable(); + MethodTable* pNestedMT = fieldTh.GetMethodTable(); if (pNestedMT == NULL) return false; @@ -7510,22 +7548,21 @@ namespace // Accepts a field compared via 'EqualityComparer.Default.Equals(this.F, other.F)', but only when // Default.Equals is itself a memcmp: F must be a bit-comparable primitive or a nested value type that - // is itself provably field-wise. - bool IsEqualityComparerDefaultEquals(MethodDesc* pGetDefault, MethodDesc* pEquals, FieldDesc* pField, StackSArray& scannedMethods) + // is itself provably field-wise. 'fieldTh' is the field's exact type. + bool IsEqualityComparerDefaultEquals(MethodDesc* pGetDefault, MethodDesc* pEquals, TypeHandle fieldTh, StackSArray& scannedMethods) { STANDARD_VM_CONTRACT; - TypeHandle fieldTh = pField->GetFieldTypeHandleThrowing(); if (!IsEqualityComparerMethod(pGetDefault, fieldTh, "get_Default", true /* isStatic */) || !IsEqualityComparerMethod(pEquals, fieldTh, "Equals", false /* isStatic */)) { return false; } - if (IsBitwiseComparableField(pField)) + if (IsBitwiseComparableType(fieldTh)) return true; - if (pField->GetFieldType() != ELEMENT_TYPE_VALUETYPE) + if (fieldTh.GetSignatureCorElementType() != ELEMENT_TYPE_VALUETYPE) return false; MethodTable* pNestedMT = fieldTh.GetMethodTable(); @@ -7567,6 +7604,8 @@ namespace const unsigned codeSize = header.GetCodeSize(); Module* pModule = pEqualsMD->GetModule(); + TypeHandle valueTypeTh(valueTypeMT); + SigTypeContext sigTypeContext(valueTypeTh); // Track which instance fields have been compared so we can require full coverage. const DWORD fieldCount = valueTypeMT->GetNumInstanceFields(); @@ -7612,10 +7651,12 @@ namespace if (leftFieldTok != rightFieldTok) return false; - FieldDesc* pField = TryResolveInModuleFieldDef(pModule, leftFieldTok); + FieldDesc* pField = TryResolveFieldToken(pModule, leftFieldTok, &sigTypeContext); + // The resolved field's enclosing MT is the open/approx definition, so match by type-def + // rather than an exact MT compare, which would fail for an instantiation. if (pField == NULL || pField->IsStatic() || - pField->GetApproxEnclosingMethodTable() != valueTypeMT) + !pField->GetApproxEnclosingMethodTable()->HasSameTypeDefAs(valueTypeMT)) { return false; } @@ -7631,10 +7672,13 @@ namespace return false; compared[numCompared++] = pField; + // The field's exact type in this instantiation (e.g. 'T' -> its concrete argument). + TypeHandle fieldTh = pField->GetExactFieldType(valueTypeTh); + if (getDefaultTok == mdTokenNil && leftLoad == CEE_LDFLD) { // Inline '==': only integer-like primitives (and enums) are memcmp-equivalent. - if (!IsBitwiseComparableField(pField)) + if (!IsBitwiseComparableType(fieldTh)) return false; int target; @@ -7665,9 +7709,9 @@ namespace // callvirt EqualityComparer::Equals(!0, !0). if (ip + 5 > codeSize || pIL[ip] != CEE_CALLVIRT) return false; - MethodDesc* pGetDefault = TryResolveMethodToken(pModule, getDefaultTok); - MethodDesc* pEquals = TryResolveMethodToken(pModule, ReadILToken(pIL + ip + 1)); - if (!IsEqualityComparerDefaultEquals(pGetDefault, pEquals, pField, scannedMethods)) + MethodDesc* pGetDefault = TryResolveMethodToken(pModule, getDefaultTok, &sigTypeContext); + MethodDesc* pEquals = TryResolveMethodToken(pModule, ReadILToken(pIL + ip + 1), &sigTypeContext); + if (!IsEqualityComparerDefaultEquals(pGetDefault, pEquals, fieldTh, scannedMethods)) return false; } else @@ -7675,8 +7719,8 @@ namespace // '.Equals' call form: a primitive's own Equals, or a nested type's field-wise Equals. if (ip + 5 > codeSize || pIL[ip] != CEE_CALL) return false; - MethodDesc* pCallee = TryResolveMethodToken(pModule, ReadILToken(pIL + ip + 1)); - if (!IsPrimitiveEqualsCall(pCallee, pField) && !IsNestedFieldwiseEquatable(pCallee, pField, scannedMethods)) + MethodDesc* pCallee = TryResolveMethodToken(pModule, ReadILToken(pIL + ip + 1), &sigTypeContext); + if (!IsPrimitiveEqualsCall(pCallee, fieldTh) && !IsNestedFieldwiseEquatable(pCallee, fieldTh, scannedMethods)) return false; } ip += 5; @@ -7728,13 +7772,6 @@ namespace { STANDARD_VM_CONTRACT; - // Generic value types are out of scope: their Equals bodies can carry TypeSpec/MethodSpec - // tokens needing a type context to resolve. Bail before any layout inspection. - if (valueTypeMT->HasInstantiation()) - { - return false; - } - // EnC can replace the Equals IL after the fold, so don't trust the scan for an editable module. if (pEqualsMD->GetModule()->IsEditAndContinueEnabled()) { @@ -7762,16 +7799,21 @@ namespace COR_ILMETHOD_DECODER header(pILMethod); const BYTE* pIL = header.Code; const unsigned codeSize = header.GetCodeSize(); + Module* pModule = pEqualsMD->GetModule(); + TypeHandle valueTypeTh(valueTypeMT); + SigTypeContext sigTypeContext(valueTypeTh); - // 02 71 03 28 2A + // 02 71 03 28 2A. The 'ldobj' operand is a TypeSpec for a generic + // type, so resolve it in context rather than comparing the raw token. if (pIL != NULL && codeSize == 13 && pIL[0] == CEE_LDARG_0 && - pIL[1] == CEE_LDOBJ && ReadILToken(pIL + 2) == valueTypeMT->GetCl() && + pIL[1] == CEE_LDOBJ && pIL[6] == CEE_LDARG_1 && pIL[7] == CEE_CALL && - pIL[12] == CEE_RET) + pIL[12] == CEE_RET && + TryResolveTypeToken(pModule, ReadILToken(pIL + 2), &sigTypeContext).GetMethodTable() == valueTypeMT) { - MethodDesc* pOpEquality = TryResolveInModuleMethodDef(pEqualsMD->GetModule(), ReadILToken(pIL + 8)); + MethodDesc* pOpEquality = TryResolveMethodToken(pModule, ReadILToken(pIL + 8), &sigTypeContext); if (pOpEquality != NULL && pOpEquality->IsStatic() && pOpEquality->GetMethodTable() == valueTypeMT && diff --git a/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs index 04e644fe52a4e1..5d3797ab6fdf63 100644 --- a/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs +++ b/src/tests/Loader/classloader/BitwiseEquatable/BitwiseEquatable.cs @@ -72,6 +72,18 @@ public static void IsBitwiseEquatable_MatchesExpected() Check(true); Check(true); Check(true); + // Generic value types: the exact instantiation must be threaded through field and token + // resolution. A 'T' field is compared via EqualityComparer.Default.Equals (Roslyn cannot + // emit inline '==' for a type parameter), so this also exercises that path per instantiation. + Check>(true); + Check>(true); + Check>(true); + Check>(false); // reference argument: contains GC pointers + Check>(false); // float: Default.Equals is not bitwise + Check>(true); // inline '==' for an int field plus EqualityComparer for T + Check>(false); + Check>(true);// forwards into a generic op_Equality + Check>(false); // leading byte forces padding before the T field } // The following structs have no Equals/GetHashCode override, so ValueType.Equals/GetHashCode go @@ -102,6 +114,35 @@ public record struct RecPadded(int X, byte Y); public record struct RecFloat(float X, int Y); public record struct RecEnum(ColorInt A, ColorInt B); + // Generic value types. A 'T' field is compared via EqualityComparer.Default.Equals. + public record struct GenPair(T A, T B); + + public struct GenMixed : IEquatable> + { + public int X; public T Y; + public bool Equals(GenMixed o) => X == o.X && System.Collections.Generic.EqualityComparer.Default.Equals(Y, o.Y); + public override bool Equals(object o) => o is GenMixed p && Equals(p); + public override int GetHashCode() => 0; + } + + public struct GenForwardsToOp : IEquatable> + { + public T V; + public bool Equals(GenForwardsToOp o) => this == o; + public static bool operator ==(GenForwardsToOp a, GenForwardsToOp b) => System.Collections.Generic.EqualityComparer.Default.Equals(a.V, b.V); + public static bool operator !=(GenForwardsToOp a, GenForwardsToOp b) => !(a == b); + public override bool Equals(object o) => o is GenForwardsToOp p && Equals(p); + public override int GetHashCode() => 0; + } + + public struct GenPadded : IEquatable> + { + public byte B; public T V; + public bool Equals(GenPadded o) => B == o.B && System.Collections.Generic.EqualityComparer.Default.Equals(V, o.V); + public override bool Equals(object o) => o is GenPadded p && Equals(p); + public override int GetHashCode() => 0; + } + // Int-backed enum: two fields pack to 8 bytes with no padding. public enum ColorInt { A, B, C }