Remove bogus "fast path" in unroller branch update
authorJoseph Tremoulet <jotrem@microsoft.com>
Tue, 22 Nov 2016 16:59:29 +0000 (08:59 -0800)
committerJoseph Tremoulet <jotrem@microsoft.com>
Tue, 22 Nov 2016 17:07:51 +0000 (09:07 -0800)
BasicBlocks whose `bbJumpKind` doesn't make use of `bbJumpDest` may have
arbitrary garbage in their `bbJumpDest` field, so remove the code from
loop unrolling that was expecting such blocks to have null `bbJumpDest`s,
and instead always defer to the `optCopyBlkDest`/`optRedirectBlock`
helpers that check the jump kind before checking the jump dest.

Fixes #8231.

src/jit/optimizer.cpp
tests/src/JIT/Regression/JitBlue/GitHub_8231/GitHub_8231.cs [new file with mode: 0644]
tests/src/JIT/Regression/JitBlue/GitHub_8231/GitHub_8231.csproj [new file with mode: 0644]

index 737cdbd..2eea1d1 100644 (file)
@@ -3164,25 +3164,9 @@ void Compiler::optUnrollLoops()
                 // Now redirect any branches within the newly-cloned iteration
                 for (block = head->bbNext; block != bottom; block = block->bbNext)
                 {
-                    if (BasicBlock* oldDest = block->bbJumpDest)
-                    {
-                        BasicBlock* newBlock = blockMap[block];
-                        BasicBlock* newDest;
-                        if (!blockMap.Lookup(oldDest, &newDest))
-                        {
-                            // This was a loop exit; route to the same exit
-                            newDest = oldDest;
-                        }
-
-                        newBlock->bbJumpDest = newDest;
-                    }
-                    else if (block->bbJumpKind == BBJ_SWITCH)
-                    {
-                        BasicBlock* newBlock = blockMap[block];
-                        assert(newBlock->bbJumpKind == BBJ_SWITCH);
-                        optCopyBlkDest(block, newBlock);
-                        optRedirectBlock(newBlock, &blockMap);
-                    }
+                    BasicBlock* newBlock = blockMap[block];
+                    optCopyBlkDest(block, newBlock);
+                    optRedirectBlock(newBlock, &blockMap);
                 }
 
                 /* update the new value for the unrolled iterator */
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_8231/GitHub_8231.cs b/tests/src/JIT/Regression/JitBlue/GitHub_8231/GitHub_8231.cs
new file mode 100644 (file)
index 0000000..4d2ec76
--- /dev/null
@@ -0,0 +1,62 @@
+// 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;
+using System.Numerics;
+using System.Runtime.CompilerServices;
+
+namespace N
+{
+    public static class C
+    {
+        // This is a regression test for a failure in loop unrolling when
+        // the unrolled loop contains a switch statement.
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static int Test()
+        {
+            int s = 0;
+
+            // Loop to some Vector<T>.Count to trigger unrolling.
+            for (int i = 0; i < Vector<int>.Count; i++)
+            {
+                // Loop contains switch; the bug was that the clones
+                // of the switch were all sharing its BBswtDesc instead
+                // of getting their own, so updates to their jump targets
+                // were incorrectly shared.
+                switch (i)
+                {
+                    case 1: s += 4; break;
+                    case 2: s += 2; break;
+                    case 3: s += i; break;
+                }
+            }
+
+            return s;
+        }
+
+        public static int Main(string[] args)
+        {
+            int result = Test();
+
+            // Expected result is a function of Vector<int>.Count.
+            int expected;
+            switch(Vector<int>.Count)
+            {
+                case 1:
+                    expected = 4;
+                    break;
+                case 2:
+                    expected = 6;
+                    break;
+                default:
+                    expected = 9;
+                    break;
+            }
+
+            // Return 100 on success (result == expected), other
+            // values on failure.
+            return 100 + result - expected;
+        }
+   }
+}
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_8231/GitHub_8231.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_8231/GitHub_8231.csproj
new file mode 100644 (file)
index 0000000..844638f
--- /dev/null
@@ -0,0 +1,39 @@
+<?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>
+    <SchemaVersion>2.0</SchemaVersion>
+    <ProjectGuid>{2649FAFE-07BF-4F93-8120-BA9A69285ABB}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <AppDesignerFolder>Properties</AppDesignerFolder>
+    <FileAlignment>512</FileAlignment>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <ReferencePath>$(ProgramFiles)\Common Files\microsoft shared\VSTT\11.0\UITestExtensionPackages</ReferencePath>
+    <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>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <ItemGroup>
+    <Compile Include="GitHub_8231.cs" />
+  </ItemGroup>
+  <PropertyGroup>
+    <ProjectJson>$(JitPackagesConfigFileDirectory)benchmark\project.json</ProjectJson>
+    <ProjectLockJson>$(JitPackagesConfigFileDirectory)benchmark\project.lock.json</ProjectLockJson>
+  </PropertyGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
+  </PropertyGroup>
+</Project>