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 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
27 changes: 16 additions & 11 deletions src/coreclr/jit/fginline.cpp
Original file line number Diff line number Diff line change
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* madeChanges = static_cast<bool*>(data->pCallbackData);
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);
*madeChanges = true;
comp->compCurBB->bbFlags |= (bbFlags & BBF_SPLIT_GAINED);

#ifdef DEBUG
Expand Down Expand Up @@ -610,6 +612,7 @@ Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTr
// Just assign the inlinee to a variable to keep it simple.
tree->ReplaceWith(comp->fgAssignStructInlineeToVar(tree, retClsHnd), comp);
}
*madeChanges = true;
}
break;

Expand Down Expand Up @@ -689,9 +692,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 +735,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 +759,7 @@ Compiler::fgWalkResult Compiler::fgLateDevirtualization(GenTree** pTree, fgWalkD
if (newClass != NO_CLASS_HANDLE)
{
comp->lvaUpdateClass(lclNum, newClass, isExact);
*madeChanges = true;
}
}
}
Expand All @@ -770,6 +776,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 +796,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 +826,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
52 changes: 52 additions & 0 deletions src/tests/JIT/opt/Inline/tests/Inline_DetectChanges.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// 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.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;
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()
{
try
{
Foo(null);
}
catch (NullReferenceException)
{
return 100;
}
return 101;
}

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
Original file line number Diff line number Diff line change
@@ -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>