Skip to content

Commit

Permalink
JIT: more fixes for VN loop dependence tracking (#56184)
Browse files Browse the repository at this point in the history 
Specify `Overwrite` when setting loop dependence map entries, as we may
refine the initial result.

Fixes #56174.

Extract loop dependence of `VNF_PhiMemoryDef`.

Fixes new case noted in #55936, and 13/16 or so other cases Jakob sent
me privately. Also update a comment and fix tests to work better with
jitstress per other notes on that PR.
  • Loading branch information
@AndyAyersMS
AndyAyersMS authored Jul 27, 2021
1 parent eaaf43f commit b18ff29
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 46 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/jit/optimizer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6117,7 +6117,7 @@ void Compiler::optRecordLoopMemoryDependence(GenTree* tree, BasicBlock* block, V
// we know of. Update the map.
//
JITDUMP(" ==> Updating loop memory dependence of [%06u] to " FMT_LP "\n", dspTreeID(tree), updateLoopNum);
map->Set(tree, optLoopTable[updateLoopNum].lpEntry);
map->Set(tree, optLoopTable[updateLoopNum].lpEntry, NodeToLoopMemoryBlockMap::Overwrite);
}

//------------------------------------------------------------------------
Expand Down
12 changes: 9 additions & 3 deletions src/coreclr/jit/valuenum.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2125,7 +2125,7 @@ ValueNum ValueNumStore::VNForFunc(var_types typ, VNFunc func, ValueNum arg0VN, V
//
// Return Value: - Returns the ValueNum associated with 'func'('arg0VN','arg1VN','arg2VN','arg3VN')
//
// Note: Currently the only four operand func is the VNF_PtrToArrElem operation
// Note: Currently the only four operand funcs are VNF_PtrToArrElem and VNF_MapStore.
//
ValueNum ValueNumStore::VNForFunc(
var_types typ, VNFunc func, ValueNum arg0VN, ValueNum arg1VN, ValueNum arg2VN, ValueNum arg3VN)
Expand Down Expand Up @@ -4354,8 +4354,9 @@ var_types ValueNumStore::TypeOfVN(ValueNum vn)
}

//------------------------------------------------------------------------
// LoopOfVN: If the given value number is VNF_MemOpaque or VNF_MapStore, return
// the loop number where the memory update occurs, otherwise returns MAX_LOOP_NUM.
// LoopOfVN: If the given value number is VNF_MemOpaque, VNF_MapStore, or
// VNF_MemoryPhiDef, return the loop number where the memory update occurs,
// otherwise returns MAX_LOOP_NUM.
//
// Arguments:
// vn - Value number to query
Expand All @@ -4377,6 +4378,11 @@ BasicBlock::loopNumber ValueNumStore::LoopOfVN(ValueNum vn)
{
return (BasicBlock::loopNumber)funcApp.m_args[3];
}
else if (funcApp.m_func == VNF_PhiMemoryDef)
{
BasicBlock* const block = reinterpret_cast<BasicBlock*>(ConstantValue<ssize_t>(funcApp.m_args[0]));
return block->bbNatLoopNum;
}
}

return BasicBlock::MAX_LOOP_NUM;
Expand Down
84 changes: 42 additions & 42 deletions src/tests/JIT/Regression/JitBlue/Runtime_54118/Runtime_54118.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,29 +29,29 @@ void Test(string name, Func<bool> act)
index++;
}

