Skip to content

Conversation

@AndyAyersMS
Copy link
Member

This reverts commit b179e19.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 1, 2020
@AndyAyersMS
Copy link
Member Author

Undoing change that I think might have lead to gc stress failures; see #38230 and #36971.

@AndyAyersMS
Copy link
Member Author

Most of the stress runs are done (and all the linux arm ones) -- there are about 30 failures on linux arm amd ~100 total failures.

Arm linux failures are mostly in various HW Intrinsics tests. So it seems the change I've reverted above is the root cause for the spike in failures.

@BruceForstall @CarolEidt -- Seems like we should revert #37679.

@BruceForstall
Copy link
Contributor

Agreed, although #37223 will regress and will need a different fix. It's optional that you'd want to disable the test there that will regress.

@AndyAyersMS
Copy link
Member Author

What's the mechanism for excluding a libraries test?

@BruceForstall
Copy link
Contributor

E.g.,

https://github.com/BruceForstall/runtime/blob/95d9bdb3a4f250b9335035cad28ca6eb32766844/src/libraries/System.Numerics.Vectors/tests/Matrix3x2Tests.cs#L689

[SkipOnCoreClr("https://github.com/dotnet/runtime/issues/36587", RuntimeTestModes.JitStress)]

or, e.g., #38580

@CarolEidt
Copy link
Contributor

Can you articulate why reverting this change eliminates the issue? Codegen still uses r2, I believe, and it is codegen, not LSRA, that tracks the GC state.

@AndyAyersMS
Copy link
Member Author

There are special tricks to extend gc liveness of r0 around the late profile hook and they won't work right if the return value is temporarily elsewhere. I'll add more details when I have a chance (might be a few days).

@AndyAyersMS
Copy link
Member Author

@BruceForstall looks like we didn't specify which test hit this assert, so I don't know what to disable. It might affect more than one test. @CarolEidt do you recall?

@CarolEidt
Copy link
Contributor

@AndyAyersMS - were you asking about which test hit the assert that I fixed with #37679? The issue was #37223 and it failed in System.Threading.Tasks.Dataflow.Tests, though it doesn't directly fail in any of the tests themselves, and I don't recall if I was able to narrow it down to a class or method within those tests.

@AndyAyersMS
Copy link
Member Author

@CarolEidt thanks -- the method was Dictionary.Remove so not surprising if it ends up impacting more than one test or in the harness itself.

Let me see if the failure still repros and if so we can figure out whether to exclude it here or do something else.

@AndyAyersMS AndyAyersMS marked this pull request as ready for review July 15, 2020 00:07
@AndyAyersMS AndyAyersMS reopened this Jul 15, 2020
@AndyAyersMS
Copy link
Member Author

Hmm, can't get CI to run. Let me close this an open a new PR.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants