Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Commit

Permalink
Jit: fix issue with single-def type propagation
Browse files Browse the repository at this point in the history
To avoid overly aggressive type propagation when there are multiple
reaching definitions, only propagate types to single-IL-def locals
when the definiting value comes from the same basic block as the store.
We check this conservatively by insisting that the block's entry stack
be empty.

Added a test case where the jit will improperly devirtualize without
such a check.

Closes #10858.
  • Loading branch information
AndyAyersMS committed Apr 10, 2017
1 parent 9cfc763 commit a9044a1
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10094,7 +10094,11 @@ void Compiler::impImportBlockCode(BasicBlock* block)
const bool isSingleILStoreLocal =
!lvaTable[lclNum].lvHasMultipleILStoreOp && !lvaTable[lclNum].lvHasLdAddrOp;

if (isSingleILStoreLocal)
// Conservative check that there is just one
// definition that reaches this store.
const bool hasSingleReachingDef = (block->bbStackDepthOnEntry() == 0);

if (isSingleILStoreLocal && hasSingleReachingDef)
{
lvaUpdateClass(lclNum, op1, clsHnd);
}
Expand Down
46 changes: 46 additions & 0 deletions tests/src/JIT/opt/Devirtualization/GitHub_10858.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;

class B
{
public virtual string F() { return "B"; }
}

sealed class D : B
{
public override string F() { return "D"; }
}

sealed class E : B
{
public override string F() { return "E"; }
}

class X
{
public static int Main(string[] args)
{
// When optimizing IL, CSC will leave the newobj's on the stack
// across the branches to the common def point.
B b1 = (args.Length > 0) ? (B)new E() : (B)new D();
B b2 = null;

// Conditional flow here to forces b1 to a local instead of
// remaining on the stack. So we have a single def point with
// two reaching values.
if (args.Length > 0)
{
b2 = new D();
}
else
{
b2 = new E();
}

// We should not be able to devirtualize either call.
return b2.F()[0] + b1.F()[0] - 37;
}
}
37 changes: 37 additions & 0 deletions tests/src/JIT/opt/Devirtualization/GitHub_10858.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
<PropertyGroup>
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
<AssemblyName>$(MSBuildProjectName)</AssemblyName>
<SchemaVersion>2.0</SchemaVersion>
<ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
<OutputType>Exe</OutputType>
<ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
</PropertyGroup>
<!-- Default configurations to help VS understand the configurations -->
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
</PropertyGroup>
<ItemGroup>
<CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
<Visible>False</Visible>
</CodeAnalysisDependentAssemblyPaths>
</ItemGroup>
<PropertyGroup>
<DebugType>None</DebugType>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="GitHub_10858.cs" />
</ItemGroup>
<ItemGroup>
<Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
</ItemGroup>
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
<PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
</PropertyGroup>
</Project>

0 comments on commit a9044a1

Please sign in to comment.