Repository navigation
Correct integer switch table dispatch - #583
Conversation
| if (customEqualMethod == null && caseCount >= minSwitchTableSize && switchValueType.IsIntegerOrUnderlyingInteger() && | ||
| Type.GetTypeCode(switchValueType) is not (TypeCode.Int64 or TypeCode.UInt64)) |
There was a problem hiding this comment.
I think we may create a IsIntegerOrUnderlyingIntegerUpTo32bits which drops TypeCode.Int64 or TypeCode.UInt64 internally and used here instead of more complex comparison. May be can be reused in other places too
There was a problem hiding this comment.
Implemented in 772360c: IsIntegerOrUnderlyingIntegerUpTo32bits now replaces the compound condition at the switch-table gate. It uses one TypeCode classification for primitive and enum types, accepting Char/SByte/Byte/Int16/UInt16/Int32/UInt32 and excluding Int64/UInt64 (including their enums). Wide operands keep the existing comparison dispatch.
I found no other current call site that needs the 32-bit restriction, so I kept this change local. I also extended IntegerSwitchDispatchRegressionTests to exercise the smaller integer widths and their enums. All five registered methods pass for both Standard and LightExpression on .NET 8, .NET 10 and .NET Framework 4.7.2, including the existing wide-value alias, negative/unsigned origin, duplicate-case and single-evaluation checks. Release probe builds passed with zero warnings/errors. This helper extraction was not performance-remeasured; the PR description distinguishes the initial measurements from this follow-up.
772360c to
39c658e
Compare
|
lgtm |
Limit IL switch-table indices to 32-bit integer types. Keep long and ulong operands on ordered comparison dispatch. Normalize table origins by subtracting the actual Int32 origin and emit Int32 outlier comparisons. Preserve first-case selection for duplicate tests. Standard and Light regressions cover signed/unsigned 32-bit origins, wide values and enums, low-bit aliases, duplicate tests and single operand evaluation. Focused tests pass on net8.0, net10.0 and net472; the unfixed compiler fails the switch matrix. Full solution builds, legacy suites and Framework runners pass. Existing master MultiplyChecked parity failures reproduce identically on net8.0/net10.0.
Use IsIntegerOrUnderlyingIntegerUpTo32bits at the switch-table gate, as suggested in review. Classify primitive and enum types with a single TypeCode check, preserving the ordered-comparison fallback for Int64/UInt64. Extend the shared switch regression fixture across all smaller integer widths and their enum types. Five registered methods per API pass on .NET 8, .NET 10 and .NET Framework 4.7.2; Release probe builds for all three targets pass with zero warnings/errors.
39c658e to
a76fcc6
Compare
Integer switch-table normalization can select the wrong/default branch for negative or unsigned origins, and 64-bit values can alias their low bits when fed to the 32-bit IL switch instruction.
Keep Int64/UInt64 operands (including enums) on the existing ordered-comparison path. Emit Int32 origins/outlier comparisons and subtract the actual table origin. Preserve upstream duplicate-test handling. The review follow-up uses
IsIntegerOrUnderlyingIntegerUpTo32bitsat the table gate to express the supported operand types directly.Based on freshly fetched upstream master
50ace74b3438c273e9e9ae215530719f3213f029, with two commits:b2c9cb53(dispatch correctness) anda76fcc6b(the helper suggested by dadhi and expanded small-integer coverage). Independent of #558 and the other standalone fixes.Correctness and validation
Five shared regression methods cover smaller integer widths and their enums, first-case duplicate selection, signed/unsigned 64-bit operands and enums, low-bit aliases, negative/unsigned 32-bit origin boundaries, defaults, and single evaluation of the switch operand. Results are checked against System compilation with strict FEC compilation for both Standard and LightExpression.
Validation rerun after the November 4 rebase:
Rebase
Runner-registration conflicts were resolved by retaining all upstream registrations, including the newly merged by-ref conversion and custom/Extension reduction fixtures, alongside this PR's IntegerSwitchDispatchRegressionTests. The production changes and regression tests are unchanged; both logical commits are preserved.
Performance was not remeasured after the rebase. Earlier tables used the previous master revision and have been removed to avoid presenting stale values as current-base measurements. No new timing or allocation claim is made for this head.