Skip to content
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.

Sign up for GitHub

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

Jump to bottom

OSR tests failing in jit-experimental pipeline #86125

Closed
AndyAyersMS opened this issue May 11, 2023 · 2 comments · Fixed by #86157
Closed

OSR tests failing in jit-experimental pipeline #86125

AndyAyersMS opened this issue May 11, 2023 · 2 comments · Fixed by #86157
Assignees
@AndyAyersMS
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI blocking-clean-ci-optional Blocking optional rolling runs
Milestone
8.0.0

Comments

@AndyAyersMS
Copy link
Member

https://dev.azure.com/dnceng-public/public/_build/results?buildId=269074&view=ms.vss-test-web.build-test-results-tab

set DOTNET_TieredCompilation=1
set DOTNET_TC_OnStackReplacement=1
set DOTNET_TC_QuickJitForLoops=1
set DOTNET_TC_OnStackReplacement_InitialCounter=1
set DOTNET_OSR_HitLimit=2
set DOTNET_JitRandomOnStackReplacement=1

  Starting:    JIT.Generics.XUnitWrapper (parallel test collections = on, max threads = 4)
    JIT\Generics\ConstrainedCall\vt4_il_r\vt4_il_r.cmd [FAIL]
      
      Assert failure(PID 3644 [0x00000e3c], Thread: 3716 [0x0e84]): Assertion failed 'compMinOptsIsSet' in 'MyCounter[MyInt]:Val():int:this' during 'Pre-import' (IL size 21; hash 0x4d594f91; Optimization-Level-Not-Yet-Set)
      
          File: D:\a\_work\1\s\src\coreclr\jit\compiler.h Line: 9272
          Image: C:\h\w\9C710833\p\corerun.exe
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 11, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label May 11, 2023
@AndyAyersMS AndyAyersMS self-assigned this May 11, 2023
@ghost
Copy link

ghost commented May 11, 2023

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

https://dev.azure.com/dnceng-public/public/_build/results?buildId=269074&view=ms.vss-test-web.build-test-results-tab

set DOTNET_TieredCompilation=1
set DOTNET_TC_OnStackReplacement=1
set DOTNET_TC_QuickJitForLoops=1
set DOTNET_TC_OnStackReplacement_InitialCounter=1
set DOTNET_OSR_HitLimit=2
set DOTNET_JitRandomOnStackReplacement=1

  Starting:    JIT.Generics.XUnitWrapper (parallel test collections = on, max threads = 4)
    JIT\Generics\ConstrainedCall\vt4_il_r\vt4_il_r.cmd [FAIL]
      
      Assert failure(PID 3644 [0x00000e3c], Thread: 3716 [0x0e84]): Assertion failed 'compMinOptsIsSet' in 'MyCounter[MyInt]:Val():int:this' during 'Pre-import' (IL size 21; hash 0x4d594f91; Optimization-Level-Not-Yet-Set)
      
          File: D:\a\_work\1\s\src\coreclr\jit\compiler.h Line: 9272
          Image: C:\h\w\9C710833\p\corerun.exe
Author: AndyAyersMS
Assignees: -
Labels:

area-CodeGen-coreclr, untriaged
Milestone: -

@AndyAyersMS AndyAyersMS added blocking-clean-ci-optional Blocking optional rolling runs and removed untriaged New issue has not been triaged by the area owner labels May 11, 2023
@AndyAyersMS AndyAyersMS added this to the 8.0.0 milestone May 11, 2023
@AndyAyersMS
Copy link
Member Author

This is fallout from #85860, where we decide to fuse simple branch-to-next in some compilation modes but not all. We have (in Tier0) put a patchpoint at the start of the next block of a simple branch-to-next. But when we OSR rejit we fuse and so fail to find the OSR entry block. This leads to invoking a noway_assert before we have determined the opt level, and so we hit a regular assert.

Possible fixes:

  • don't allow patchpoints at the start of blocks that might be fused in optimized or instrumented compiles. This might be tricky since we can set patchpoints at the start of backedge source blocks and those might be fusion targets.
  • always fuse blocks if a method can have patchpoints (we probably don't know this early enough)
  • always fuse blocks unless debug or true minopts codegen (seems appealing but might lead to some SPMI breaks?)
  • find the block containing the right IL offset, scan through the statements in the block looking for one with the right IL offset and split the block in two

AndyAyersMS added a commit to AndyAyersMS/runtime that referenced this issue May 12, 2023
@AndyAyersMS
JIT: do early block merging in more cases
fd5a270 
OSR and PGO both rely on the fact that the early flowgraph the JIT builds is
the same flowgraph as the one seen in a previous JIT complation of that method,
since there is metadata (patchpoint offset, pgo schema) that refers to the
flowgraph. Previous work here (dotnet#85860) didn't ensure this for enough cases.

In particular if Tier0 does not do early block merging, but OSR does, it's possible
for the OSR entry point to fall within a merged block range, which the JIT is not
prepared to handle. This lead to the asserts seen in dotnet#86125.

The fix is to enable early block merging unless we're truly in a minopts or
debug codegen mode (previous to this Tier0 would not merge unless it also
was instrumenting; now it will always merge).

An alternative fix would be to find the block containing the OSR entry IL
offset, scan its statements, and split the block at the right spot, but that
seemed more involved.

Fixes dotnet#86125.
@AndyAyersMS AndyAyersMS mentioned this issue May 12, 2023
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 12, 2023
AndyAyersMS added a commit that referenced this issue May 13, 2023
@AndyAyersMS
JIT: do early block merging in more cases (#86157)
aa38055 
OSR and PGO both rely on the fact that the early flowgraph the JIT builds is
the same flowgraph as the one seen in a previous JIT complation of that method,
since there is metadata (patchpoint offset, pgo schema) that refers to the
flowgraph. Previous work here (#85860) didn't ensure this for enough cases.

In particular if Tier0 does not do early block merging, but OSR does, it's possible
for the OSR entry point to fall within a merged block range, which the JIT is not
prepared to handle. This lead to the asserts seen in #86125.

The fix is to enable early block merging unless we're truly in a minopts or
debug codegen mode (previous to this Tier0 would not merge unless it also
was instrumenting; now it will always merge).

An alternative fix would be to find the block containing the OSR entry IL
offset, scan its statements, and split the block at the right spot, but that
seemed more involved.

Fixes #86125.
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 13, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jun 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@AndyAyersMS AndyAyersMS

Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI blocking-clean-ci-optional Blocking optional rolling runs
Projects
None yet
Milestone
8.0.0
Development

Successfully merging a pull request may close this issue.

JIT: do early block merging in more cases AndyAyersMS/runtime
1 participant
@AndyAyersMS