-
Notifications
You must be signed in to change notification settings - Fork 5.1k
JIT: arrays are now not always TYP_REF #115801
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Remove some asserts saying otherwise. Fixes dotnet#115495.
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses runtime issue #115495 by adding a new JIT regression test and updating the SIMD code to remove incorrect assumptions about array reference types.
- Introduces a new test project (
Runtime_115495
) with optimized build settings. - Adds a C# test (
Runtime_115495.cs
) that verifiesVector3.GetHashCode
behavior and exercises array-based loops. - Removes two
assert
calls insimd.cpp
that assumed array elements were alwaysTYP_REF
.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
File | Description |
---|---|
src/tests/JIT/Regression/JitBlue/Runtime_115495/Runtime_115495.csproj | New test project without target framework specified |
src/tests/JIT/Regression/JitBlue/Runtime_115495/Runtime_115495.cs | C# test case with custom Assert and array-based checksum logic |
src/coreclr/jit/simd.cpp | Removed outdated assert checks for TYP_REF on array refs |
Comments suppressed due to low confidence (4)
src/tests/JIT/Regression/JitBlue/Runtime_115495/Runtime_115495.cs:17
- [nitpick] This
Assert
class shadowsXunit.Assert
, which could be confusing. Consider renaming it (e.g.,SimpleAssert
) to avoid conflicts.
public class Assert
src/tests/JIT/Regression/JitBlue/Runtime_115495/Runtime_115495.cs:35
- [nitpick] The static delegate field
multiply
should use PascalCase (Multiply
) or include a suffix (multiplyFunc
) to follow .NET naming conventions.
static Func<int, int> multiply = x => x * 2;
src/tests/JIT/Regression/JitBlue/Runtime_115495/Runtime_115495.cs:55
- Currently the test always enters the
foreach
branch. Add a variant wherechecksum % 4 != 0
to cover the branch where the loop is skipped.
if (checksum % 4 == 0)
src/tests/JIT/Regression/JitBlue/Runtime_115495/Runtime_115495.csproj:3
- The new project file lacks a
<TargetFramework>
element, so it may not compile or run tests. Please specify a target framework (e.g.,net7.0
ornet8.0
).
<Optimize>True</Optimize>
@dotnet/jit-contrib PTAL |
/ba-g this is just removing an assert |
Remove some asserts saying otherwise. Fixes #115495.
Remove some asserts saying otherwise.
Fixes #115495.