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

Random jit stress failures in JIT\Directed\arglist\vararg #81049

Closed
AndyAyersMS opened this issue Jan 23, 2023 · 5 comments · Fixed by #81430
Closed

Random jit stress failures in JIT\Directed\arglist\vararg #81049

AndyAyersMS opened this issue Jan 23, 2023 · 5 comments · Fixed by #81430
Assignees
@jakobbotsch
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 JitStress CLR JIT issues involving JIT internal stress modes
Milestone
8.0.0

Comments

@AndyAyersMS
Copy link
Member

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

Windows x64

set DOTNET_TieredCompilation=0
set DOTNET_JitStress=3cb

  Starting:    JIT.Directed.XUnitWrapper (parallel test collections = on, max threads = 4)
    JIT\Directed\arglist\vararg_TargetWindows\vararg_TargetWindows.cmd [FAIL]

      Failure: check_passing_four_three_double_struct(first, second, third, fourth)
      Failure: TestPassingTwentyFourByteStructs()

      240 Tests run. 238 Passed, 2 Failed.
@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 Jan 23, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jan 23, 2023
@ghost
Copy link

ghost commented Jan 23, 2023

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

Issue Details

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

Windows x64

set DOTNET_TieredCompilation=0
set DOTNET_JitStress=3cb

  Starting:    JIT.Directed.XUnitWrapper (parallel test collections = on, max threads = 4)
    JIT\Directed\arglist\vararg_TargetWindows\vararg_TargetWindows.cmd [FAIL]

      Failure: check_passing_four_three_double_struct(first, second, third, fourth)
      Failure: TestPassingTwentyFourByteStructs()

      240 Tests run. 238 Passed, 2 Failed.
Author: AndyAyersMS
Assignees: -
Labels:

area-CodeGen-coreclr
Milestone: -

@AndyAyersMS AndyAyersMS added JitStress CLR JIT issues involving JIT internal stress modes and removed untriaged New issue has not been triaged by the area owner labels Jan 23, 2023
@AndyAyersMS AndyAyersMS added this to the 8.0.0 milestone Jan 23, 2023
@AndyAyersMS
Copy link
Member Author

Also failed again this week, same two parts of the tests

set DOTNET_TieredCompilation=0
set DOTNET_JitStress=3b1

 ... 
      Failure: check_passing_four_three_double_struct(first, second, third, fourth)
      Failure: TestPassingTwentyFourByteStructsc

@AndyAyersMS AndyAyersMS added the blocking-clean-ci-optional Blocking optional rolling runs label Jan 30, 2023
@runfoapp runfoapp bot mentioned this issue Jan 30, 2023
Closed
@JulieLeeMSFT
Copy link
Member

Assigning to @jakobbotsch for struct related failure.

@jakobbotsch
Copy link
Member

I bisected this to 559aa0e, i.e. the new stress mode I added recently. Investigating in more detail now.

@jakobbotsch
Copy link
Member

After some forward subs, morph and some stress cloning we end up with:

fgMorphTree BB01, STMT00021 (after)
               [000237] -ACXG------                         ▌  ASG       int   
               [000238] D------N---                         ├──▌  LCL_VAR   int    V00 loc0         
               [000239] -ACXG------                         └──▌  CALL      int    NativeVarargTest.VarArg:ReportFailure(bool,System.String,int,int):int
               [000240] -ACXG------ arg0 setup                 ├──▌  ASG       int   
               [000241] D------N---                            │  ├──▌  LCL_VAR   int    V15 tmp9         
               [000242] -ACXG------                            │  └──▌  EQ        int   
               [000243] -----------                            │     ├──▌  ADD       double
               [000244] -----------                            │     │  ├──▌  LCL_VAR   double V05 loc5          (last use)
               [000245] -----------                            │     │  └──▌  ADD       double
               [000246] -----------                            │     │     ├──▌  ADD       double
               [000247] -----------                            │     │     │  ├──▌  LCL_FLD   double(AX) V04 loc4         [+0]
               [000248] -----------                            │     │     │  └──▌  LCL_FLD   double(AX) V04 loc4         [+8]
               [000249] -----------                            │     │     └──▌  LCL_FLD   double(AX) V04 loc4         [+16]
               [000250] -ACXG------                            │     └──▌  CALL      double NativeVarargTest.VarArg:check_passing_four_three_double_struct(NativeVarargTest.ThreeDoubleStruct,NativeVarargTest.ThreeDoubleStruct,NativeVarargTest.ThreeDoubleStruct,NativeVarargTest.ThreeDoubleStruct):double
               [000252] -A--------- arg1 setup                 │        ├──▌  COMMA     void  
               [000253] -A---------                            │        │  ├──▌  COMMA     void  
               [000254] -A---------                            │        │  │  ├──▌  ASG       double
               [000255] U------N---                            │        │  │  │  ├──▌  LCL_FLD   double(AX) V13 tmp7         [+0]
               [000256] -----------                            │        │  │  │  └──▌  LCL_VAR   double V07 tmp1         
               [000257] -A---------                            │        │  │  └──▌  ASG       double
               [000258] U------N---                            │        │  │     ├──▌  LCL_FLD   double(AX) V13 tmp7         [+8]
               [000259] -----------                            │        │  │     └──▌  LCL_VAR   double V08 tmp2         
               [000260] -A---------                            │        │  └──▌  ASG       double
               [000261] U------N---                            │        │     ├──▌  LCL_FLD   double(AX) V13 tmp7         [+16]
               [000262] -----------                            │        │     └──▌  LCL_VAR   double V09 tmp3         
               [000265] -A--------- arg3 setup                 │        ├──▌  COMMA     void  
               [000266] -A---------                            │        │  ├──▌  COMMA     void  
               [000267] -A---------                            │        │  │  ├──▌  ASG       double
               [000268] U------N---                            │        │  │  │  ├──▌  LCL_FLD   double(AX) V14 tmp8         [+0]
               [000269] -----------                            │        │  │  │  └──▌  LCL_VAR   double V10 tmp4         
               [000270] -A---------                            │        │  │  └──▌  ASG       double
               [000271] U------N---                            │        │  │     ├──▌  LCL_FLD   double(AX) V14 tmp8         [+8]
               [000272] -----------                            │        │  │     └──▌  LCL_VAR   double V11 tmp5         
               [000273] -A---------                            │        │  └──▌  ASG       double
               [000274] U------N---                            │        │     ├──▌  LCL_FLD   double(AX) V14 tmp8         [+16]
               [000275] -----------                            │        │     └──▌  LCL_VAR   double V12 tmp6         
     (  3,  3) [000277] ----------- arg4 out+20                │        ├──▌  LCL_VAR_ADDR long   V04 loc4         
               [000263] ----------- arg1 in rdx                │        ├──▌  LCL_VAR_ADDR long   V13 tmp7         
               [000276] ----------- arg3 in r9                 │        ├──▌  LCL_VAR_ADDR long   V14 tmp8         
     (  3,  3) [000264] ----------- arg2 in r8                 │        ├──▌  LCL_VAR_ADDR long   V02 loc2         
               [000251] H---------- va cookie in rcx           │        └──▌  CNS_INT(h) long   0x24af975fed0 vararg
               [000278] ----------- arg0 in rcx                ├──▌  LCL_VAR   int    V15 tmp9         
               [000279] H---------- arg1 in rdx                ├──▌  CNS_INT(h) ref     'check_passing_four_three_double_struct(first, second, third, fo'

               [000280] ----------- arg2 in r8                 ├──▌  CNS_INT   int    100
               [000281] ----------- arg3 in r9                 └──▌  CNS_INT   int    84

