Skip to content

Commit

Permalink
Fix madeChanges in fgInline (#57782)
Browse files Browse the repository at this point in the history
  • Loading branch information
EgorBo committed Aug 26, 2021
1 parent 4809039 commit a2f5da6
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 11 deletions.
27 changes: 16 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* 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
@@ -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
// 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
@@ -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>

0 comments on commit a2f5da6

Please sign in to comment.