Add generic And<T> and Or<T> methods to System.Threading.Interlocked#120978
Add generic And<T> and Or<T> methods to System.Threading.Interlocked#120978
Conversation
Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
…methods Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
|
@copilot find existing usages of |
Found and replaced all usages of `Interlocked.And(ref Unsafe.As<EnumType, IntType>(ref field), value)` and `Interlocked.Or(ref Unsafe.As<EnumType, IntType>(ref field), value)` with the new generic API `Interlocked.And(ref field, value)` and `Interlocked.Or(ref field, value)`. Affected files: - System.Private.CoreLib/ComWrappers.cs (CreateComInterfaceFlagsEx enum) - System.Net.Http/CacheControlHeaderValue.cs (Flags enum) - System.Private.Uri/Uri.cs (Flags enum) - System.Private.Uri/UriSyntax.cs (Uri.Flags enum) - System.Private.Uri/UriScheme.cs (Uri.Flags enum) Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Done! Found and replaced 7 usages across 5 files: System.Private.CoreLib
System.Net.Http
System.Private.Uri
All calls now use |
|
@copilot in importercallls.cpp for: case NI_System_Threading_Interlocked_Or:
case NI_System_Threading_Interlocked_And:can you disable the intrinsic expansion for |
Modified importercalls.cpp to only expand the intrinsic for And/Or operations when genActualType(callType) is TYP_INT or TYP_LONG. Other types (like byte, short) now fall back to the managed implementation which uses CompareExchange loops. This ensures that: - int/uint and long/ulong use native atomic instructions - byte/sbyte and short/ushort use managed CompareExchange-based implementation - Generic method calls work correctly for all supported types Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
... Done! Modified The change:
All 611 System.Threading tests pass. (256f950) |
src/libraries/System.Private.CoreLib/src/System/Threading/Interlocked.cs
Outdated
Show resolved
Hide resolved
|
@copilot Change the added tests:
|
- Use callType directly in JIT intrinsic check as suggested by @EgorBo - Allow float/double support (they work via bitcast to int/long for bitwise operations) - Convert all And/Or generic tests to [Theory] with [InlineData] - Add 5-10 test cases per type including negative values and edge cases - Add comprehensive tests for float/double types - Total test count increased from 611 to 712 All tests pass successfully. Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Done! All test changes implemented:
Test count increased from 611 to 712. All tests pass. (d549cdd) |
Ah, that is a good question, it looks like Type.GetTypeCode(typeof(T)) is not a direct intrinsic and relies on it being inlined, so probably it's not a good choice indeed, didn't know that 🤔 |
|
Right. Also, |
src/libraries/System.Private.CoreLib/src/System/Threading/Interlocked.cs
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Threading/Interlocked.cs
Show resolved
Hide resolved
I've reverted to GetEnumUnderlyingType. Does it look good otherwise? |
src/libraries/System.Private.CoreLib/src/System/Threading/Interlocked.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Threading/Interlocked.cs
Outdated
Show resolved
Hide resolved
|
@copilot address the feedback over the NotSupportedException check to make it single level. |
Changed the NotSupportedException validation logic from nested if statements to a single-level if with a compound condition using logical operators. This addresses PR review feedback to: - Remove unnecessary nesting - Fix indentation issues - Make the logic more readable and maintainable The condition now checks all invalid type scenarios in one expression: - Non-primitive and non-enum types - float and double primitives - Enums backed by float or double All 705 tests pass successfully. Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
|
/ba-g deadletter |
|
/ba-g deadletter |
Summary
This PR adds generic versions of the
AndandOrmethods toSystem.Threading.Interlockedas specified in API proposal #114568.Changes
And<T>(ref T, T) where T : structmethodOr<T>(ref T, T) where T : structmethodInterlocked.And/Or(ref Unsafe.As...)with new generic APIImplementation Details
byte,sbyte,short,ushort): Uses CompareExchange-based loop (managed fallback)int,uint): Delegates to int And/Or with bitcast (JIT intrinsic)long,ulong): Delegates to long And/Or with bitcast (JIT intrinsic)Type.GetEnumUnderlyingType()check (JIT-foldable)CompareExchange<T>JIT Changes
Modified
importercalls.cppto restrict intrinsic expansion:TYP_INTandTYP_LONGare expanded as atomic operationscallTypeconsistently for type checksType Rejection
The methods reject:
Type.GetEnumUnderlyingType())The validation check is a single-level if statement with proper indentation:
Documentation
The XML documentation clearly states the restrictions:
Test Coverage
Converted all tests to
[Theory]with[InlineData]covering:Usages Replaced
Replaced 7 call sites across the codebase:
System.Private.CoreLib/ComWrappers.cs(2 calls)System.Net.Http/CacheControlHeaderValue.cs(2 calls)System.Private.Uri/Uri.cs(1 call)System.Private.Uri/UriSyntax.cs(1 call)System.Private.Uri/UriScheme.cs(1 call)All usages now use the cleaner generic API without
Unsafe.Ascasts.Test Results
✅ All 705 System.Threading tests pass (0 errors, 0 failures)
Security
✅ CodeQL analysis passed with no issues
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.