From feaa48dc100531e0af5ca79eebc8619d42b3f69d Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 29 Oct 2019 15:13:46 -0700 Subject: [PATCH] Revert removal of SuppressGCTransition from SystemNative_GetTimestamp() (dotnet/coreclr#27473) * Revert removal of SuppressGCTransition from SystemNative_GetTimestamp() * Insert GC_POLL before statement with unmanaged call. * JIT test for insertion of GCPoll Commit migrated from https://github.com/dotnet/coreclr/commit/77f64aa03e74496c34ca484d6351263fe978757d --- src/coreclr/src/jit/compiler.h | 2 +- src/coreclr/src/jit/flowgraph.cpp | 35 ++++++- src/coreclr/src/jit/morph.cpp | 2 +- .../src/JIT/Methodical/gc_poll/CMakeLists.txt | 11 +++ .../src/JIT/Methodical/gc_poll/GCPollNative.cpp | 44 +++++++++ .../src/JIT/Methodical/gc_poll/InsertGCPoll.cs | 108 +++++++++++++++++++++ .../src/JIT/Methodical/gc_poll/InsertGCPoll.csproj | 15 +++ .../Unix/System.Native/Interop.GetTimestamp.cs | 2 +- 8 files changed, 213 insertions(+), 6 deletions(-) create mode 100644 src/coreclr/tests/src/JIT/Methodical/gc_poll/CMakeLists.txt create mode 100644 src/coreclr/tests/src/JIT/Methodical/gc_poll/GCPollNative.cpp create mode 100644 src/coreclr/tests/src/JIT/Methodical/gc_poll/InsertGCPoll.cs create mode 100644 src/coreclr/tests/src/JIT/Methodical/gc_poll/InsertGCPoll.csproj diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h index 903fbc5..2b9776b 100644 --- a/src/coreclr/src/jit/compiler.h +++ b/src/coreclr/src/jit/compiler.h @@ -4827,7 +4827,7 @@ public: bool fgGCPollsCreated; void fgMarkGCPollBlocks(); void fgCreateGCPolls(); - bool fgCreateGCPoll(GCPollType pollType, BasicBlock* block); + bool fgCreateGCPoll(GCPollType pollType, BasicBlock* block, Statement* stmt = nullptr); // Requires that "block" is a block that returns from // a finally. Returns the number of successors (jump targets of diff --git a/src/coreclr/src/jit/flowgraph.cpp b/src/coreclr/src/jit/flowgraph.cpp index 3d58610..86c630f 100644 --- a/src/coreclr/src/jit/flowgraph.cpp +++ b/src/coreclr/src/jit/flowgraph.cpp @@ -3858,7 +3858,7 @@ void Compiler::fgCreateGCPolls() * a basic block. */ -bool Compiler::fgCreateGCPoll(GCPollType pollType, BasicBlock* block) +bool Compiler::fgCreateGCPoll(GCPollType pollType, BasicBlock* block, Statement* stmt) { bool createdPollBlocks; @@ -3883,15 +3883,44 @@ bool Compiler::fgCreateGCPoll(GCPollType pollType, BasicBlock* block) pollType = GCPOLL_CALL; } +#ifdef DEBUG + // If a statment was supplied it should be contained in the block. + if (stmt != nullptr) + { + bool containsStmt = false; + for (Statement* stmtMaybe : block->Statements()) + { + containsStmt = (stmtMaybe == stmt); + if (containsStmt) + { + break; + } + } + + assert(containsStmt); + } +#endif + if (GCPOLL_CALL == pollType) { createdPollBlocks = false; GenTreeCall* call = gtNewHelperCallNode(CORINFO_HELP_POLL_GC, TYP_VOID); GenTree* temp = fgMorphCall(call); - // for BBJ_ALWAYS I don't need to insert it before the condition. Just append it. - if (block->bbJumpKind == BBJ_ALWAYS) + if (stmt != nullptr) + { + // The GC_POLL should be inserted relative to the supplied statement. The safer + // location for the insertion is prior to the current statement since the supplied + // statement could be a GT_JTRUE (see fgNewStmtNearEnd() for more details). + Statement* newStmt = gtNewStmt(temp); + + // Set the GC_POLL statement to have the same IL offset at the subsequent one. + newStmt->SetILOffsetX(stmt->GetILOffsetX()); + fgInsertStmtBefore(block, stmt, newStmt); + } + else if (block->bbJumpKind == BBJ_ALWAYS) { + // for BBJ_ALWAYS I don't need to insert it before the condition. Just append it. fgNewStmtAtEnd(block, temp); } else diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp index 87a3967..da02e6b 100644 --- a/src/coreclr/src/jit/morph.cpp +++ b/src/coreclr/src/jit/morph.cpp @@ -8220,7 +8220,7 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call) if (fgGlobalMorph && call->IsUnmanaged() && call->IsSuppressGCTransition()) { // Insert a GC poll. - bool insertedBB = fgCreateGCPoll(GCPOLL_CALL, compCurBB); + bool insertedBB = fgCreateGCPoll(GCPOLL_CALL, compCurBB, compCurStmt); assert(!insertedBB); // No new block should be inserted } diff --git a/src/coreclr/tests/src/JIT/Methodical/gc_poll/CMakeLists.txt b/src/coreclr/tests/src/JIT/Methodical/gc_poll/CMakeLists.txt new file mode 100644 index 0000000..b832809 --- /dev/null +++ b/src/coreclr/tests/src/JIT/Methodical/gc_poll/CMakeLists.txt @@ -0,0 +1,11 @@ +cmake_minimum_required (VERSION 2.6) +project (GCPollNative) + +set(SOURCES + GCPollNative.cpp +) + +add_library (GCPollNative SHARED ${SOURCES}) + +# add the install targets +install (TARGETS GCPollNative DESTINATION bin) diff --git a/src/coreclr/tests/src/JIT/Methodical/gc_poll/GCPollNative.cpp b/src/coreclr/tests/src/JIT/Methodical/gc_poll/GCPollNative.cpp new file mode 100644 index 0000000..256b267 --- /dev/null +++ b/src/coreclr/tests/src/JIT/Methodical/gc_poll/GCPollNative.cpp @@ -0,0 +1,44 @@ +// 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. + +#include +#include +#include + +#if defined(_MSC_VER) + +#define STDMETHODCALLTYPE __stdcall +#define EXPORT(type) extern "C" type __declspec(dllexport) + +#else // !defined(_MSC_VER) + +#ifdef __i386__ +#define STDMETHODCALLTYPE __attribute__((stdcall)) +#else +#define STDMETHODCALLTYPE +#endif +#define EXPORT(type) extern "C" __attribute__((visibility("default"))) type + +#endif // defined(_MSC_VER) + +namespace +{ + std::atomic _n{ 0 }; + + template + T NextUInt(T t) + { + return (T)((++_n) + t); + } +} + +EXPORT(uint32_t) STDMETHODCALLTYPE NextUInt32(uint32_t t) +{ + return NextUInt(t); +} + +EXPORT(uint64_t) STDMETHODCALLTYPE NextUInt64(uint64_t t) +{ + return NextUInt(t); +} \ No newline at end of file diff --git a/src/coreclr/tests/src/JIT/Methodical/gc_poll/InsertGCPoll.cs b/src/coreclr/tests/src/JIT/Methodical/gc_poll/InsertGCPoll.cs new file mode 100644 index 0000000..37bd076 --- /dev/null +++ b/src/coreclr/tests/src/JIT/Methodical/gc_poll/InsertGCPoll.cs @@ -0,0 +1,108 @@ +// 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.Runtime.InteropServices; + +static class GCPollNative +{ + // Simple function that can be marked as SuppressGCTransition which will + // result in a GCPoll insertion. + [DllImport(nameof(GCPollNative))] + [SuppressGCTransition] + public static extern uint NextUInt32(uint n); + + // Simple function that can be marked as SuppressGCTransition which will + // result in a GCPoll insertion. + [DllImport(nameof(GCPollNative))] + [SuppressGCTransition] + public static extern ulong NextUInt64(ulong n); +} + +class InsertGCPoll +{ + private static int PropNextInt32 => (int)GCPollNative.NextUInt32(0); + private static long PropNextInt64 => (long)GCPollNative.NextUInt64(0); + + private static void AccessAsProperty32() + { + int a = PropNextInt32; + int b = PropNextInt32; + DisplayValues(a, b); + } + + private static void AccessAsProperty64() + { + long a = PropNextInt64; + long b = PropNextInt64; + DisplayValues(a, b); + } + + private static void DisplayValues(T a, T b) + { + Console.WriteLine($"{a} {b}"); + } + + private static void BranchOnProperty32() + { + if (-1 == PropNextInt32) + { + Console.WriteLine(""); + } + } + + private static void BranchOnProperty64() + { + if (-1 == PropNextInt64) + { + Console.WriteLine(""); + } + } + + private static void CompoundStatementBranchOnProperty() + { + if (-1 == (PropNextInt64 + PropNextInt32 - PropNextInt64 + PropNextInt64 - PropNextInt32)) + { + Console.WriteLine(""); + } + } + + private static void LoopOn32() + { + uint i = 0; + for (int j = 0; j < 10 || i < 32; ++j) + { + i += GCPollNative.NextUInt32(1); + } + } + + private static void LoopOn64() + { + ulong i = 0; + for (int j = 0; j < 10 || i < 32; ++j) + { + i += GCPollNative.NextUInt64(1); + } + } + + public static int Main() + { + try + { + AccessAsProperty32(); + AccessAsProperty64(); + BranchOnProperty32(); + BranchOnProperty64(); + CompoundStatementBranchOnProperty(); + LoopOn32(); + LoopOn64(); + } + catch (Exception e) + { + Console.WriteLine(e.ToString()); + return 101; + } + return 100; + } +} \ No newline at end of file diff --git a/src/coreclr/tests/src/JIT/Methodical/gc_poll/InsertGCPoll.csproj b/src/coreclr/tests/src/JIT/Methodical/gc_poll/InsertGCPoll.csproj new file mode 100644 index 0000000..06d2478 --- /dev/null +++ b/src/coreclr/tests/src/JIT/Methodical/gc_poll/InsertGCPoll.csproj @@ -0,0 +1,15 @@ + + + Exe + true + + + True + + + + + + + + \ No newline at end of file diff --git a/src/libraries/System.Private.CoreLib/src/Interop/Unix/System.Native/Interop.GetTimestamp.cs b/src/libraries/System.Private.CoreLib/src/Interop/Unix/System.Native/Interop.GetTimestamp.cs index 60968d8..53acdbc 100644 --- a/src/libraries/System.Private.CoreLib/src/Interop/Unix/System.Native/Interop.GetTimestamp.cs +++ b/src/libraries/System.Private.CoreLib/src/Interop/Unix/System.Native/Interop.GetTimestamp.cs @@ -12,7 +12,7 @@ internal static partial class Interop internal static extern ulong GetTimestampResolution(); [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetTimestamp", ExactSpelling = true)] - // [SuppressGCTransition] // https://github.com/dotnet/coreclr/issues/27465 + [SuppressGCTransition] internal static extern ulong GetTimestamp(); } } -- 2.7.4