This misses some GTF_GLOB_REF markings on the [000247]-[000249] uses. Later fgFindOperOrder comes along and reverses the operands of the EQ node, which causes the problem.

There is a fundamental problem here which is that we don't set GTF_GLOB_REF on accesses of locals until during morph, yet the last-use copy elision is marking an existing local as address exposed during morph. So previous uses will not be marked GTF_GLOB_REF. Which is actually correct, those uses will not be global uses, but only as long as they not reordered with the future call argument.

Need to ponder a bit how to fix it. The temporary simple workaround might be to revisit the current statement and recompute GTF_GLOB_REF flags when we do this optimization, but that doesn't really prevent something downstream from coming along and reordering previous statements/local uses with the call. We might need to mark these as address exposed before morph to handle this (this could maybe be done together with #76926).

jakobbotsch added a commit to jakobbotsch/runtime that referenced this issue Jan 31, 2023
@jakobbotsch
JIT: Update GTF_GLOB_REF after last-use copy omission
89c1f5f 
Morph is responsible for marking accesses of address-exposed locals with
GTF_GLOB_REF, however the last-use copy elision can newly address expose
a preexisting local. This means previous uses that morph already saw
have not had GTF_GLOB_REF added. This updates the JIT to at least add
GTF_GLOB_REF on locals within the same statement as the call. This is
not a complete fix as nothing prevents downstream phases from reordering
the call with uses from earlier statements, but this at least fixes the
one issue we know of right now (which is itself stress-only).

A more complete fix will likely entail moving the identifying of
last-use candidates earlier and address exposing them at that point.
However, that first requires ABI determination to be moved earlier, so
this depends on dotnet#76926, which I expect to get to within the near future.

Fix dotnet#81049
@jakobbotsch jakobbotsch mentioned this issue Jan 31, 2023
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jan 31, 2023
jakobbotsch added a commit that referenced this issue Feb 1, 2023
@jakobbotsch
JIT: Update GTF_GLOB_REF after last-use copy omission (#81430)
58176f5 
Morph is responsible for marking accesses of address-exposed locals with
GTF_GLOB_REF, however the last-use copy elision can newly address expose
a preexisting local. This means previous uses that morph already saw
have not had GTF_GLOB_REF added. This updates the JIT to at least add
GTF_GLOB_REF on locals within the same statement as the call. This is
not a complete fix as nothing prevents downstream phases from reordering
the call with uses from earlier statements, but this at least fixes the
one issue we know of right now (which is itself stress-only).

A more complete fix will likely entail moving the identifying of
last-use candidates earlier and address exposing them at that point.
However, that first requires ABI determination to be moved earlier, so
this depends on #76926, which I expect to get to within the near future.

Fix #81049
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Feb 1, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@jakobbotsch jakobbotsch

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 JitStress CLR JIT issues involving JIT internal stress modes
Projects
None yet
Milestone
8.0.0
Development

Successfully merging a pull request may close this issue.

JIT: Update GTF_GLOB_REF after last-use copy omission jakobbotsch/runtime
3 participants
@jakobbotsch @AndyAyersMS @JulieLeeMSFT