-
Notifications
You must be signed in to change notification settings - Fork 5.1k
JIT: Cast UInt64 to Single directly during const folding #106419
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
Merged
Merged
Changes from 12 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
ceb81bf
Fix cast folding on ARM64
amanasifkhalid d729037
Add test
amanasifkhalid 0a36998
Fix weird comment spacing
amanasifkhalid a740f04
Fix test
amanasifkhalid c98af0a
Fix test
amanasifkhalid 08370ac
Flip params
amanasifkhalid 70c60b3
Run test on CoreCLR only
amanasifkhalid 8dd3473
Add RequiresProcessIsolation
amanasifkhalid 163d581
Merge branch 'main' into fp-cast-arm64
amanasifkhalid 3900e5d
Do single-step conversion on all platforms
amanasifkhalid de0d0c9
Expand test coverage
amanasifkhalid 08fd38c
Update test
amanasifkhalid 6ee12fc
Try running test on all CoreCLR platforms
amanasifkhalid 0ea8525
Fix condition
amanasifkhalid File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
35 changes: 35 additions & 0 deletions
35
src/tests/JIT/Regression/JitBlue/Runtime_106338/Runtime_106338.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
// Generated by Fuzzlyn v2.2 on 2024-08-13 00:04:04 | ||
// Run on Arm64 MacOS | ||
// Seed: 13207615092246842583-vectort,vector64,vector128,armadvsimd,armadvsimdarm64,armaes,armarmbase,armarmbasearm64,armcrc32,armcrc32arm64,armdp,armrdm,armrdmarm64,armsha1,armsha256 | ||
// Reduced from 226.8 KiB to 0.4 KiB in 00:02:12 | ||
// Debug: Outputs 1600094603 | ||
// Release: Outputs 1600094604 | ||
using System; | ||
using System.Runtime.InteropServices; | ||
using System.Runtime.Intrinsics.X86; | ||
using Xunit; | ||
|
||
public class Runtime_106338 | ||
{ | ||
[Fact] | ||
public static void TestEntryPoint() | ||
{ | ||
ulong vr10 = 16105307123914158031UL; | ||
float vr11 = 4294967295U | vr10; | ||
uint result = BitConverter.SingleToUInt32Bits(vr11); | ||
|
||
if ((RuntimeInformation.ProcessArchitecture == Architecture.Arm64) || Avx512F.IsSupported) | ||
{ | ||
// Expected to cast ulong -> float directly | ||
Assert.Equal(1600094603U, result); | ||
} | ||
else | ||
{ | ||
// Expected to cast ulong -> double -> float | ||
Assert.Equal(1600094604U, result); | ||
} | ||
} | ||
} |
12 changes: 12 additions & 0 deletions
12
src/tests/JIT/Regression/JitBlue/Runtime_106338/Runtime_106338.csproj
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
<PropertyGroup> | ||
<Optimize>True</Optimize> | ||
<!-- Needed for CLRTestTargetUnsupported --> | ||
<RequiresProcessIsolation>true</RequiresProcessIsolation> | ||
<CLRTestTargetUnsupported Condition="'$(TargetArchitecture)' != 'arm64' AND '$(TargetArchitecture)' != 'x64'">true</CLRTestTargetUnsupported> | ||
<CLRTestTargetUnsupported Condition="'$(RuntimeFlavor)' != 'coreclr'">true</CLRTestTargetUnsupported> | ||
</PropertyGroup> | ||
<ItemGroup> | ||
<Compile Include="$(MSBuildProjectName).cs" /> | ||
</ItemGroup> | ||
</Project> |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
The only need is we skip this on Mono today, right?
Is that because they're always doing the
ulong -> double -> float
2-step behavior?Can we not instead use
[SkipOnMono("https://github.com/dotnet/runtime/issues/#######", TestPlatforms.Any)]
and ensure that the test is compiled for all platforms?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.
Yes, Mono seems to also do the two-step cast, so the test was failing across all Mono legs.
Sure, I'll update it.