Skip to content

Commit

Permalink
JIT: limit phi-refinement to loop-invariant VNs (#106229)
Browse files Browse the repository at this point in the history
PhiArg VNs should be invariant in the loop that contains the Phi, otherwise
the same VN may refer to values from more than one iteration.

Fixes #105792.
  • Loading branch information
AndyAyersMS committed Aug 12, 2024
1 parent e5b1d02 commit ca4000c
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 8 deletions.
35 changes: 27 additions & 8 deletions src/coreclr/jit/valuenum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11147,6 +11147,7 @@ void Compiler::fgValueNumberPhiDef(GenTreeLclVar* newSsaDef, BasicBlock* blk, bo

GenTreePhi* phiNode = newSsaDef->AsLclVar()->Data()->AsPhi();
ValueNumPair sameVNP;
VNSet loopInvariantCache(getAllocator(CMK_ValueNumber));

for (GenTreePhi::Use& use : phiNode->Uses())
{
Expand All @@ -11167,16 +11168,34 @@ void Compiler::fgValueNumberPhiDef(GenTreeLclVar* newSsaDef, BasicBlock* blk, bo

ValueNumPair phiArgVNP = lvaGetDesc(phiArg)->GetPerSsaData(phiArg->GetSsaNum())->m_vnPair;

#ifdef DEBUG
if (verbose && isUpdate && (phiArgVNP != phiArg->gtVNPair))
if (isUpdate && (phiArgVNP != phiArg->gtVNPair))
{
printf("Updating phi arg [%06u] VN from ", dspTreeID(phiArg));
vnpPrint(phiArg->gtVNPair, 0);
printf(" to ");
vnpPrint(phiArgVNP, 0);
printf("\n");
}
FlowGraphNaturalLoop* const blockLoop = m_loops->GetLoopByHeader(blk);
bool const canUseNewVN = optVNIsLoopInvariant(phiArgVNP.GetConservative(), blockLoop, &loopInvariantCache);

if (canUseNewVN)
{
#ifdef DEBUG
if (verbose)
{
printf("Updating phi arg [%06u] VN from ", dspTreeID(phiArg));
vnpPrint(phiArg->gtVNPair, 0);
printf(" to ");
vnpPrint(phiArgVNP, 0);
printf("\n");
}
#endif
}
else
{
JITDUMP("Can't update phi arg [%06u] with " FMT_VN " -- varies in " FMT_LP "\n", dspTreeID(phiArg),
phiArgVNP.GetConservative(), blockLoop->GetIndex());

// Code below uses phiArgVNP, reset to the old value
//
phiArgVNP = phiArg->gtVNPair;
}
}

phiArg->gtVNPair = phiArgVNP;

Expand Down
47 changes: 47 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_105792/Runtime_105792.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// 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;
using Xunit;

public class Runtime_105792
{
[MethodImpl(MethodImplOptions.NoInlining)]
static int Problem1(int x)
{
int y = 0;
while (x != 0)
{
if (y == x) return -1;
y = x;
Update(ref x);
}
return x;
}

[MethodImpl(MethodImplOptions.NoInlining)]
static int Problem2(int x)
{
int y = 0;
while (x != 0)
{
if (y == x + 1) return -1;
y = x + 1;
Update(ref x);
}
return x;
}

[MethodImpl(MethodImplOptions.NoInlining)]
static void Update(ref int x)
{
x = x - 1;
}

[Fact]
public static int Test1() => Problem1(10) + 100;

[Fact]
public static int Test2() => Problem2(10) + 100;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>

0 comments on commit ca4000c

Please sign in to comment.