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.

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

OSR gets wrong offset for memory arg on arm64 OSX #68194

Closed
AndyAyersMS opened this issue Apr 19, 2022 · 2 comments · Fixed by #68202
Closed

OSR gets wrong offset for memory arg on arm64 OSX #68194

AndyAyersMS opened this issue Apr 19, 2022 · 2 comments · Fixed by #68202
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Comments

@AndyAyersMS
Copy link
Member

AndyAyersMS commented Apr 19, 2022

Running libraries pgo w/osr "stress", arm64 OSX (trying to verify fix for #68170) I hit a different error.

export COMPlus_TieredCompilation=1
export COMPlus_TC_OnStackReplacement=1
export COMPlus_TC_QuickJitForLoops=1
export COMPlus_TC_OnStackReplacement_InitialCounter=1
export COMPlus_OSR_HitLimit=1

%% dotnet exec --runtimeconfig System.Text.Json.Tests.runtimeconfig.json --depsfile System.Text.Json.Tests.deps.json xunit.console.dll System.Text.Json.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing -parallel none -method System.Text.Json.Nodes.Tests.JsonArrayTests.ConvertJSONArrayToObjectArray

  Discovering: System.Text.Json.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  System.Text.Json.Tests (found 1 of 4368 test case)
  Starting:    System.Text.Json.Tests (parallel test collections = off, max threads = 10)
Process terminated. Assertion failed.
owner and m cannot be set for transparent methods

Isolated this to OSR codegen for DynamicMethod:Init.

Note arg8 and arg9 have the same offset in the OSR disasm below (and likewise in the codegen)

; Assembly listing for method System.Reflection.Emit.DynamicMethod:Init(System.String,int,int,System.Type,System.Type[],System.Type,System.Reflection.Module,bool,bool):this
; Emitting BLENDED_CODE for generic ARM64 CPU - MacOS
; Tier-1 compilation
; OSR variant for entry point 0x91
; optimized code
; fp based frame
; partially interruptible
; No PGO data
; 0 inlinees with PGO data; 21 single block inlinees; 10 inlinees without PGO data
; Final local variable assignments
;
;  V00 this         [V00,T02] ( 19, 20   )     ref  ->  x19         this class-hnd single-def
;  V01 arg1         [V01,T18] (  3,  3   )     ref  ->  x24         class-hnd single-def
;  V02 arg2         [V02,T19] (  3,  3   )     int  ->  x25         single-def
;  V03 arg3         [V03,T20] (  3,  3   )     int  ->  x26         single-def
;  V04 arg4         [V04,T17] (  5,  4   )     ref  ->  x23         class-hnd single-def
;  V05 arg5         [V05,T12] (  5,  7   )     ref  ->  x20         class-hnd single-def
;  V06 arg6         [V06,T15] (  8,  5   )     ref  ->  x22         class-hnd single-def
;  V07 arg7         [V07,T13] ( 11,  6.50)     ref  ->  x21         class-hnd single-def
;  V08 arg8         [V08,T39] (  2,  1   )    bool  ->  [fp+1D0H]   single-def tier0-frame
;  V09 arg9         [V09    ] (  1,  1   )    bool  ->  [fp+1D0H]   do-not-enreg[X] addr-exposed ld-addr-op single-def tier0-frame

Looks like the issue goes back at least to the Tier0 patchpoint info creation.

--OSR--- Total Frame Size 320, local offset adjust is -320
--OSR-- V00 is at virtual offset -8
--OSR-- V01 is at virtual offset -16
--OSR-- V02 is at virtual offset -20
--OSR-- V03 is at virtual offset -24
--OSR-- V04 is at virtual offset -32
--OSR-- V05 is at virtual offset -40
--OSR-- V06 is at virtual offset -48
--OSR-- V07 is at virtual offset -56
--OSR-- V08 is at virtual offset 0
--OSR-- V09 is at virtual offset 0 (exposed)

However Tier0 codegen gets this right, and it's based on the same information. So not clear yet what is going wrong.

; Tier-0 compilation
...
;  V08 arg8         [V08    ] (  1,  1   )    bool  ->  [fp+140H]   do-not-enreg[]
;  V09 arg9         [V09    ] (  1,  1   )    bool  ->  [fp+141H]   do-not-enreg[]
@dotnet-issue-labeler dotnet-issue-labeler bot added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI untriaged New issue has not been triaged by the area owner labels Apr 19, 2022
@AndyAyersMS AndyAyersMS self-assigned this Apr 19, 2022
@ghost
Copy link

ghost commented Apr 19, 2022

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

Issue Details

Running libraries pgo w/osr "stress", arm64 OSX (trying to verify fix for #68170) I hit a different error.

export COMPlus_TieredCompilation=1
export COMPlus_TC_OnStackReplacement=1
export COMPlus_TC_QuickJitForLoops=1
export COMPlus_TC_OnStackReplacement_InitialCounter=1
export COMPlus_OSR_HitLimit=1

%% dotnet exec --runtimeconfig System.Text.Json.Tests.runtimeconfig.json --depsfile System.Text.Json.Tests.deps.json xunit.console.dll System.Text.Json.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing -parallel none -method System.Text.Json.Nodes.Tests.JsonArrayTests.ConvertJSONArrayToObjectArray

  Discovering: System.Text.Json.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  System.Text.Json.Tests (found 1 of 4368 test case)
  Starting:    System.Text.Json.Tests (parallel test collections = off, max threads = 10)
Process terminated. Assertion failed.
owner and m cannot be set for transparent methods

Note arg8 and arg9 have the same offset in the OSR disasm below (and likewise in the codegen)

; Assembly listing for method System.Reflection.Emit.DynamicMethod:Init(System.String,int,int,System.Type,System.Type[],System.Type,System.Reflection.Module,bool,bool):this
; Emitting BLENDED_CODE for generic ARM64 CPU - MacOS
; Tier-1 compilation
; OSR variant for entry point 0x91
; optimized code
; fp based frame
; partially interruptible
; No PGO data
; 0 inlinees with PGO data; 21 single block inlinees; 10 inlinees without PGO data
; Final local variable assignments
;
;  V00 this         [V00,T02] ( 19, 20   )     ref  ->  x19         this class-hnd single-def
;  V01 arg1         [V01,T18] (  3,  3   )     ref  ->  x24         class-hnd single-def
;  V02 arg2         [V02,T19] (  3,  3   )     int  ->  x25         single-def
;  V03 arg3         [V03,T20] (  3,  3   )     int  ->  x26         single-def
;  V04 arg4         [V04,T17] (  5,  4   )     ref  ->  x23         class-hnd single-def
;  V05 arg5         [V05,T12] (  5,  7   )     ref  ->  x20         class-hnd single-def
;  V06 arg6         [V06,T15] (  8,  5   )     ref  ->  x22         class-hnd single-def
;  V07 arg7         [V07,T13] ( 11,  6.50)     ref  ->  x21         class-hnd single-def
;  V08 arg8         [V08,T39] (  2,  1   )    bool  ->  [fp+1D0H]   single-def tier0-frame
;  V09 arg9         [V09    ] (  1,  1   )    bool  ->  [fp+1D0H]   do-not-enreg[X] addr-exposed ld-addr-op single-def tier0-frame

Looks like the issue goes back at least to the Tier0 patchpoint info creation.

--OSR--- Total Frame Size 320, local offset adjust is -320
--OSR-- V00 is at virtual offset -8
--OSR-- V01 is at virtual offset -16
--OSR-- V02 is at virtual offset -20
--OSR-- V03 is at virtual offset -24
--OSR-- V04 is at virtual offset -32
--OSR-- V05 is at virtual offset -40
--OSR-- V06 is at virtual offset -48
--OSR-- V07 is at virtual offset -56
--OSR-- V08 is at virtual offset 0
--OSR-- V09 is at virtual offset 0 (exposed)

However Tier0 codegen gets this right, and it's based on the same information. So not clear yet what is going wrong.

; Tier-0 compilation
...
;  V08 arg8         [V08    ] (  1,  1   )    bool  ->  [fp+140H]   do-not-enreg[]
;  V09 arg9         [V09    ] (  1,  1   )    bool  ->  [fp+141H]   do-not-enreg[]
Author: AndyAyersMS
Assignees: -
Labels:

area-CodeGen-coreclr, untriaged

Milestone: -

@AndyAyersMS AndyAyersMS removed the untriaged New issue has not been triaged by the area owner label Apr 19, 2022
@AndyAyersMS
Copy link
Member Author

Ah, the issue is that the low bit of the offset in the patchpoint info is used to track exposure info, so any local with and odd offset is going to get the wrong value. That made sense for x64 and most arm64, but on OSX the ABI variances make it possible for args to be at odd offsets.

Will need to revise how this information is stored.

// FP relative offset of this local in the original method
int Offset(unsigned localNum) const
{
return (m_offsetAndExposureData[localNum] & ~EXPOSURE_MASK);
}

enum
{
EXPOSURE_MASK = 0x1
};

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Apr 19, 2022
AndyAyersMS added a commit to AndyAyersMS/runtime that referenced this issue Apr 19, 2022
We can now have Tier0 locals at byte offsets, so rework how the offset
information is incoded in patchpoints to make this possible.

Closes dotnet#68194.
AndyAyersMS added a commit that referenced this issue Apr 19, 2022
We can now have Tier0 locals at byte offsets, so rework how the offset
information is incoded in patchpoints to make this possible.

Closes #68194.
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Apr 19, 2022
directhex pushed a commit to directhex/runtime that referenced this issue Apr 21, 2022
We can now have Tier0 locals at byte offsets, so rework how the offset
information is incoded in patchpoints to make this possible.

Closes dotnet#68194.
@dotnet dotnet locked as resolved and limited conversation to collaborators May 19, 2022
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 a pull request may close this issue.

1 participant