JIT: Enable phi-based jump threading when SSA updates aren't needed #77748
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Leverage the new SSA accounting to look for cases of phi-based jump threading that will not require SSA updates. In particular cases where the phis are all locally consumed. Also update documentation on the SSA checker implemenatation (which aims to ensure that the SSA accounting we're relying on here is accurate).
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsLeverage the new SSA accounting to look for cases of phi-based jump threading that will not require SSA updates. In particular cases where the phis are all locally consumed. Also update documentation on the SSA checker implemenatation (which aims to ensure that the SSA accounting we're relying on here is accurate).
|
|
@BruceForstall PTAL This should get a lot of the cases we were getting before. Have verified it avoids the known troublemakers from #76636 and #76507, eg in the latter: |
| // is soundly approximated by the data stored in the LclSsaVarDsc entries. | ||
| // | ||
| // In particular the properties claimed for an SSA lifetime via its | ||
| // LclSsaVarDsc must be accurate or an over-estimate. We tolerate over-estimates |
There was a problem hiding this comment.
must be accurate or an over-estimate
Seems a good idea to mention the implicit uses quirk for promoted fields here.
|
There are also sizeable diffs in ASP.NET, unfortunately collections are lagging all these GUID updates. Locally: Improvements/regressions per collection
|
|
/azp run fuzzlyn |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@jakobbotsch guessing there may be real fuzzlyn failures but something goes wrong trying to surface them? |
It looks like something in JIT is printing a node to stdout without checking |
|
Reduced example: // Generated by Fuzzlyn v1.5 on 2022-11-02 09:45:09
// Run on X64 Windows
// Seed: 14352496230403376157
// Reduced from 191.3 KiB to 0.5 KiB in 00:19:24
// Exits with error:
// Malformed result
public struct S0
{
public int F1;
}
public struct S2
{
public uint F0;
public ushort F1;
}
public class Program
{
public static S2 s_1;
public static S2 s_16;
public static sbyte[][] s_20;
public static int[] s_21;
public static void Main()
{
var vr9 = new S0();
s_1.F0 <<= M41(vr9, ref s_16.F1);
}
public static sbyte M41(S0 argThis, ref ushort arg1)
{
argThis.F1 = s_21[0];
return s_20[0][0];
}
}Output with [000083] ----------- IL_OFFSET void INLRT @ 0x000[E-]
N001 ( 1, 1) [000047] ----------- t47 = CNS_INT int 0 $80
┌──▌ t47 int
N003 ( 5, 4) [000048] DA--------- ▌ STORE_LCL_VAR int V04 tmp3 d:2
[000084] ----------- IL_OFFSET void INLRT @ 0x008[E-]
N001 ( 2, 10) [000009] ----------- t9 = CNS_INT long 0x7ff9a30af770 $100
N002 ( 1, 1) [000010] ----------- t10 = CNS_INT int 3 $82
┌──▌ t9 long arg0 in rcx
├──▌ t10 int arg1 in rdx
N003 ( 17, 18) [000011] H-CXG------ t11 = ▌ CALL help long HELPER.CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE $1c1
┌──▌ t11 long
N005 ( 17, 18) [000077] DA-XG------ ▌ STORE_LCL_VAR long V08 cse0 d:1
N008 ( 3, 10) [000004] I---------- t4 = CNS_INT(h) long 0x1432dc01ec0 static box ptr $200
┌──▌ t4 long
N009 ( 5, 12) [000005] #---------- t5 = ▌ IND ref $240
N010 ( 1, 1) [000006] ----------- t6 = CNS_INT long 8 Fseq[s_1] $102
┌──▌ t5 ref
├──▌ t6 long
N011 ( 7, 14) [000007] ----------- t7 = ▌ ADD byref $280
┌──▌ t7 byref
N014 ( 25, 33) [000050] DA-XG------ ▌ STORE_LCL_VAR byref V05 tmp4 d:2
N015 ( 1, 1) [000051] ----------- t51 = LCL_VAR byref V05 tmp4 u:2 $280
┌──▌ t51 byref
N016 ( 2, 2) [000052] -----O----- ▌ NULLCHECK byte $VN.Void
N001 ( 1, 1) [000018] ----------- t18 = LCL_VAR byref V05 tmp4 u:2 $280
┌──▌ t18 byref
N002 ( 3, 2) [000019] n---GO----- t19 = ▌ IND int <l:$340, c:$380>
┌──▌ t19 int
N004 ( 3, 3) [000036] DA--GO----- ▌ STORE_LCL_VAR int V03 tmp2 d:2
read: N001 ( 1, 1) [000017] ----------- t17 = LCL_VAR byref V05 tmp4 u:2 (last use) $280
N003 ( 1, 1) [000081] ----------- t81 = CNS_INT int 0 $80
┌──▌ t81 int
N005 ( 1, 3) [000073] DA--------- ▌ STORE_LCL_VAR int V07 tmp6 d:2
N006 ( 3, 10) [000021] I---------- t21 = CNS_INT(h) long 0x1432dc01ec8 static box ptr $204
┌──▌ t21 long
N007 ( 5, 12) [000022] #---------- t22 = ▌ IND ref $241
N008 ( 1, 1) [000023] ----------- t23 = CNS_INT long 8 Fseq[s_16] $102
┌──▌ t22 ref
├──▌ t23 long
N009 ( 7, 14) [000024] ----------- t24 = ▌ ADD byref $281
┌──▌ t24 byref
write: N011 ( 7, 14) [000059] DA--G------ ▌ STORE_LCL_VAR byref V05 tmp4 d:3
N012 ( 1, 1) [000063] ----------- t63 = LCL_VAR byref V05 tmp4 u:3 (last use) $281
N013 ( 1, 1) [000064] ----------- t64 = CNS_INT long 4 $105
┌──▌ t63 byref
├──▌ t64 long
N014 ( 3, 3) [000065] ----------- t65 = ▌ ADD byref $282
┌──▌ t65 byref
N017 ( 10, 17) [000070] DA--GO----- ▌ STORE_LCL_VAR byref V06 tmp5 d:2
N018 ( 1, 1) [000071] ----------- t71 = LCL_VAR byref V06 tmp5 u:2 (last use) $282
N019 ( 1, 1) [000082] ----------- t82 = CNS_INT int 0 $80
┌──▌ t71 byref arg1 in rdx
├──▌ t82 int arg0 in rcx
N020 ( 33, 29) [000032] --CXGO----- t32 = ▌ CALL int Program.M41 $381
N021 ( 1, 1) [000039] ----------- t39 = CNS_INT int 31 $83
┌──▌ t32 int
├──▌ t39 int
N022 ( 35, 31) [000040] ---XGO----- t40 = ▌ AND int $341
N023 ( 1, 1) [000037] ----------- t37 = LCL_VAR int V03 tmp2 u:2 (last use) <l:$340, c:$380>
┌──▌ t37 int
├──▌ t40 int
N024 ( 40, 33) [000041] ---XGO----- t41 = ▌ LSH int <l:$342, c:$343>
┌──▌ t17 byref
├──▌ t41 int
user: [000085] -ACXGO----- ▌ STOREIND int
[000086] ----------- IL_OFFSET void INLRT @ 0x029[E-]
N001 ( 0, 0) [000044] ----------- RETURN void $VN.Void
Assert failure(PID 33568 [0x00008320], Thread: 26964 [0x6954]): Assertion failed '!"Write to unaliased local overlaps outstanding read"' in 'Program:Main()' during 'Rationalize IR' (IL size 42; hash 0xcb019401; FullOpts)
File: C:\dev\dotnet\runtime3\src\coreclr\jit\lir.cpp Line: 1407
Image: C:\dev\dotnet\runtime3\artifacts\tests\coreclr\windows.x64.Checked\Tests\Core_Root\corerun.exeI will try to make Fuzzlyn a bit more robust, we should be able to print information like this before asserts in checked builds. |
|
Looks introduced/exposed by #77641, cc @SingleAccretion (edit: opened #77773 for it) |
|
jakobbotsch/Fuzzlyn@99360ba should make Fuzzlyn handle this more cleanly. |
|
/azp run Fuzzlyn |
|
Azure Pipelines successfully started running 1 pipeline(s). |
BruceForstall
approved these changes
Nov 2, 2022
|
@BruceForstall just to be sure: the added comments on SSA checking were helpful enough? |
Yeah, those were great. Thanks. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Leverage the new SSA accounting to look for cases of phi-based jump threading that will not require SSA updates. In particular cases where the phis are all locally consumed.
Also update documentation on the SSA checker implemenatation (which aims to ensure that the SSA accounting we're relying on here is accurate).