unsigned lnum,
LoopHoistContext* hoistCtxt,
bool* firstBlockAndBeforeSideEffect,
- bool* pHoistable);
+ bool* pHoistable,
+ bool* pCctorDependent);
// Performs the hoisting 'tree' into the PreHeader for loop 'lnum'
void optHoistCandidate(GenTreePtr tree, unsigned lnum, LoopHoistContext* hoistCtxt);
#define GTF_FLD_NULLCHECK 0x80000000 // GT_FIELD -- need to nullcheck the "this" pointer
#define GTF_FLD_VOLATILE 0x40000000 // GT_FIELD/GT_CLS_VAR -- same as GTF_IND_VOLATILE
+#define GTF_FLD_INITCLASS 0x20000000 // GT_FIELD/GT_CLS_VAR -- field access requires preceding class/static init helper
#define GTF_INX_RNGCHK 0x80000000 // GT_INDEX -- the array reference should be range-checked.
#define GTF_INX_REFARR_LAYOUT 0x20000000 // GT_INDEX -- same as GTF_IND_REFARR_LAYOUT
(GTF_IND_VOLATILE | GTF_IND_REFARR_LAYOUT | GTF_IND_TGTANYWHERE | GTF_IND_NONFAULTING | GTF_IND_TLS_REF | \
GTF_IND_UNALIGNED | GTF_IND_INVARIANT | GTF_IND_ARR_INDEX)
-#define GTF_CLS_VAR_ASG_LHS 0x04000000 // GT_CLS_VAR -- this GT_CLS_VAR node is (the effective val) of the LHS
- // of an assignment; don't evaluate it independently.
+#define GTF_CLS_VAR_ASG_LHS 0x04000000 // GT_CLS_VAR -- this GT_CLS_VAR node is (the effective val) of the LHS
+ // of an assignment; don't evaluate it independently.
+#define GTF_CLS_VAR_VOLATILE 0x40000000 // GT_FIELD/GT_CLS_VAR -- same as GTF_IND_VOLATILE
+#define GTF_CLS_VAR_INITCLASS 0x20000000 // GT_FIELD/GT_CLS_VAR -- same as GTF_FLD_INITCLASS
#define GTF_ADDR_ONSTACK 0x80000000 // GT_ADDR -- this expression is guaranteed to be on the stack
// In future, it may be better to just create the right tree here instead of folding it later.
op1 = gtNewFieldRef(lclTyp, pResolvedToken->hField);
+ if (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_INITCLASS)
+ {
+ op1->gtFlags |= GTF_FLD_INITCLASS;
+ }
+
if (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_STATIC_IN_HEAP)
{
op1->gtType = TYP_REF; // points at boxed object
else
#endif // _TARGET_64BIT_
{
- // Only volatile could be set, and it maps over
- noway_assert((tree->gtFlags & ~(GTF_FLD_VOLATILE | GTF_COMMON_MASK)) == 0);
- noway_assert(GTF_FLD_VOLATILE == GTF_IND_VOLATILE);
+ // Only volatile or classinit could be set, and they map over
+ noway_assert((tree->gtFlags & ~(GTF_FLD_VOLATILE | GTF_FLD_INITCLASS | GTF_COMMON_MASK)) == 0);
+ static_assert_no_msg(GTF_FLD_VOLATILE == GTF_CLS_VAR_VOLATILE);
+ static_assert_no_msg(GTF_FLD_INITCLASS == GTF_CLS_VAR_INITCLASS);
tree->SetOper(GT_CLS_VAR);
tree->gtClsVar.gtClsVarHnd = symHnd;
FieldSeqNode* fieldSeq =
{
GenTreePtr stmtTree = stmt->gtStmtExpr;
bool hoistable;
- (void)optHoistLoopExprsForTree(stmtTree, lnum, hoistCtxt, &firstBlockAndBeforeSideEffect, &hoistable);
+ bool cctorDependent;
+ (void)optHoistLoopExprsForTree(stmtTree, lnum, hoistCtxt, &firstBlockAndBeforeSideEffect, &hoistable,
+ &cctorDependent);
if (hoistable)
{
// we will try to hoist the top-level stmtTree
//
// This function returns true if 'tree' is a loop invariant expression.
-// It also sets '*pHoistable' to true if 'tree' can be hoisted into a loop PreHeader block
+// It also sets '*pHoistable' to true if 'tree' can be hoisted into a loop PreHeader block,
+// and sets '*pCctorDependent' if 'tree' is a function of a static field that must not be
+// hoisted (even if '*pHoistable' is true) unless a preceding corresponding cctor init helper
+// call is also hoisted.
//
-bool Compiler::optHoistLoopExprsForTree(
- GenTreePtr tree, unsigned lnum, LoopHoistContext* hoistCtxt, bool* pFirstBlockAndBeforeSideEffect, bool* pHoistable)
+bool Compiler::optHoistLoopExprsForTree(GenTreePtr tree,
+ unsigned lnum,
+ LoopHoistContext* hoistCtxt,
+ bool* pFirstBlockAndBeforeSideEffect,
+ bool* pHoistable,
+ bool* pCctorDependent)
{
// First do the children.
// We must keep track of whether each child node was hoistable or not
//
unsigned nChildren = tree->NumChildren();
bool childrenHoistable[GenTree::MAX_CHILDREN];
+ bool childrenCctorDependent[GenTree::MAX_CHILDREN];
// Initialize the array elements for childrenHoistable[] to false
for (unsigned i = 0; i < nChildren; i++)
{
- childrenHoistable[i] = false;
+ childrenHoistable[i] = false;
+ childrenCctorDependent[i] = false;
}
- bool treeIsInvariant = true;
+ // Initclass CLS_VARs are the base case of cctor dependent trees.
+ bool treeIsCctorDependent = (tree->OperIs(GT_CLS_VAR) && ((tree->gtFlags & GTF_CLS_VAR_INITCLASS) != 0));
+ bool treeIsInvariant = true;
for (unsigned childNum = 0; childNum < nChildren; childNum++)
{
if (!optHoistLoopExprsForTree(tree->GetChild(childNum), lnum, hoistCtxt, pFirstBlockAndBeforeSideEffect,
- &childrenHoistable[childNum]))
+ &childrenHoistable[childNum], &childrenCctorDependent[childNum]))
{
treeIsInvariant = false;
}
+
+ if (childrenCctorDependent[childNum])
+ {
+ // Normally, a parent of a cctor-dependent tree is also cctor-dependent.
+ treeIsCctorDependent = true;
+
+ // Check for the case where we can stop propagating cctor-dependent upwards.
+ if (tree->OperIs(GT_COMMA) && (childNum == 1))
+ {
+ GenTreePtr op1 = tree->gtGetOp1();
+ if (op1->OperIs(GT_CALL))
+ {
+ GenTreeCall* call = op1->AsCall();
+ if ((call->gtCallType == CT_HELPER) &&
+ s_helperCallProperties.MayRunCctor(eeGetHelperNum(call->gtCallMethHnd)))
+ {
+ // Hoisting the comma is ok because it would hoist the initialization along
+ // with the static field reference.
+ treeIsCctorDependent = false;
+ // Hoisting the static field without hoisting the initialization would be
+ // incorrect; unset childrenHoistable for the field to ensure this doesn't
+ // happen.
+ childrenHoistable[0] = false;
+ }
+ }
+ }
+ }
}
- // If all the children of "tree" are hoistable, then "tree" itself can be hoisted
- //
- bool treeIsHoistable = treeIsInvariant;
+ // If all the children of "tree" are hoistable, then "tree" itself can be hoisted,
+ // unless it has a static var reference that can't be hoisted past its cctor call.
+ bool treeIsHoistable = treeIsInvariant && !treeIsCctorDependent;
// But we must see if anything else prevents "tree" from being hoisted.
//
if (treeIsInvariant)
{
// Tree must be a suitable CSE candidate for us to be able to hoist it.
- treeIsHoistable = optIsCSEcandidate(tree);
+ treeIsHoistable &= optIsCSEcandidate(tree);
// If it's a call, it must be a helper call, and be pure.
// Further, if it may run a cctor, it must be labeled as "Hoistable"
treeIsHoistable = false;
}
}
- // Currently we must give up on reads from static variables (even if we are in the first block).
- //
- if (tree->OperGet() == GT_CLS_VAR)
- {
- // TODO-CQ: test that fails if we hoist GT_CLS_VAR: JIT\Directed\Languages\ComponentPascal\pi_r.exe
- // method Main
- treeIsHoistable = false;
- }
}
// Is the value of the whole tree loop invariant?
}
}
- *pHoistable = treeIsHoistable;
+ *pHoistable = treeIsHoistable;
+ *pCctorDependent = treeIsCctorDependent;
return treeIsInvariant;
}
--- /dev/null
+// 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.
+
+// Repro case for a bug involving hoisting of static field loads out of
+// loops and (illegally) above the corresponding type initializer calls.
+
+using System.Runtime.CompilerServices;
+
+namespace N
+{
+ public struct Pair
+ {
+ public int Left;
+ public int Right;
+
+ public static Pair TenFour = new Pair() { Left = 10, Right = 4 };
+ }
+
+ static class C
+ {
+ static int Sum;
+ static int Two;
+
+ // Bug repro requires a use of a Pair value; this is a small fn that takes
+ // a Pair by value to serve as that use. Inline it aggressively so that
+ // we won't think the call might kill the static field.
+ [MethodImpl(MethodImplOptions.AggressiveInlining)]
+ static void Accumulate(Pair pair)
+ {
+ Sum += pair.Left + pair.Right;
+ }
+
+
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ static void SumNFourteens(int n)
+ {
+ for (int i = 0; i < n; ++i)
+ {
+ Two = 2; // Store to C.Two here is a global side-effect above which we won't hoist the static initializer (since it may throw).
+ Accumulate(Pair.TenFour); // Hoisting the load of Pair.TenFour above the static init call is incorrect.
+ }
+ }
+
+ public static int Main(string[] args)
+ {
+ Sum = 0;
+ SumNFourteens(7); // Now Sum = 14 * 7 = 98 (and Two = 2)
+ return Sum + Two; // 98 + 2 = 100 on success
+ }
+ }
+}
--- /dev/null
+<?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>{1D93D1C3-458A-44ED-ABCC-7D0739B28C92}</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>
+
+ <NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
+ </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></DebugType>
+ <Optimize>True</Optimize>
+ <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+ </PropertyGroup>
+ <ItemGroup>
+ <Compile Include="GitHub_10780.cs" />
+ </ItemGroup>
+ <PropertyGroup>
+ <CLRTestBatchPreCommands><![CDATA[
+$(CLRTestBatchPreCommands)
+set COMPlus_TailcallStress=1
+]]></CLRTestBatchPreCommands>
+ <BashCLRTestPreCommands><![CDATA[
+$(BashCLRTestPreCommands)
+export COMPlus_TailcallStress=1
+]]></BashCLRTestPreCommands>
+ </PropertyGroup>
+ <ItemGroup>
+ <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+ </ItemGroup>
+ <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+ <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
+ </PropertyGroup>
+</Project>
projects / platform / upstream / dotnet / runtime.git / commitdiff