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

[TEST] Test clearing VN after optComputeLoopSideEffects() #82328

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/coreclr/jit/optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4304,17 +4304,19 @@ PhaseStatus Compiler::optUnrollLoops()
// Heuristic: Estimated cost in code size of the unrolled loop.

{
ClrSafeInt<unsigned> loopCostSz; // Cost is size of one iteration
auto tryIndex = loop.lpTop->bbTryIndex;
ClrSafeInt<unsigned> loopCostSz; // Cost is size of one iteration
const BasicBlock* const top = loop.lpTop;

// Besides calculating the loop cost, also ensure that all loop blocks are within the same EH
// region, and count the number of BBJ_RETURN blocks in the loop.
loopRetCount = 0;
for (BasicBlock* const block : loop.LoopBlocks())
{
if (block->bbTryIndex != tryIndex)
if (!BasicBlock::sameEHRegion(block, top))
{
// Unrolling would require cloning EH regions
// Note that only non-funclet model (x86) could actually have a loop including a handler
// but not it's corresponding `try`, if its `try` was moved due to being marked "rare".
JITDUMP("Failed to unroll loop " FMT_LP ": EH constraint\n", lnum);
goto DONE_LOOP;
}
Expand Down Expand Up @@ -8799,6 +8801,11 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk)
}
}
}

// If there is a tree node, clear its ValueNumber. This is to ensure that for those blocks that
// are unreachable, which we still handle in this loop but not during Value Numbering,
// USE THIS:
// stmt->GetRootNode()->gtVNPair.SetBoth(ValueNumStore::NoVN);
}

if (memoryHavoc != emptyMemoryKindSet)
Expand Down
16 changes: 16 additions & 0 deletions src/coreclr/jit/valuenum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8086,6 +8086,22 @@ PhaseStatus Compiler::fgValueNumber()
// Compute the side effects of loops.
optComputeLoopSideEffects();

// The implementation of optComputeLoopSideEffects() can set some value numbers. These should not be used,
// and in fact, the implementation should probably clear them or use a side table or other implementation
// to avoid touching the value number field, as for reachable blocks they will get overridden below.
{
for (BasicBlock* const blk : Blocks())
{
for (Statement* const stmt : blk->NonPhiStatements())
{
for (GenTree* const tree : stmt->TreeList())
{
tree->gtVNPair.SetBoth(ValueNumStore::NoVN);
}
}
}
}

// At the block level, we will use a modified worklist algorithm. We will have two
// "todo" sets of unvisited blocks. Blocks (other than the entry block) are put in a
// todo set only when some predecessor has been visited, so all blocks have at least one
Expand Down
68 changes: 68 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_81675/Runtime_81675.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

// Generated by Fuzzlyn v1.5 on 2023-02-06 00:41:34
// Run on X86 Windows
// Seed: 12611629827253727687
// Reduced from 444.9 KiB to 1.1 KiB in 00:13:23
// Hits JIT assert in Release:
// Assertion failed '!"Jump into the middle of handler region"' in 'Program:Main(Fuzzlyn.ExecutionServer.IRuntime)' during 'Unroll loops' (IL size 131; hash 0xade6b36b; FullOpts)
//
// File: D:\a\_work\1\s\src\coreclr\jit\fgdiagnostic.cpp Line: 2609
//
public interface I0
{
}

public class C0 : I0
{
public sbyte F3;
}

public class Runtime_81675
{
public static ushort s_11;
public static bool[][] s_18;
public static C0 s_26;
public static short[] s_42;
public static void Main()
{
short vr7 = default(short);
for (int vr8 = 0; vr8 < 0; vr8++)
{
try
{
System.Console.WriteLine(32767);
}
finally
{
I0 vr9 = new C0();
}
}

for (int vr10 = 0; vr10 < 0; vr10++)
{
if (s_18[0][0])
{
var vr11 = s_26.F3;
}
else
{
System.Console.WriteLine(0);
}

if (!((ushort)(-s_11++) >= 0))
{
vr7 = vr7;
try
{
vr7 = 0;
}
finally
{
vr7 = s_42[0];
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>