Test(nameof(TestConstantByref), TestConstantByref);
Test(nameof(TestConstantArr), TestConstantArr);
Test(nameof(TestConstantClsVar), TestConstantClsVar);
Test(nameof(TestParamByref), () => TestParamByref(1));
Test(nameof(TestParamArr), () => TestParamArr(1));
Test(nameof(TestParamClsVar), () => TestParamClsVar(1));
Test(nameof(TestPhiByref), TestPhiByref);
Test(nameof(TestPhiArr), TestPhiArr);
Test(nameof(TestPhiClsVar), TestPhiClsVar);
Test(nameof(TestCastByref), TestCastByref);
Test(nameof(TestCastArr), TestCastArr);
Test(nameof(TestCastClsVar), TestCastClsVar);
Test(nameof(TestConstantByref), () => TestConstantByref(2));
Test(nameof(TestConstantArr), () => TestConstantArr(2));
Test(nameof(TestConstantClsVar), () => TestConstantClsVar(2));
Test(nameof(TestParamByref), () => TestParamByref(1, 2));
Test(nameof(TestParamArr), () => TestParamArr(1, 2));
Test(nameof(TestParamClsVar), () => TestParamClsVar(1, 2));
Test(nameof(TestPhiByref), () => TestPhiByref(2));
Test(nameof(TestPhiArr), () => TestPhiArr(2));
Test(nameof(TestPhiClsVar), () => TestPhiClsVar(2));
Test(nameof(TestCastByref), () => TestCastByref(2));
Test(nameof(TestCastArr), () => TestCastArr(2));
Test(nameof(TestCastClsVar), () => TestCastClsVar(2));

return 100 + result;
}

