From 25f0a72643dab90b1463f203c16870f9ef207b9d Mon Sep 17 00:00:00 2001 From: Eugene Rozenfeld Date: Wed, 19 Aug 2020 14:40:18 -0700 Subject: [PATCH] Improve tailcallstress testing 1. Dispatch all tail calls under TailCallStress via helpers. That increases our coverage since valid non-tail-prefixed calls are dispatched as fast calls in normal non-stress mode. 2. Don't attempt to tail call from methods that have a localloc unless there is an explicit tail prefix. Note that we already disallowed fast tail calls from such methods so this change only affects tailcallstress mode. 3. Fix a bug in TestInvokeDOPAndCancel. As the test was written this assert was firing under tailcallstress: https://github.com/dotnet/runtime/blob/480c49b2419ab4a0b34bfd86754abc2f17079c77/src/libraries/System.Threading.Tasks.Parallel/tests/ParallelForTests.cs#L1074 When this call to `cts.Cancel`: https://github.com/dotnet/runtime/blob/480c49b2419ab4a0b34bfd86754abc2f17079c77/src/libraries/System.Threading.Tasks.Parallel/tests/ParallelForTests.cs#L1065 is dispatched via helpers, it takes much longer than when it's dispatched via a fast tail call (we have to jit a couple of IL stubs). Because of that, it's possible that all other actions complete on the other thread before cancellation is completed. --- src/coreclr/src/jit/morph.cpp | 16 +++++++++++++++- .../tests/ParallelForTests.cs | 2 +- .../System.Threading.Thread/tests/ThreadTests.cs | 4 ++-- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp index b173993d6bb55..844692cc41543 100644 --- a/src/coreclr/src/jit/morph.cpp +++ b/src/coreclr/src/jit/morph.cpp @@ -6774,6 +6774,12 @@ bool Compiler::fgCanFastTailCall(GenTreeCall* callee, const char** failReason) return false; } + if (callee->IsStressTailCall()) + { + reportFastTailCallDecision("Fast tail calls are not performed under tail call stress"); + return false; + } + // Note on vararg methods: // If the caller is vararg method, we don't know the number of arguments passed by caller's caller. // But we can be sure that in-coming arg area of vararg caller would be sufficient to hold its @@ -7226,6 +7232,14 @@ GenTree* Compiler::fgMorphPotentialTailCall(GenTreeCall* call) // is set. This avoids the need for iterating through all lcl vars of the current // method. Right now throughout the code base we are not consistently using 'set' // method to set lvHasLdAddrOp and lvAddrExposed flags. + + bool isImplicitOrStressTailCall = call->IsImplicitTailCall() || call->IsStressTailCall(); + if (isImplicitOrStressTailCall && compLocallocUsed) + { + failTailCall("Localloc used"); + return nullptr; + } + bool hasStructParam = false; for (unsigned varNum = 0; varNum < lvaCount; varNum++) { @@ -7235,7 +7249,7 @@ GenTree* Compiler::fgMorphPotentialTailCall(GenTreeCall* call) // We still must check for any struct parameters and set 'hasStructParam' // so that we won't transform the recursive tail call into a loop. // - if (call->IsImplicitTailCall() || call->IsStressTailCall()) + if (isImplicitOrStressTailCall) { if (varDsc->lvHasLdAddrOp && !lvaIsImplicitByRefLocal(varNum)) { diff --git a/src/libraries/System.Threading.Tasks.Parallel/tests/ParallelForTests.cs b/src/libraries/System.Threading.Tasks.Parallel/tests/ParallelForTests.cs index 583bc08f773fa..9457f8e89425c 100644 --- a/src/libraries/System.Threading.Tasks.Parallel/tests/ParallelForTests.cs +++ b/src/libraries/System.Threading.Tasks.Parallel/tests/ParallelForTests.cs @@ -1061,7 +1061,7 @@ public static void TestInvokeDOPAndCancel() { int newVal = Interlocked.Increment(ref counter); if (newVal == 1) throw new Exception("some non-cancellation-related exception"); - if (newVal == 2) cts.Cancel(); + if (newVal >= 2) cts.Cancel(); }; for (int i = 0; i < numActions; i++) actions[i] = a3; diff --git a/src/libraries/System.Threading.Thread/tests/ThreadTests.cs b/src/libraries/System.Threading.Thread/tests/ThreadTests.cs index 5bc54ba6b75b6..66d8250f5fea7 100644 --- a/src/libraries/System.Threading.Thread/tests/ThreadTests.cs +++ b/src/libraries/System.Threading.Thread/tests/ThreadTests.cs @@ -29,8 +29,8 @@ public static class ThreadTests [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] public static void ConstructorTest() { - const int SmallStackSizeBytes = 64 << 10; // 64 KB, currently accepted in all supported platforms, and is the PAL minimum - const int LargeStackSizeBytes = 16 << 20; // 16 MB + const int SmallStackSizeBytes = 128 << 10; // 128 KB + const int LargeStackSizeBytes = 16 << 20; // 16 MB int pageSizeBytes = Environment.SystemPageSize;