diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index ec79901011d828..9861f63d8aa37c 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -462,7 +462,8 @@ enum GenTreeFlags : unsigned GTF_CALL_NULLCHECK = 0x08000000, // GT_CALL -- must check instance pointer for null GTF_CALL_POP_ARGS = 0x04000000, // GT_CALL -- caller pop arguments? - GTF_CALL_HOISTABLE = 0x02000000, // GT_CALL -- call is hoistable + GTF_CALL_HOISTABLE = 0x02000000, // GT_CALL -- call is hoistable (any cctor it runs may run earlier; + // the call still may modify the heap) GTF_TLS_GET_ADDR = 0x01000000, // GT_CALL -- call is tls_get_addr GTF_MEMORYBARRIER_LOAD = 0x40000000, // GT_MEMORYBARRIER -- Load barrier diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 47bce3f36b2d5f..289a05d49c29da 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -4659,7 +4659,7 @@ void Compiler::optHoistLoopBlocks(FlowGraphNaturalLoop* loop, // 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" - // (meaning it won't run a cctor because the class is not precise-init). + // (meaning the class is not precise-init, so the cctor may run earlier). if (treeIsHoistable && tree->IsCall()) { GenTreeCall* call = tree->AsCall(); @@ -4740,7 +4740,7 @@ void Compiler::optHoistLoopBlocks(FlowGraphNaturalLoop* loop, { // If it's a call, it must be a helper call that does not mutate the heap. // Further, if it may run a cctor, it must be labeled as "Hoistable" - // (meaning it won't run a cctor because the class is not precise-init). + // (meaning the class is not precise-init, so the cctor may run earlier). GenTreeCall* call = tree->AsCall(); if (!call->IsHelperCall()) { @@ -5448,21 +5448,12 @@ void Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk, FlowGraphNatura if (call->IsHelperCall()) { CorInfoHelpFunc helpFunc = call->GetHelperNum(); - if (s_helperCallProperties.MutatesHeap(helpFunc)) + // A class constructor run by the helper may modify the heap. + if (s_helperCallProperties.MutatesHeap(helpFunc) || + s_helperCallProperties.MayRunCctor(helpFunc)) { memoryHavoc |= memoryKindSet(GcHeap, ByrefExposed); } - else if (s_helperCallProperties.MayRunCctor(helpFunc)) - { - // If the call is labeled as "Hoistable", then we've checked the - // class that would be constructed, and it is not precise-init, so - // the cctor will not be run by this call. Otherwise, it might be, - // and might have arbitrary side effects. - if ((tree->gtFlags & GTF_CALL_HOISTABLE) == 0) - { - memoryHavoc |= memoryKindSet(GcHeap, ByrefExposed); - } - } } else { diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index ac8164bcc400c1..90e53779f2d9ed 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -15546,8 +15546,8 @@ bool Compiler::fgValueNumberHelperCall(GenTreeCall* call) bool pure = s_helperCallProperties.IsPure(helpFunc); bool isAlloc = s_helperCallProperties.IsAllocator(helpFunc); - bool modHeap = s_helperCallProperties.MutatesHeap(helpFunc); bool mayRunCctor = s_helperCallProperties.MayRunCctor(helpFunc); + bool modHeap = s_helperCallProperties.MutatesHeap(helpFunc) || mayRunCctor; // a cctor may modify the heap bool noThrow = s_helperCallProperties.NoThrow(helpFunc); ValueNumPair vnpExc = ValueNumStore::VNPForEmptyExcSet(); @@ -15697,14 +15697,6 @@ bool Compiler::fgValueNumberHelperCall(GenTreeCall* call) { VNFunc vnf = fgValueNumberJitHelperMethodVNFunc(helpFunc); - if (mayRunCctor) - { - if ((call->gtFlags & GTF_CALL_HOISTABLE) == 0) - { - modHeap = true; - } - } - if (isAlloc && (call->gtArgs.FindWellKnownArg(WellKnownArg::StackArrayLocal) != nullptr)) { if (vnf == VNF_JitNewArr) diff --git a/src/tests/JIT/Regression_ro_2/Runtime_136042.cs b/src/tests/JIT/Regression_ro_2/Runtime_136042.cs new file mode 100644 index 00000000000000..3b86a7733de6df --- /dev/null +++ b/src/tests/JIT/Regression_ro_2/Runtime_136042.cs @@ -0,0 +1,175 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Runtime_136042; + +using System.Runtime.CompilerServices; +using Xunit; + +// Accessing a static of a beforefieldinit class may run its static constructor, which can +// modify the heap. A static read after the access must observe that effect. +// Methods without AggressiveOptimization are precompiled when running with crossgen2. +public static class Runtime_136042 +{ + static class Straight + { + public static int Counter; + public class G { public static int Value = ++Counter; } + } + + static class StraightGen + { + public static int Counter; + public class G { public static int Value = ++Counter; } + } + + static class StraightOpt + { + public static int Counter; + public class G { public static int Value = ++Counter; } + } + + static class StraightOptGen + { + public static int Counter; + public class G { public static int Value = ++Counter; } + } + + static class Loop + { + public static int Counter; + public class G { public static int Value = ++Counter; } + } + + static class LoopGen + { + public static int Counter; + public class G { public static int Value = ++Counter; } + } + + static class LoopOpt + { + public static int Counter; + public class G { public static int Value = ++Counter; } + } + + static class LoopOptGen + { + public static int Counter; + public class G { public static int Value = ++Counter; } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static (int, int, int) StraightNonGeneric() + { + int before = Straight.Counter; + int value = Straight.G.Value; + return (before, value, Straight.Counter); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static (int, int, int) StraightGeneric() + { + int before = StraightGen.Counter; + int value = StraightGen.G.Value; + return (before, value, StraightGen.Counter); + } + + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] + static (int, int, int) StraightOptNonGeneric() + { + int before = StraightOpt.Counter; + int value = StraightOpt.G.Value; + return (before, value, StraightOpt.Counter); + } + + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] + static (int, int, int) StraightOptGeneric() + { + int before = StraightOptGen.Counter; + int value = StraightOptGen.G.Value; + return (before, value, StraightOptGen.Counter); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int LoopNonGeneric(int n) + { + int seen = 0; + int sum = 0; + for (int i = 0; i < n; i++) + { + seen = seen * 10 + Loop.Counter; + sum += Loop.G.Value; + } + return seen * 100 + sum; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int LoopGeneric(int n) + { + int seen = 0; + int sum = 0; + for (int i = 0; i < n; i++) + { + seen = seen * 10 + LoopGen.Counter; + sum += LoopGen.G.Value; + } + return seen * 100 + sum; + } + + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] + static int LoopOptNonGeneric(int n) + { + int seen = 0; + int sum = 0; + for (int i = 0; i < n; i++) + { + seen = seen * 10 + LoopOpt.Counter; + sum += LoopOpt.G.Value; + } + return seen * 100 + sum; + } + + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] + static int LoopOptGeneric(int n) + { + int seen = 0; + int sum = 0; + for (int i = 0; i < n; i++) + { + seen = seen * 10 + LoopOptGen.Counter; + sum += LoopOptGen.G.Value; + } + return seen * 100 + sum; + } + + static void CheckStraight((int Before, int Value, int After) result, int expectedValue) + { + Assert.Equal(expectedValue, result.Value); + Assert.Equal(expectedValue, result.After); + Assert.True(result.Before == expectedValue - 1 || result.Before == expectedValue); + } + + static void CheckLoop(int result, int expectedValue) + { + // Two iterations: the first may observe the counter before or after the static constructor ran, + // the second must observe it after. + int before = expectedValue - 1; + int seenEarly = (before * 10 + expectedValue) * 100 + 2 * expectedValue; + int seenLate = (expectedValue * 10 + expectedValue) * 100 + 2 * expectedValue; + Assert.True(result == seenEarly || result == seenLate, $"Unexpected result {result}"); + } + + [Fact] + public static void TestEntryPoint() + { + CheckStraight(StraightNonGeneric(), 1); + CheckStraight(StraightGeneric(), 1); + CheckStraight(StraightOptNonGeneric(), 1); + CheckStraight(StraightOptGeneric(), 1); + CheckLoop(LoopNonGeneric(2), 1); + CheckLoop(LoopGeneric(2), 1); + CheckLoop(LoopOptNonGeneric(2), 1); + CheckLoop(LoopOptGeneric(2), 1); + } +}