-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Conversation
Need to check `supportSIMDType()` and not `featureSIMD` in rationalize. Fixes #24055.
@CarolEidt PTAL We should probably audit the other uses of |
@AndyAyersMS - thanks! I'd started to look at this (@echesakovMSFT had mentioned this on #23675), but ran into some process issues. |
@AndyAyersMS - there were also failures in RayTracer and Vector3Interop that were related to Vector3, and a few other testrs that seem to be SIMD-related that I haven't looked into: https://dev.azure.com/dnceng/public/_build/results?buildId=157586&view=ms.vss-test-web.build-test-results-tab If you could verify that those succeed, that would be great. I don't think there's a way to trigger r2r testing on Arm64, but if you'd prefer I can check whether this fixes those as well. |
Sure, I'll look into those failures. I already have everything set up and ready to go. |
A bunch of these now pass, but some still fail. DevDiv_714266 and GitHub_20260: fix looks similar.
ConsoleMandel: failure looks different.
Let me verify the fix for the first two and then I'll look into this one. |
The ConsoleMandel one seems to be asserting on the |
Pushed a fix for the first assert. |
Thanks @AndyAyersMS - I think that the CSE case is another place where we're bitten by the fact that we don't retain handles on rvalues in all cases. I think the right thing to do here is probably a workaround, but I think something like @mikedn's work to cache the layout and retain it on rhs as well as lhs will be a better approach. |
Workaround, as in ... ? I may need to leave that part to you -- you can either take over this PR (add my public fork as a remote and push to the branch), or do that as a separate fix. |
The workaround would be to exempt the UpperSave and UpperRestore intrinsics from the base type check as the base type is not used in those. |
I'd suggest that we go ahead and merge your fixes, and I'll put up a separate PR to fix the ConsoleMandel case. |
Formatter failure seems spurious. @dotnet-bot retest Ubuntu x64 Formatting |
Failed again -- ubuntu formatting leg seems to be broken:
|
There's an earlier error (from jitutils build.cmd?); not sure if that matters:
|
I think that particular failure is for the |
Arm64 failure was in the "contextual reflection" test. Seems likely to be unrelated? But did not fail in neighboring test runs. So will retry.... @dotnet-bot retest Windows_NT arm64 Cross Checked Innerloop Build and Test |
@dotnet-bot test Ubuntu arm Cross Checked crossgen_comparison Build and Test |
@dotnet-bot test Ubuntu x64 Formatting |
@dotnet/jit-contrib - I propose that we go ahead and merge this. The only changes are under |
@RussKeldorph Are you re-triggering crossgen_comparison because it failed? Because you're just adding to our arm32 backlog problem. |
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.
Looks Good
@BruceForstall Yes. |
JIT: fix arm64 crossgen simd issue Commit migrated from dotnet/coreclr@af83b8a
Need to check
supportSIMDType()
and notfeatureSIMD
in rationalize.Fixes #24055.