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

Assertion failed 'false' during 'Early Value Propagation' #36588

Closed
BruceForstall opened this issue May 15, 2020 · 3 comments · Fixed by #37335 or #38243
Closed

Assertion failed 'false' during 'Early Value Propagation' #36588

BruceForstall opened this issue May 15, 2020 · 3 comments · Fixed by #37335 or #38243
Assignees
@erozenfeld
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone
5.0.0

Comments

@BruceForstall
Copy link
Member

Failure in libraries System.Data.Common.Tests: release libraries, checked coreclr with:

set COMPlus_TieredCompilation=0
set COMPlus_JitStress=2

Windows x64

E.g.,

build -s libs+libs.tests -runtimeConfiguration checked -c Release -arch x64 /p:ArchiveTests=true

(the /p:ArchiveTests=true is required to build the RunTests.cmd wrapper scripts)

C:\gh\runtime\artifacts\bin\System.Data.Common.Tests\net5.0-Release>call .\RunTests.cmd -r C:\gh\runtime\artifacts\bin\testhost\net5.0-Windows_NT-Release-x64

C:\gh\runtime\artifacts\bin\System.Data.Common.Tests\net5.0-Release>"C:\gh\runtime\artifacts\bin\testhost\net5.0-Windows_NT-Release-x64\dotnet.exe" exec --runtimeconfig System.Data.Common.Tests.runtimeconfig.json --depsfile System.Data.Common.Tests.deps.json xunit.console.dll System.Data.Common.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing
  Discovering: System.Data.Common.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  System.Data.Common.Tests (found 1789 of 1791 test cases)
  Starting:    System.Data.Common.Tests (parallel test collections = on, max threads = 8)
BBF_HAS_IDX_LEN is not set on BB03 but is required because of the following tree
N002 (  3,  3) [000248] ---X--------              *  ARR_LENGTH int
N001 (  1,  1) [000249] ------------              \--*  LCL_VAR   ref    V08 tmp6         u:2 (last use)

Assert failure(PID 39480 [0x00009a38], Thread: 41660 [0xa2bc]): Assertion failed 'false' in '<>c:<Properties>b__6_2():System.Object:this' during 'Early Value Propagation' (IL size 19)

    File: C:\gh\runtime\src\coreclr\src\jit\earlyprop.cpp Line: 189
    Image: C:\gh\runtime\artifacts\bin\testhost\net5.0-Windows_NT-Release-x64\dotnet.exe

@dotnet/jit-contrib
@BruceForstall BruceForstall added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 15, 2020
@BruceForstall BruceForstall added this to the 5.0 milestone May 15, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label May 15, 2020
@erozenfeld
Copy link
Member

I'll investigate.

@erozenfeld erozenfeld self-assigned this May 16, 2020
@BruceForstall
Copy link
Member Author

This failure is in System.Data.Tests.SqlTypes.SqlBinaryTest.Properties()

@BruceForstall BruceForstall removed the untriaged New issue has not been triaged by the area owner label May 17, 2020
@erozenfeld erozenfeld mentioned this issue Jun 3, 2020
Merged
erozenfeld added a commit to erozenfeld/runtime that referenced this issue Jun 4, 2020
@erozenfeld
Changes for basic block flags.
c1f9e25 
Fix the code that propagates flags from the basic block of the return
expression to the caller's basic block during inlining. We are now
properly tracking the basic block of the return expression.
Fixes dotnet#36588.

Don't mark BBJ_RETURN blocks with BBF_BACKWARD_JUMP since they are
executed at most once.

The first change had a few diffs because we propagated BBF_BACKWARD_JUMP
flag. After analyzing them I realized that we shouldn't mark BBJ_RETURN
blocks with BBF_BACKWARD_JUMP in the first place. That resulted in some
diffs because we are less aggressive with inlining of calls outside of
loops.
erozenfeld added a commit that referenced this issue Jun 5, 2020
@erozenfeld
Changes for basic block flags. (#37335)
f326f52 
Fix the code that propagates flags from the basic block of the return
expression to the caller's basic block during inlining. We are now
properly tracking the basic block of the return expression.
Fixes #36588.

Don't mark BBJ_RETURN blocks with BBF_BACKWARD_JUMP since they are
executed at most once.

The first change had a few diffs because we propagated BBF_BACKWARD_JUMP
flag. After analyzing them I realized that we shouldn't mark BBJ_RETURN
blocks with BBF_BACKWARD_JUMP in the first place. That resulted in some
diffs because we are less aggressive with inlining of calls outside of
loops.
@BruceForstall
Copy link
Member Author

This assert still exists in many libraries test runs, e.g.

System.Security.Cryptography.Csp.Tests

set COMPlus_TieredCompilation=0
set COMPlus_JitStress=1

BBF_HAS_IDX_LEN is not set on BB07 but is required because of the following tree 
N006 (  5,  4) [000300] ---X--------              *  ARR_LENGTH int   
N005 (  3,  2) [000301] ------------              \--*  LCL_VAR   ref    V01 loc1         u:2

Assert failure(PID 5380 [0x00001504], Thread: 5968 [0x1750]): Assertion failed 'false' in 'System.Security.Cryptography.RNG.Tests.RNGCryptoServiceProviderTests:GetNonZeroBytes_Array()' during 'Early Value Propagation' (IL size 99)

    File: F:\workspace\_work\1\s\src\coreclr\src\jit\earlyprop.cpp Line: 189
    Image: C:\h\w\C1AA0A16\p\dotnet.exe

https://dev.azure.com/dnceng/public/_build/results?buildId=691028&view=ms.vss-test-web.build-test-results-tab&runId=21453406&paneView=dotnet-dnceng.dnceng-build-release-tasks.helix-test-information-tab&resultId=177144

There are a few other tests which hit this, and it appears to be most, if not all platform.

@BruceForstall BruceForstall reopened this Jun 17, 2020
erozenfeld added a commit to erozenfeld/runtime that referenced this issue Jun 22, 2020
@erozenfeld
Propagate basic block flags after a failed inline.
cbabbf9 
This is a fllow-up to dotnet#37335 and dotnet#37840.

When an inline fails we replace `GT_RET_EXPR`with the
original `GT_CALL` node. `GT_RET_EXPR`may end up in
a basic block other than the original `GT_CALL` so we need
to propagate basic block flags.

Fixes dotnet#36588.
@erozenfeld erozenfeld mentioned this issue Jun 22, 2020
Merged
erozenfeld added a commit that referenced this issue Jun 23, 2020
@erozenfeld
Propagate basic block flags after a failed inline. (#38243)
81ed1f7 
This is a fllow-up to #37335 and #37840.

When an inline fails we replace `GT_RET_EXPR`with the
original `GT_CALL` node. `GT_RET_EXPR`may end up in
a basic block other than the original `GT_CALL` so we need
to propagate basic block flags.

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

@erozenfeld erozenfeld

Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Milestone
5.0.0
3 participants
@erozenfeld @BruceForstall @Dotnet-GitSync-Bot