Skip to content
Merged
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
2 changes: 1 addition & 1 deletion src/coreclr/src/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -6512,7 +6512,7 @@ class Compiler
void optCopyProp(BasicBlock* block, Statement* stmt, GenTree* tree, LclNumToGenTreePtrStack* curSsaName);
void optBlockCopyPropPopStacks(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName);
void optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName);
bool optIsSsaLocal(GenTree* tree);
unsigned optIsSsaLocal(GenTree* tree);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably could have been renamed from an "Is" to "Get", but this is fine for the 5.0 port

int optCopyProp_LclVarScore(LclVarDsc* lclVarDsc, LclVarDsc* copyVarDsc, bool preferOp2);
void optVnCopyProp();
INDEBUG(void optDumpCopyPropStack(LclNumToGenTreePtrStack* curSsaName));
Expand Down
71 changes: 54 additions & 17 deletions src/coreclr/src/jit/copyprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ void Compiler::optBlockCopyPropPopStacks(BasicBlock* block, LclNumToGenTreePtrSt
{
continue;
}
unsigned lclNum = tree->AsLclVarCommon()->GetLclNum();
if (!lvaInSsa(lclNum))
const unsigned lclNum = optIsSsaLocal(tree);
if (lclNum == BAD_VAR_NUM)
{
continue;
}
Expand All @@ -61,8 +61,19 @@ void Compiler::optDumpCopyPropStack(LclNumToGenTreePtrStack* curSsaName)
JITDUMP("{ ");
for (LclNumToGenTreePtrStack::KeyIterator iter = curSsaName->Begin(); !iter.Equal(curSsaName->End()); ++iter)
{
GenTree* node = iter.GetValue()->Top();
JITDUMP("%d-[%06d]:V%02u ", iter.Get(), dspTreeID(node), node->AsLclVarCommon()->GetLclNum());
GenTreeLclVarCommon* lclVar = iter.GetValue()->Top()->AsLclVarCommon();
unsigned ssaLclNum = optIsSsaLocal(lclVar);
assert(ssaLclNum != BAD_VAR_NUM);

if (ssaLclNum == lclVar->GetLclNum())
{
JITDUMP("%d-[%06d]:V%02u ", iter.Get(), dspTreeID(lclVar), ssaLclNum);
}
else
{
// A promoted field was asigned using the parent struct, print `ssa field lclNum(parent lclNum)`.
JITDUMP("%d-[%06d]:V%02u(V%02u) ", iter.Get(), dspTreeID(lclVar), ssaLclNum, lclVar->GetLclNum());
}
}
JITDUMP("}\n\n");
}
Expand Down Expand Up @@ -150,10 +161,10 @@ void Compiler::optCopyProp(BasicBlock* block, Statement* stmt, GenTree* tree, Lc
{
return;
}
unsigned lclNum = tree->AsLclVarCommon()->GetLclNum();
const unsigned lclNum = optIsSsaLocal(tree);

// Skip non-SSA variables.
if (!lvaInSsa(lclNum))
if (lclNum == BAD_VAR_NUM)
{
return;
}
Expand Down Expand Up @@ -291,13 +302,39 @@ void Compiler::optCopyProp(BasicBlock* block, Statement* stmt, GenTree* tree, Lc
return;
}

/**************************************************************************************
*
* Helper to check if tree is a local that participates in SSA numbering.
*/
bool Compiler::optIsSsaLocal(GenTree* tree)
//------------------------------------------------------------------------------
// optIsSsaLocal : helper to check if the tree is a local that participates in SSA numbering.
//
// Arguments:
// tree - The tree to perform the check on;
//
// Returns:
// - lclNum if the local is participating in SSA;
// - fieldLclNum if the parent local can be replaced by its only field;
// - BAD_VAR_NUM otherwise.
//
unsigned Compiler::optIsSsaLocal(GenTree* tree)
{
return tree->IsLocal() && lvaInSsa(tree->AsLclVarCommon()->GetLclNum());
if (!tree->IsLocal())
{
return BAD_VAR_NUM;
}

GenTreeLclVarCommon* lclNode = tree->AsLclVarCommon();
unsigned lclNum = lclNode->GetLclNum();
LclVarDsc* varDsc = lvaGetDesc(lclNum);

if (!lvaInSsa(lclNum) && varDsc->CanBeReplacedWithItsField(this))
{
lclNum = varDsc->lvFieldLclStart;
}

if (!lvaInSsa(lclNum))
{
return BAD_VAR_NUM;
}

return lclNum;
}

//------------------------------------------------------------------------------
Expand Down Expand Up @@ -351,22 +388,22 @@ void Compiler::optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curS
// embedded update. Killing the variable is a simplification to produce 0 ASM diffs
// for an update release.
//
if (optIsSsaLocal(tree) && (tree->gtFlags & GTF_VAR_DEF))
const unsigned lclNum = optIsSsaLocal(tree);
if ((lclNum != BAD_VAR_NUM) && (tree->gtFlags & GTF_VAR_DEF))
{
VarSetOps::AddElemD(this, optCopyPropKillSet, lvaTable[tree->AsLclVarCommon()->GetLclNum()].lvVarIndex);
VarSetOps::AddElemD(this, optCopyPropKillSet, lvaTable[lclNum].lvVarIndex);
}
}

// This logic must be in sync with SSA renaming process.
for (GenTree* tree = stmt->GetTreeList(); tree != nullptr; tree = tree->gtNext)
{
if (!optIsSsaLocal(tree))
const unsigned lclNum = optIsSsaLocal(tree);
if (lclNum == BAD_VAR_NUM)
{
continue;
}

unsigned lclNum = tree->AsLclVarCommon()->GetLclNum();

// As we encounter a definition add it to the stack as a live definition.
if (tree->gtFlags & GTF_VAR_DEF)
{
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/src/jit/valuenum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6789,6 +6789,10 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree)
{
fgMutateAddressExposedLocal(tree DEBUGARG("COPYBLK - address-exposed local"));
}
else
{
JITDUMP("LHS V%02u not in ssa at [%06u], so no VN assigned\n", lhsLclNum, dspTreeID(lclVarTree));
}
}
else
{
Expand Down
67 changes: 67 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_41100/Runtime_41100.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

// The test was showing a wrong copy propogation when a struct field was rewritten by
// a call assignment to the parent struct but that assignment was not supported by copyprop.

using System;
using System.Collections.Immutable;
using System.Runtime.CompilerServices;

class X
{
[MethodImpl(MethodImplOptions.NoInlining)]
public static void E(ImmutableArray<string> a) {}

[MethodImpl(MethodImplOptions.NoInlining)]
public static ImmutableArray<string> G() => ImmutableArray<string>.Empty;

[MethodImpl(MethodImplOptions.NoInlining)]
public static ImmutableArray<string> H()
{
string[] a = new string[100];

for (int i = 0; i < a.Length; i++)
{
a[i] = "hello";
}

return ImmutableArray.Create<string>(a);
}

[MethodImpl(MethodImplOptions.NoInlining)]
public static int F()
{
var a = H();
int r = 0;

foreach (var s in a)
{
if (s.Equals("hello")) r++;
}

var aa = a;

if (r > 0)

{
foreach (var s in a)
{
if (s.Equals("hello")) r--;
}

aa = G();

foreach (var s in a)
{
if (s.Equals("hello")) r++;
}
}

E(aa);

return r;
}

public static int Main() => F();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
</PropertyGroup>
<PropertyGroup>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>