[MethodImpl(MethodImplOptions.NoInlining)]
static bool TestConstantByref()
static bool TestConstantByref(int k)
{
int[] arr = { -1 };
ref int r = ref arr[0];
int val = -1;
for (int i = 0; i < 2; i++)
for (int i = 0; i < k; i++)
{
r = 1;
val = r;
Expand All @@ -61,11 +61,11 @@ static bool TestConstantByref()
}

[MethodImpl(MethodImplOptions.NoInlining)]
static bool TestConstantArr()
static bool TestConstantArr(int k)
{
int[] arr = { -1 };
int val = -1;
for (int i = 0; i < 2; i++)
for (int i = 0; i < k; i++)
{
arr[0] = 1;
val = arr[0];
Expand All @@ -77,11 +77,11 @@ static bool TestConstantArr()
static int _clsVar;

[MethodImpl(MethodImplOptions.NoInlining)]
static bool TestConstantClsVar()
static bool TestConstantClsVar(int k)
{
_clsVar = -1;
int val = -1;
for (int i = 0; i < 2; i++)
for (int i = 0; i < k; i++)
{
_clsVar = 1;
val = _clsVar;
Expand All @@ -91,12 +91,12 @@ static bool TestConstantClsVar()
}

[MethodImpl(MethodImplOptions.NoInlining)]
static bool TestParamByref(int one)
static bool TestParamByref(int one, int k)
{
int[] arr = { -1 };
ref int r = ref arr[0];
int val = -1;
for (int i = 0; i < 2; i++)
for (int i = 0; i < k; i++)
{
r = one;
val = r;
Expand All @@ -106,11 +106,11 @@ static bool TestParamByref(int one)
}

[MethodImpl(MethodImplOptions.NoInlining)]
static bool TestParamArr(int one)
static bool TestParamArr(int one, int k)
{
int[] arr = { -1 };
int val = -1;
for (int i = 0; i < 2; i++)
for (int i = 0; i < k; i++)
{
arr[0] = one;
val = arr[0];
Expand All @@ -120,11 +120,11 @@ static bool TestParamArr(int one)
}

[MethodImpl(MethodImplOptions.NoInlining)]
static bool TestParamClsVar(int one)
static bool TestParamClsVar(int one, int k)
{
_clsVar = -1;
int val = -1;
for (int i = 0; i < 2; i++)
for (int i = 0; i < k; i++)
{
_clsVar = one;
val = _clsVar;
Expand All @@ -134,14 +134,14 @@ static bool TestParamClsVar(int one)
}

[MethodImpl(MethodImplOptions.NoInlining)]
static bool TestPhiByref()
static bool TestPhiByref(int k)
{
int[] arr = { -1 };
ref int r = ref arr[0];
int val = -1;
for (int i = 0; i < 2; i++)
for (int i = 0; i < k; i++)
{
for (int j = 0; j < 2; j++)
for (int j = 0; j < k; j++)
{
r = i;
val = r;
Expand All @@ -152,13 +152,13 @@ static bool TestPhiByref()
}

[MethodImpl(MethodImplOptions.NoInlining)]
static bool TestPhiArr()
static bool TestPhiArr(int k)
{
int[] arr = { -1 };
int val = -1;
for (int i = 0; i < 2; i++)
for (int i = 0; i < k; i++)
{
for (int j = 0; j < 2; j++)
for (int j = 0; j < k; j++)
{
arr[0] = i;
val = arr[0];
Expand All @@ -169,13 +169,13 @@ static bool TestPhiArr()
}

[MethodImpl(MethodImplOptions.NoInlining)]
static bool TestPhiClsVar()
static bool TestPhiClsVar(int k)
{
_clsVar = -1;
int val = -1;
for (int i = 0; i < 2; i++)
for (int i = 0; i < k; i++)
{
for (int j = 0; j < 2; j++)
for (int j = 0; j < k; j++)
{
_clsVar = i;
val = _clsVar;
Expand All @@ -186,14 +186,14 @@ static bool TestPhiClsVar()
}

[MethodImpl(MethodImplOptions.NoInlining)]
static bool TestCastByref()
static bool TestCastByref(int k)
{
int[] arr = { -1 };
ref int r = ref arr[0];
int val = -1;
for (int i = 0; i < 2; i++)
for (int i = 0; i < k; i++)
{
for (int j = 0; j < 2; j++)
for (int j = 0; j < k; j++)
{
r = i;
val = (byte)r;
Expand All @@ -204,13 +204,13 @@ static bool TestCastByref()
}

[MethodImpl(MethodImplOptions.NoInlining)]
static bool TestCastArr()
static bool TestCastArr(int k)
{
int[] arr = { -1 };
int val = -1;
for (int i = 0; i < 2; i++)
for (int i = 0; i < k; i++)
{
for (int j = 0; j < 2; j++)
for (int j = 0; j < k; j++)
{
arr[0] = i;
val = (byte)arr[0];
Expand All @@ -221,13 +221,13 @@ static bool TestCastArr()
}

[MethodImpl(MethodImplOptions.NoInlining)]
static bool TestCastClsVar()
static bool TestCastClsVar(int k)
{
_clsVar = -1;
int val = -1;
for (int i = 0; i < 2; i++)
for (int i = 0; i < k; i++)
{
for (int j = 0; j < 2; j++)
for (int j = 0; j < k; j++)
{
_clsVar = i;
val = (byte)_clsVar;
Expand Down
46 changes: 46 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_54118/Runtime_54118_1.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Runtime.CompilerServices;

// Generated by Fuzzlyn v1.2 on 2021-07-22 13:37:14
// Seed: 14815563263006255362
// Reduced from 12.9 KiB to 0.4 KiB in 00:00:17
// Debug: Outputs 1
// Release: Outputs 0

// VN must properly report loop dependence of a PhiMemoryDef
// or the jit will hoist the load of vr7[0] past the store.

public class Runtime_54118
{
static short[] s_2;

[MethodImpl(MethodImplOptions.NoInlining)]
static bool Eval(byte b) => b == 100;

[MethodImpl(MethodImplOptions.NoInlining)]
static int Bound() => 2;

public static int Main()
{
byte[] vr7 = new byte[]{0};
bool vr11 = default(bool);
bool ok = false;
int k = Bound();
for (int vr9 = 0; vr9 < k; vr9++)
{
if (vr11)
{
s_2[0] = 0;
}

vr7[0] = 100;
byte vr10 = vr7[0];
ok = Eval(vr10);
}

return ok ? 100 : -1;
}
}
12 changes: 12 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_54118/Runtime_54118_1.csproj
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
</PropertyGroup>
<PropertyGroup>
<DebugType>None</DebugType>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>

0 comments on commit b18ff29

Please sign in to comment.