fix DevDiv_546017 (#16090)
authorSergey Andreenko <seandree@microsoft.com>
Tue, 30 Jan 2018 23:39:10 +0000 (15:39 -0800)
committerGitHub <noreply@github.com>
Tue, 30 Jan 2018 23:39:10 +0000 (15:39 -0800)
* fix DevDiv_546017

* add repro

src/jit/morph.cpp
tests/src/JIT/Regression/JitBlue/GitHub_16041/GitHub_16041.il [new file with mode: 0644]
tests/src/JIT/Regression/JitBlue/GitHub_16041/GitHub_16041.ilproj [new file with mode: 0644]

index b378ff5..febf198 100644 (file)
@@ -10040,11 +10040,6 @@ GenTree* Compiler::fgMorphBlkNode(GenTreePtr tree, bool isDest)
     GenTree* addr       = nullptr;
     if (tree->OperIs(GT_COMMA))
     {
-        GenTree* effectiveVal = tree->gtEffectiveVal();
-        addr                  = gtNewOperNode(GT_ADDR, TYP_BYREF, effectiveVal);
-#ifdef DEBUG
-        addr->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED;
-#endif
         // In order to CSE and value number array index expressions and bounds checks,
         // the commas in which they are contained need to match.
         // The pattern is that the COMMA should be the address expression.
@@ -10052,17 +10047,32 @@ GenTree* Compiler::fgMorphBlkNode(GenTreePtr tree, bool isDest)
         // TODO-1stClassStructs: Consider whether this can be improved.
         // Also consider whether some of this can be included in gtNewBlockVal (though note
         // that doing so may cause us to query the type system before we otherwise would).
-        GenTree* lastComma = nullptr;
-        for (GenTree* next = tree; next != nullptr && next->gtOper == GT_COMMA; next = next->gtGetOp2())
+        // Example:
+        //   before: [3] comma struct <- [2] comma struct <- [1] LCL_VAR struct
+        //   after: [3] comma byref <- [2] comma byref <- [4] addr byref <- [1] LCL_VAR struct
+
+        addr                  = tree;
+        GenTree* effectiveVal = tree->gtEffectiveVal();
+
+        GenTreePtrStack commas(this);
+        for (GenTree* comma = tree; comma != nullptr && comma->gtOper == GT_COMMA; comma = comma->gtGetOp2())
         {
-            next->gtType = TYP_BYREF;
-            lastComma    = next;
+            commas.Push(comma);
         }
-        if (lastComma != nullptr)
+
+        GenTree* lastComma = commas.Top();
+        noway_assert(lastComma->gtGetOp2() == effectiveVal);
+        GenTree* effectiveValAddr = gtNewOperNode(GT_ADDR, TYP_BYREF, effectiveVal);
+#ifdef DEBUG
+        effectiveValAddr->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED;
+#endif
+        lastComma->gtOp.gtOp2 = effectiveValAddr;
+
+        while (commas.Height() > 0)
         {
-            noway_assert(lastComma->gtGetOp2() == effectiveVal);
-            lastComma->gtOp.gtOp2 = addr;
-            addr                  = tree;
+            GenTree* comma = commas.Pop();
+            comma->gtType  = TYP_BYREF;
+            gtUpdateNodeSideEffects(comma);
         }
 
         handleTree = effectiveVal;
@@ -10096,6 +10106,8 @@ GenTree* Compiler::fgMorphBlkNode(GenTreePtr tree, bool isDest)
         {
             tree = new (this, GT_BLK) GenTreeBlk(GT_BLK, structType, addr, genTypeSize(structType));
         }
+
+        gtUpdateNodeSideEffects(tree);
 #ifdef DEBUG
         tree->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED;
 #endif
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_16041/GitHub_16041.il b/tests/src/JIT/Regression/JitBlue/GitHub_16041/GitHub_16041.il
new file mode 100644 (file)
index 0000000..50c2e8d
--- /dev/null
@@ -0,0 +1,81 @@
+// 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.
+
+.assembly extern mscorlib { auto }
+.assembly GitHub_16041 { }
+
+// The test originally hit a problem with "Extra flags on tree" on amd64,
+// because "fgMorphBlockOperand" did not set flags when morphing comma for addr exposed lcl_Var.
+// "StructY" is struct with exposed addr, it needs to be larger than 8 bytes for amd64.
+
+
+.class private auto ansi beforefieldinit Test
+       extends [mscorlib]System.Object
+{
+  .method private hidebysig static int32  Main() cil managed
+  {
+    .entrypoint
+    // Code size       10 (0xa)
+    .maxstack  8
+    .locals init (valuetype StructX V_0)
+    IL_0000:  ldloc      V_0
+    IL_0004:  call       void Test::Test(valuetype StructX)
+    IL_0009:  ldc.i4 100
+    IL_000a:  ret
+  } // end of method Test::Main
+
+  .method private hidebysig static void  Test(valuetype StructX StructX) cil managed noinlining
+  {
+    // Code size       10 (0xa)
+    .maxstack  8
+    .locals (valuetype StructY V_0)
+    IL_0000:  ldarga.s   StructX
+    IL_0002:  call       valuetype StructY Test::Convert(valuetype StructX*)
+    IL_0007:  stloc.s    V_0
+    IL_0009:  ret
+  } // end of method Test::Test
+
+  .method public hidebysig static valuetype StructY 
+          Convert(valuetype StructX* StructX) cil managed
+  {
+    // Code size       13 (0xd)
+    .maxstack  3
+    .locals (valuetype StructY V_0)
+    IL_0000:  ldloca.s   V_0
+    IL_0002:  ldarg.0
+    IL_0003:  conv.i
+    IL_0004:  ldc.i4.s   28
+    IL_0006:  unaligned. 4 // This opcode created the bad tree, "Convert" needed to be inlined to have a LCL_VAR here.
+    IL_0009:  cpblk
+    IL_000b:  ldloc.0
+    IL_000c:  ret
+  } // end of method Test::Convert
+
+  .method public hidebysig specialname rtspecialname 
+          instance void  .ctor() cil managed
+  {
+    // Code size       7 (0x7)
+    .maxstack  8
+    IL_0000:  ldarg.0
+    IL_0001:  call       instance void [mscorlib]System.Object::.ctor()
+    IL_0006:  ret
+  } // end of method Test::.ctor
+
+} // end of class Test
+
+.class public sequential ansi sealed beforefieldinit StructX
+       extends [mscorlib]System.ValueType
+{
+  .field private int32 A
+  .field private int32 B
+  .field private int32 C
+} // end of class StructX
+
+.class private sequential ansi sealed beforefieldinit StructY
+       extends [mscorlib]System.ValueType
+{
+  .pack 0
+  .size 28
+  .field private int32 Padding
+} // end of class StructY
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_16041/GitHub_16041.ilproj b/tests/src/JIT/Regression/JitBlue/GitHub_16041/GitHub_16041.ilproj
new file mode 100644 (file)
index 0000000..89871f7
--- /dev/null
@@ -0,0 +1,23 @@
+<?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>
+    <ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "></PropertyGroup>
+  <PropertyGroup>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="GitHub_16041.il" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
+</Project>