Skip to content

Commit

Permalink
JIT: Handle GT_RETFILT in fgInsertStmtNearEnd (#85420)
Browse files Browse the repository at this point in the history
Also add some logging for read backs in physical promotion.

Fix #85088
  • Loading branch information
jakobbotsch authored Apr 27, 2023
1 parent 64bd3b1 commit a08818e
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 12 deletions.
22 changes: 13 additions & 9 deletions src/coreclr/jit/fgstmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ Statement* Compiler::fgNewStmtAtEnd(BasicBlock* block, GenTree* tree, const Debu

//------------------------------------------------------------------------
// fgInsertStmtNearEnd: Insert the given statement at the end of the given basic block,
// but before the GT_JTRUE, if present.
// but before the terminating node, if present.
//
// Arguments:
// block - the block into which 'stmt' will be inserted;
Expand All @@ -182,7 +182,7 @@ void Compiler::fgInsertStmtNearEnd(BasicBlock* block, Statement* stmt)
// This routine can only be used when in tree order.
assert(fgOrder == FGOrderTree);

if (block->KindIs(BBJ_COND, BBJ_SWITCH, BBJ_RETURN))
if (block->KindIs(BBJ_EHFINALLYRET, BBJ_EHFAULTRET, BBJ_EHFILTERRET, BBJ_COND, BBJ_SWITCH, BBJ_RETURN))
{
Statement* firstStmt = block->firstStmt();
noway_assert(firstStmt != nullptr);
Expand All @@ -191,24 +191,28 @@ void Compiler::fgInsertStmtNearEnd(BasicBlock* block, Statement* stmt)
Statement* insertionPoint = lastStmt->GetPrevStmt();

#if DEBUG
if (block->bbJumpKind == BBJ_COND)
if (block->KindIs(BBJ_COND))
{
assert(lastStmt->GetRootNode()->gtOper == GT_JTRUE);
assert(lastStmt->GetRootNode()->OperIs(GT_JTRUE));
}
else if (block->bbJumpKind == BBJ_RETURN)
else if (block->KindIs(BBJ_RETURN))
{
assert((lastStmt->GetRootNode()->gtOper == GT_RETURN) || (lastStmt->GetRootNode()->gtOper == GT_JMP) ||
assert((lastStmt->GetRootNode()->OperIs(GT_RETURN, GT_JMP)) ||
// BBJ_RETURN blocks in functions returning void do not get a GT_RETURN node if they
// have a .tail prefix (even if canTailCall returns false for these calls)
// code:Compiler::impImportBlockCode (search for the RET: label)
// Ditto for real tail calls (all code after them has been removed)
((lastStmt->GetRootNode()->gtOper == GT_CALL) &&
(lastStmt->GetRootNode()->OperIs(GT_CALL) &&
((info.compRetType == TYP_VOID) || lastStmt->GetRootNode()->AsCall()->IsTailCall())));
}
else if (block->KindIs(BBJ_EHFINALLYRET, BBJ_EHFAULTRET, BBJ_EHFILTERRET))
{
assert(lastStmt->GetRootNode()->OperIs(GT_RETFILT));
}
else
{
assert(block->bbJumpKind == BBJ_SWITCH);
assert(lastStmt->GetRootNode()->gtOper == GT_SWITCH);
assert(block->KindIs(BBJ_SWITCH));
assert(lastStmt->GetRootNode()->OperIs(GT_SWITCH));
}
#endif // DEBUG

Expand Down
8 changes: 5 additions & 3 deletions src/coreclr/jit/promotion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1770,11 +1770,13 @@ PhaseStatus Promotion::Run()
assert(!rep.NeedsReadBack || !rep.NeedsWriteBack);
if (rep.NeedsReadBack)
{
JITDUMP("Reading back replacement V%02u.[%03u..%03u) -> V%02u at the end of " FMT_BB "\n", i,
JITDUMP("Reading back replacement V%02u.[%03u..%03u) -> V%02u near the end of " FMT_BB ":\n", i,
rep.Offset, rep.Offset + genTypeSize(rep.AccessType), rep.LclNum, bb->bbNum);

GenTree* readBack = replacer.CreateReadBack(i, rep);
m_compiler->fgInsertStmtNearEnd(bb, m_compiler->fgNewStmtFromTree(readBack));
GenTree* readBack = replacer.CreateReadBack(i, rep);
Statement* stmt = m_compiler->fgNewStmtFromTree(readBack);
DISPSTMT(stmt);
m_compiler->fgInsertStmtNearEnd(bb, stmt);
rep.NeedsReadBack = false;
}

Expand Down
50 changes: 50 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_85088/Runtime_85088.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// 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_85088
{
[Fact]
public static int Test()
{
Foo f = new();
try
{
try
{
throw new Exception();
}
finally
{
f.X = 15;
f.Y = 20;
f.X += f.Y;
f.Y *= f.X;

// f will be physically promoted and will require a read back after this call.
// Since this is a finally, some platforms will have a GT_RETFILT that we were
// inserting IR after instead of before.
f = Call(f);
}
}
catch
{
}

return f.X + f.Y;
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static Foo Call(Foo f)
{
return new Foo { X = 75, Y = 25 };
}

private struct Foo
{
public short X, Y;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
<CLRTestEnvironmentVariable Include="DOTNET_JitStressModeNames" Value="STRESS_PHYSICAL_PROMOTION STRESS_PHYSICAL_PROMOTION_COST STRESS_NO_OLD_PROMOTION" />
</ItemGroup>
</Project>

0 comments on commit a08818e

Please sign in to comment.