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 Test Failures #43534

Closed
AndyAyersMS opened this issue Oct 16, 2020 · 1 comment · Fixed by #53897
Closed

OSR Test Failures #43534

AndyAyersMS opened this issue Oct 16, 2020 · 1 comment · Fixed by #53897
Assignees
@AndyAyersMS
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone
6.0.0

Comments

@AndyAyersMS
Copy link
Member

AndyAyersMS commented Oct 16, 2020 •
edited by BruceForstall
Loading

Two different types of failures have cropped up in the experimental OSR test runs

See test results.

  1. Assertion failed ppInfo->HasKeptAliveThis() in roslyn based tests
  2. Assertion failed pData->profiledRsp == (void*)ctx.Rsp in profiler tests

category:correctness
theme:osr
skill-level:expert
cost:medium
@AndyAyersMS AndyAyersMS added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Oct 16, 2020
@AndyAyersMS AndyAyersMS added this to the 6.0.0 milestone Oct 16, 2020
@AndyAyersMS AndyAyersMS self-assigned this Oct 16, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Oct 16, 2020
@jeffschwMSFT jeffschwMSFT removed the untriaged New issue has not been triaged by the area owner label Oct 16, 2020
@BruceForstall BruceForstall added the JitUntriaged CLR JIT issues needing additional triage label Oct 28, 2020
@BruceForstall BruceForstall removed the JitUntriaged CLR JIT issues needing additional triage label Nov 7, 2020
AndyAyersMS added a commit to AndyAyersMS/runtime that referenced this issue Apr 10, 2021
@AndyAyersMS
JIT: fix osr gc info this reporting
829ca65 
With the advent of dotnet#38229 an optimized method may need to report generics
context via `this` while the un-optimzed version did not need to report.

This impacts OSR, which previously was always trying to use the unoptimized
root method frame reporting. Now under OSR we must sometimes add a slot to
the OSR frame instead.

Addresses one of the failure cases in dotnet#43534.
@AndyAyersMS AndyAyersMS mentioned this issue Apr 10, 2021
Merged
AndyAyersMS added a commit that referenced this issue Apr 11, 2021
@AndyAyersMS
JIT: fix osr gc info this reporting (#51057)
125253b 
With the advent of #38229 an optimized method may need to report generics
context via `this` while the un-optimzed version did not need to report.

This impacts OSR, which previously was always trying to use the unoptimized
root method frame reporting. Now under OSR we must sometimes add a slot to
the OSR frame instead.

Addresses one of the failure cases in #43534.
@AndyAyersMS
Copy link
Member Author

First set of failures is now fixed.

The second set of failures are cases where we have a profiler leave hook in an OSR method. The jit is not passing back the expected value of the caller stack pointer, since it require more work to get this for OSR. This needs to be fixed both for windows and SysV. Here's the current code for the latter:

runtime/src/coreclr/jit/codegenxarch.cpp

Lines 8923 to 8929 in 2ada920

// RSI = caller's SP
if (compiler->lvaDoneFrameLayout == Compiler::FINAL_FRAME_LAYOUT)
{
int callerSPOffset = compiler->lvaToCallerSPRelativeOffset(0, isFramePointerUsed());
GetEmitter()->emitIns_R_AR(INS_lea, EA_PTRSIZE, REG_ARG_1, genFramePointerReg(), -callerSPOffset);
}
else

@AndyAyersMS AndyAyersMS mentioned this issue Apr 27, 2021
Merged
@AndyAyersMS AndyAyersMS mentioned this issue May 20, 2021
Merged
AndyAyersMS added a commit to AndyAyersMS/runtime that referenced this issue Jun 8, 2021
@AndyAyersMS
JIT: fix OSR reporting for special stack slots
536842f 
Revise the reporting of the special stack slots for OSR to be more uniform.
* Always record the original method FP-relative offset.
* Always apply the same adjustment for original method slosts i the OSR frame
* Handle caller-SP relative adjustment in `lvaToCallerSPRelativeOffset`

In particular, this fixes dotnet#43534 where we were reporting the wrong caller SP
for the profiler exit hook.
@AndyAyersMS AndyAyersMS mentioned this issue Jun 8, 2021
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 8, 2021
AndyAyersMS added a commit that referenced this issue Jun 9, 2021
@AndyAyersMS
JIT: fix OSR reporting for special stack slots (#53897)
3d13204 
Revise the reporting of the special stack slots for OSR to be more uniform.
* Always record the original method FP-relative offset.
* Always apply the same adjustment for original method slosts i the OSR frame
* Handle caller-SP relative adjustment in `lvaToCallerSPRelativeOffset`

In particular, this fixes #43534 where we were reporting the wrong caller SP
for the profiler exit hook.
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 9, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jul 9, 2021
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
Projects
None yet
Milestone
6.0.0
Development

Successfully merging a pull request may close this issue.

JIT: fix OSR reporting for special stack slots AndyAyersMS/runtime
4 participants
@jeffschwMSFT @AndyAyersMS @BruceForstall @Dotnet-GitSync-Bot