-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
JIT: Cast UInt64 to Single directly during const folding #106419
Changes from 1 commit
ceb81bf
d729037
0a36998
a740f04
c98af0a
08370ac
70c60b3
8dd3473
163d581
3900e5d
de0d0c9
08fd38c
6ee12fc
0ea8525
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,8 @@ | |
<Optimize>True</Optimize> | ||
<!-- Needed for CLRTestTargetUnsupported --> | ||
<RequiresProcessIsolation>true</RequiresProcessIsolation> | ||
<CLRTestTargetUnsupported Condition="'$(TargetArchitecture)' != 'arm64' or '$(RuntimeFlavor)' != 'coreclr'">true</CLRTestTargetUnsupported> | ||
<CLRTestTargetUnsupported Condition="'$(TargetArchitecture)' != 'arm64' AND '$(TargetArchitecture)' != 'x64'">true</CLRTestTargetUnsupported> | ||
<CLRTestTargetUnsupported Condition="'$(RuntimeFlavor)' != 'coreclr'">true</CLRTestTargetUnsupported> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only need is we skip this on Mono today, right? Can we not instead use There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
</PropertyGroup> | ||
<ItemGroup> | ||
<Compile Include="$(MSBuildProjectName).cs" /> | ||
|
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.
Should the test rather assert that its producing
1600094604u
if(RuntimeInformation.ProcessArchitecture == Architecture.Arm64) || Avx512F.IsSupported
isfalse
?That way we can detect any changes for other platforms or scenarios?
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.
Good idea; I'll update it.
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.
I just remembered that we don't build this test if the target arch isn't arm64/x64 (or if the runtime isn't CoreCLR). @tannergooding are you ok with only testing those platforms for now?
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.
I think that’s fine, but ideally we’d ensure this runs everywhere long term