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

Fix madeChanges in fgInline #57782

Merged
merged 9 commits into from Aug 26, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 17 additions & 11 deletions src/coreclr/jit/fginline.cpp
Expand Up @@ -185,7 +185,7 @@ PhaseStatus Compiler::fgInline()
// replacement may have enabled optimizations by providing more
// specific types for trees or variables.
fgWalkTree(stmt->GetRootNodePointer(), fgUpdateInlineReturnExpressionPlaceHolder, fgLateDevirtualization,
(void*)this);
(void*)&madeChanges);

// See if stmt is of the form GT_COMMA(call, nop)
// If yes, we can get rid of GT_COMMA.
Expand Down Expand Up @@ -497,8 +497,9 @@ Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTr
return WALK_SKIP_SUBTREES;
}

Compiler* comp = data->compiler;
CORINFO_CLASS_HANDLE retClsHnd = NO_CLASS_HANDLE;
bool* madeChanged = static_cast<bool*>(data->pCallbackData);
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
Compiler* comp = data->compiler;
CORINFO_CLASS_HANDLE retClsHnd = NO_CLASS_HANDLE;

while (tree->OperGet() == GT_RET_EXPR)
{
Expand Down Expand Up @@ -560,6 +561,7 @@ Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTr
}

tree->ReplaceWith(inlineCandidate, comp);
*madeChanged = true;
comp->compCurBB->bbFlags |= (bbFlags & BBF_SPLIT_GAINED);

#ifdef DEBUG
Expand Down Expand Up @@ -604,11 +606,13 @@ Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTr
{
// Either lhs is a call V05 = call(); or lhs is addr, and asg becomes a copyBlk.
comp->fgAttachStructInlineeToAsg(parent, tree, retClsHnd);
*madeChanged = true;
}
else
{
// Just assign the inlinee to a variable to keep it simple.
tree->ReplaceWith(comp->fgAssignStructInlineeToVar(tree, retClsHnd), comp);
*madeChanged = true;
}
}
break;
Expand Down Expand Up @@ -689,9 +693,10 @@ Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTr

Compiler::fgWalkResult Compiler::fgLateDevirtualization(GenTree** pTree, fgWalkData* data)
{
GenTree* tree = *pTree;
GenTree* parent = data->parent;
Compiler* comp = data->compiler;
GenTree* tree = *pTree;
GenTree* parent = data->parent;
Compiler* comp = data->compiler;
bool* madeChanges = static_cast<bool*>(data->pCallbackData);

// In some (rare) cases the parent node of tree will be smashed to a NOP during
// the preorder by fgAttachStructToInlineeArg.
Expand Down Expand Up @@ -731,6 +736,7 @@ Compiler::fgWalkResult Compiler::fgLateDevirtualization(GenTree** pTree, fgWalkD
bool explicitTailCall = (call->AsCall()->gtCallMoreFlags & GTF_CALL_M_EXPLICIT_TAILCALL) != 0;
comp->impDevirtualizeCall(call, nullptr, &method, &methodFlags, &context, nullptr, isLateDevirtualization,
explicitTailCall);
*madeChanges = true;
}
}
else if (tree->OperGet() == GT_ASG)
Expand All @@ -754,6 +760,7 @@ Compiler::fgWalkResult Compiler::fgLateDevirtualization(GenTree** pTree, fgWalkD
if (newClass != NO_CLASS_HANDLE)
{
comp->lvaUpdateClass(lclNum, newClass, isExact);
*madeChanges = true;
}
}
}
Expand All @@ -770,6 +777,7 @@ Compiler::fgWalkResult Compiler::fgLateDevirtualization(GenTree** pTree, fgWalkD
JITDUMP("... removing self-assignment\n");
DISPTREE(tree);
tree->gtBashToNOP();
*madeChanges = true;
}
}
else if (tree->OperGet() == GT_JTRUE)
Expand All @@ -789,20 +797,18 @@ Compiler::fgWalkResult Compiler::fgLateDevirtualization(GenTree** pTree, fgWalkD
comp->gtUpdateNodeSideEffects(tree);
assert((tree->gtFlags & GTF_SIDE_EFFECT) == 0);
tree->gtBashToNOP();
*madeChanges = true;

BasicBlock* bTaken = nullptr;
BasicBlock* bNotTaken = nullptr;

if (condTree->AsIntCon()->gtIconVal != 0)
{
block->bbJumpKind = BBJ_ALWAYS;
bTaken = block->bbJumpDest;
bNotTaken = block->bbNext;
}
else
{
block->bbJumpKind = BBJ_NONE;
bTaken = block->bbNext;
bNotTaken = block->bbJumpDest;
}

Expand All @@ -821,14 +827,14 @@ Compiler::fgWalkResult Compiler::fgLateDevirtualization(GenTree** pTree, fgWalkD
{
const var_types retType = tree->TypeGet();
GenTree* foldedTree = comp->gtFoldExpr(tree);
const var_types newType = foldedTree->TypeGet();

GenTree* putArgType = comp->fgCheckCallArgUpdate(data->parent, foldedTree, retType);
if (putArgType != nullptr)
{
foldedTree = putArgType;
}
*pTree = foldedTree;
*pTree = foldedTree;
*madeChanges = true;
}

return WALK_CONTINUE;
Expand Down
41 changes: 41 additions & 0 deletions src/tests/JIT/opt/Inline/tests/Inline_DetectChanges.cs
@@ -0,0 +1,41 @@
// Generated by Fuzzlyn v1.3 on 2021-08-19 21:40:28
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
// Run on .NET 6.0.0-dev on X64 Windows
// Seed: 17821235008355468541
// Reduced from 60.7 KiB to 0.8 KiB in 00:42:23
// Exits with error:
//
// Assert failure(PID 35672 [0x00008b58], Thread: 10196 [0x27d4]): Assertion failed 'm_compGenTreeID == m_compiler->compGenTreeID' in 'Program:Foo(System.Object)' during 'Morph - Inlining' (IL size 55)
//
// File: D:\dev\dotnet\runtime\src\coreclr\jit\phase.cpp Line: 47
// Image: D:\dev\Fuzzlyn\Fuzzlyn\publish\windows-x64\Fuzzlyn.exe
//
//

using System.Runtime.CompilerServices;

public class Program
{
internal static object s_rt;
internal static ulong[, ] s_3;
internal static byte s_7;
internal static ulong[] s_16;
public static int Main()
{
Foo(null);
return 100;
}

public static void Foo(object o)
{
s_rt = o;
var vr3 = new sbyte[]{0};
var vr4 = s_3[0, 0];
var vr5 = s_3[0, 0];
M16(vr3, 0, ref s_16, 0, vr4, vr5, (uint)(s_7 | 0), 0);
}

[MethodImpl(MethodImplOptions.NoInlining)]
internal static void M16(sbyte[] arg0, short arg1, ref ulong[] arg2, byte arg3, ulong arg4, ulong arg5, uint arg6, uint arg7)
{
}
}
10 changes: 10 additions & 0 deletions src/tests/JIT/opt/Inline/tests/Inline_DetectChanges.csproj
@@ -0,0 +1,10 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<DebugType>None</DebugType>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="Inline_DetectChanges.cs" />
</ItemGroup>
</Project>