Skip to content

Commit

Permalink
JIT: Fix invalid zero-init suppression for untracked variables (#91580)
Browse files Browse the repository at this point in the history
optRemoveRedundantZeroInits has logic to remove unnecessary zero inits
if we can determine that the local will be zeroed in the prolog. In
addition, it also has logic to suppress the prolog zero init if there is
a dominating initialization already. The latter logic was trying to
reason about liveness for untracked locals, which does not make sense.

Fix #91576
  • Loading branch information
jakobbotsch authored Sep 9, 2023
1 parent 9f138e0 commit 671d42c
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 8 deletions.
16 changes: 8 additions & 8 deletions src/coreclr/jit/optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9044,9 +9044,9 @@ void Compiler::optRemoveRedundantZeroInits()
CompAllocator allocator(getAllocator(CMK_ZeroInit));
LclVarRefCounts refCounts(allocator);
BitVecTraits bitVecTraits(lvaCount, this);
BitVec zeroInitLocals = BitVecOps::MakeEmpty(&bitVecTraits);
bool hasGCSafePoint = false;
bool canThrow = false;
BitVec zeroInitLocals = BitVecOps::MakeEmpty(&bitVecTraits);
bool hasGCSafePoint = false;
bool hasImplicitControlFlow = false;

assert(fgNodeThreading == NodeThreading::AllTrees);

Expand All @@ -9057,6 +9057,8 @@ void Compiler::optRemoveRedundantZeroInits()
CompAllocator allocator(getAllocator(CMK_ZeroInit));
LclVarRefCounts defsInBlock(allocator);
bool removedTrackedDefs = false;
bool hasEHSuccs = block->HasPotentialEHSuccs(this);

for (Statement* stmt = block->FirstNonPhiDef(); stmt != nullptr;)
{
Statement* next = stmt->GetNextStmt();
Expand All @@ -9067,10 +9069,7 @@ void Compiler::optRemoveRedundantZeroInits()
hasGCSafePoint = true;
}

if ((tree->gtFlags & GTF_EXCEPT) != 0)
{
canThrow = true;
}
hasImplicitControlFlow |= hasEHSuccs && ((tree->gtFlags & GTF_EXCEPT) != 0);

switch (tree->gtOper)
{
Expand Down Expand Up @@ -9216,7 +9215,8 @@ void Compiler::optRemoveRedundantZeroInits()
}
}

if (!removedExplicitZeroInit && isEntire && (!canThrow || !lclDsc->lvLiveInOutOfHndlr))
if (!removedExplicitZeroInit && isEntire &&
(!hasImplicitControlFlow || (lclDsc->lvTracked && !lclDsc->lvLiveInOutOfHndlr)))
{
// If compMethodRequiresPInvokeFrame() returns true, lower may later
// insert a call to CORINFO_HELP_INIT_PINVOKE_FRAME which is a gc-safe point.
Expand Down
51 changes: 51 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_91576/Runtime_91576.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.aa

// Generated by Fuzzlyn v1.6 on 2023-09-03 15:59:01
// Run on X64 Windows
// Seed: 11520325105937570553
// Reduced from 294.5 KiB to 0.7 KiB in 00:04:32
// Debug: Outputs False
// Release: Outputs True
using System;
using System.Runtime.CompilerServices;
using Xunit;

public class Runtime_91576
{
[Fact]
public static int TestEntryPoint()
{
Assert.Throws<NullReferenceException>(() =>
{
Run(new int[1]);
Run(null);
});

return s_result;
}

static int s_result;

[MethodImpl(MethodImplOptions.NoInlining)]
private static void Run(int[] l)
{
bool b = false;
try
{
int result = l[0];
b = true;
}
finally
{
Check(ref b);
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static void Check(ref bool b)
{
s_result = b ? 101 : 100;
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>

0 comments on commit 671d42c

Please sign in to comment.