Skip to content

Commit

Permalink
Fix issue with hoisting and copy prop interaction (#85493)
Browse files Browse the repository at this point in the history
* Fix issue with hoisting and copy prop interaction

After hoisting creates a tree copy, it morphs it. That morph might
lose the value numbers on LCL_VAR uses, but leave the SSA numbers.
Copy prop was assuming that such a use had a VN. Instead, check this
dynamically instead of asserting.

Fixes #84619

* Further optimize cast in fgOptimizeCastOnStore()

If we optimize the cast, call `fgOptimizeCast` to see if it can be
optimized further.

* Add regression test

* Feedback
  • Loading branch information
BruceForstall authored May 18, 2023
1 parent 7b0d8fe commit 32b7be7
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/coreclr/jit/copyprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@ int Compiler::optCopyProp_LclVarScore(const LclVarDsc* lclVarDsc, const LclVarDs
bool Compiler::optCopyProp(
BasicBlock* block, Statement* stmt, GenTreeLclVarCommon* tree, unsigned lclNum, LclNumToLiveDefsMap* curSsaName)
{
assert(((tree->gtFlags & GTF_VAR_DEF) == 0) && (tree->GetLclNum() == lclNum) && tree->gtVNPair.BothDefined());
assert((tree->gtFlags & GTF_VAR_DEF) == 0);
assert(tree->GetLclNum() == lclNum);

bool madeChanges = false;
LclVarDsc* varDsc = lvaGetDesc(lclNum);
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10196,6 +10196,9 @@ GenTree* Compiler::fgOptimizeCastOnStore(GenTree* store)
{
// This is a type-changing cast so we cannot remove it entirely.
cast->gtCastType = genActualType(castToType);

// See if we can optimize the new cast.
store->Data() = fgOptimizeCast(cast);
}

return store;
Expand Down
28 changes: 28 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_84619/Runtime_84619.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Xunit;

// Generated by Fuzzlyn v1.5 on 2023-04-09 16:37:03
// Run on X64 Windows
// Seed: 6230188300048624105
// Reduced from 155.1 KiB to 0.2 KiB in 00:01:48
// Hits JIT assert in Release:
// Assertion failed '((tree->gtFlags & GTF_VAR_DEF) == 0) && (tree->GetLclNum() == lclNum) && tree->gtVNPair.BothDefined()' in 'Program:Main(Fuzzlyn.ExecutionServer.IRuntime)' during 'VN based copy prop' (IL size 25; hash 0xade6b36b; FullOpts)
//
// File: D:\a\_work\1\s\src\coreclr\jit\copyprop.cpp Line: 161
//
public class Runtime_84619
{
public static sbyte s_27;
[Fact]
public static int TestEntryPoint()
{
long vr1 = 0;
for (int vr3 = 0; vr3 < -1; vr3++)
{
s_27 = (sbyte)(vr1 | vr1);
}
return 100;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<Optimize>True</Optimize>
<DebugType>None</DebugType>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>

0 comments on commit 32b7be7

Please sign in to comment.