[llvm-branch-commits] [llvm] eec8568 - Revert "[Support] On Windows, take the affinity mask into account"
Alexandre Ganea via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Wed Jan 13 18:39:12 PST 2021
Author: Alexandre Ganea
Date: 2021-01-13T21:34:54-05:00
New Revision: eec856848ccc481b2704ebf64d725e635a3d7dca
URL: https://github.com/llvm/llvm-project/commit/eec856848ccc481b2704ebf64d725e635a3d7dca
DIFF: https://github.com/llvm/llvm-project/commit/eec856848ccc481b2704ebf64d725e635a3d7dca.diff
LOG: Revert "[Support] On Windows, take the affinity mask into account"
This reverts commit 336ab2d51dfdd5ca09c2a9c506453db4fe653584.
Added:
Modified:
llvm/include/llvm/Support/Program.h
llvm/lib/Support/Program.cpp
llvm/lib/Support/Unix/Program.inc
llvm/lib/Support/Windows/Program.inc
llvm/lib/Support/Windows/Threading.inc
llvm/unittests/Support/ThreadPool.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Support/Program.h b/llvm/include/llvm/Support/Program.h
index bfd271958788..b32de47adb57 100644
--- a/llvm/include/llvm/Support/Program.h
+++ b/llvm/include/llvm/Support/Program.h
@@ -14,7 +14,6 @@
#define LLVM_SUPPORT_PROGRAM_H
#include "llvm/ADT/ArrayRef.h"
-#include "llvm/ADT/BitVector.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Config/llvm-config.h"
@@ -126,11 +125,9 @@ namespace sys {
///< string is non-empty upon return an error occurred while invoking the
///< program.
bool *ExecutionFailed = nullptr,
- Optional<ProcessStatistics> *ProcStat = nullptr, ///< If non-zero,
- /// provides a pointer to a structure in which process execution
- /// statistics will be stored.
- BitVector *AffinityMask = nullptr ///< CPUs or processors the new
- /// program shall run on.
+ Optional<ProcessStatistics> *ProcStat = nullptr ///< If non-zero, provides
+ /// a pointer to a structure in which process execution statistics will be
+ /// stored.
);
/// Similar to ExecuteAndWait, but returns immediately.
@@ -143,8 +140,7 @@ namespace sys {
ArrayRef<Optional<StringRef>> Redirects = {},
unsigned MemoryLimit = 0,
std::string *ErrMsg = nullptr,
- bool *ExecutionFailed = nullptr,
- BitVector *AffinityMask = nullptr);
+ bool *ExecutionFailed = nullptr);
/// Return true if the given arguments fit within system-specific
/// argument length limits.
diff --git a/llvm/lib/Support/Program.cpp b/llvm/lib/Support/Program.cpp
index c7a59642b27e..5294f65bd5a5 100644
--- a/llvm/lib/Support/Program.cpp
+++ b/llvm/lib/Support/Program.cpp
@@ -26,20 +26,17 @@ using namespace sys;
static bool Execute(ProcessInfo &PI, StringRef Program,
ArrayRef<StringRef> Args, Optional<ArrayRef<StringRef>> Env,
ArrayRef<Optional<StringRef>> Redirects,
- unsigned MemoryLimit, std::string *ErrMsg,
- BitVector *AffinityMask);
+ unsigned MemoryLimit, std::string *ErrMsg);
int sys::ExecuteAndWait(StringRef Program, ArrayRef<StringRef> Args,
Optional<ArrayRef<StringRef>> Env,
ArrayRef<Optional<StringRef>> Redirects,
unsigned SecondsToWait, unsigned MemoryLimit,
std::string *ErrMsg, bool *ExecutionFailed,
- Optional<ProcessStatistics> *ProcStat,
- BitVector *AffinityMask) {
+ Optional<ProcessStatistics> *ProcStat) {
assert(Redirects.empty() || Redirects.size() == 3);
ProcessInfo PI;
- if (Execute(PI, Program, Args, Env, Redirects, MemoryLimit, ErrMsg,
- AffinityMask)) {
+ if (Execute(PI, Program, Args, Env, Redirects, MemoryLimit, ErrMsg)) {
if (ExecutionFailed)
*ExecutionFailed = false;
ProcessInfo Result =
@@ -58,13 +55,12 @@ ProcessInfo sys::ExecuteNoWait(StringRef Program, ArrayRef<StringRef> Args,
Optional<ArrayRef<StringRef>> Env,
ArrayRef<Optional<StringRef>> Redirects,
unsigned MemoryLimit, std::string *ErrMsg,
- bool *ExecutionFailed, BitVector *AffinityMask) {
+ bool *ExecutionFailed) {
assert(Redirects.empty() || Redirects.size() == 3);
ProcessInfo PI;
if (ExecutionFailed)
*ExecutionFailed = false;
- if (!Execute(PI, Program, Args, Env, Redirects, MemoryLimit, ErrMsg,
- AffinityMask))
+ if (!Execute(PI, Program, Args, Env, Redirects, MemoryLimit, ErrMsg))
if (ExecutionFailed)
*ExecutionFailed = true;
diff --git a/llvm/lib/Support/Unix/Program.inc b/llvm/lib/Support/Unix/Program.inc
index fb56fa4b0d1d..8f41fc015163 100644
--- a/llvm/lib/Support/Unix/Program.inc
+++ b/llvm/lib/Support/Unix/Program.inc
@@ -174,8 +174,7 @@ toNullTerminatedCStringArray(ArrayRef<StringRef> Strings, StringSaver &Saver) {
static bool Execute(ProcessInfo &PI, StringRef Program,
ArrayRef<StringRef> Args, Optional<ArrayRef<StringRef>> Env,
ArrayRef<Optional<StringRef>> Redirects,
- unsigned MemoryLimit, std::string *ErrMsg,
- BitVector *AffinityMask) {
+ unsigned MemoryLimit, std::string *ErrMsg) {
if (!llvm::sys::fs::exists(Program)) {
if (ErrMsg)
*ErrMsg = std::string("Executable \"") + Program.str() +
@@ -183,9 +182,6 @@ static bool Execute(ProcessInfo &PI, StringRef Program,
return false;
}
- assert(!AffinityMask && "Starting a process with an affinity mask is "
- "currently not supported on Unix!");
-
BumpPtrAllocator Allocator;
StringSaver Saver(Allocator);
std::vector<const char *> ArgVector, EnvVector;
diff --git a/llvm/lib/Support/Windows/Program.inc b/llvm/lib/Support/Windows/Program.inc
index f1d612cf3c98..91df4488850e 100644
--- a/llvm/lib/Support/Windows/Program.inc
+++ b/llvm/lib/Support/Windows/Program.inc
@@ -171,8 +171,7 @@ static HANDLE RedirectIO(Optional<StringRef> Path, int fd,
static bool Execute(ProcessInfo &PI, StringRef Program,
ArrayRef<StringRef> Args, Optional<ArrayRef<StringRef>> Env,
ArrayRef<Optional<StringRef>> Redirects,
- unsigned MemoryLimit, std::string *ErrMsg,
- BitVector *AffinityMask) {
+ unsigned MemoryLimit, std::string *ErrMsg) {
if (!sys::fs::can_execute(Program)) {
if (ErrMsg)
*ErrMsg = "program not executable";
@@ -278,15 +277,11 @@ static bool Execute(ProcessInfo &PI, StringRef Program,
return false;
}
- unsigned CreateFlags = CREATE_UNICODE_ENVIRONMENT;
- if (AffinityMask)
- CreateFlags |= CREATE_SUSPENDED;
-
std::vector<wchar_t> CommandUtf16(Command.size() + 1, 0);
std::copy(Command.begin(), Command.end(), CommandUtf16.begin());
BOOL rc = CreateProcessW(ProgramUtf16.data(), CommandUtf16.data(), 0, 0, TRUE,
- CreateFlags, EnvBlock.empty() ? 0 : EnvBlock.data(),
- 0, &si, &pi);
+ CREATE_UNICODE_ENVIRONMENT,
+ EnvBlock.empty() ? 0 : EnvBlock.data(), 0, &si, &pi);
DWORD err = GetLastError();
// Regardless of whether the process got created or not, we are done with
@@ -334,13 +329,6 @@ static bool Execute(ProcessInfo &PI, StringRef Program,
}
}
- // Set the affinity mask
- if (AffinityMask) {
- ::SetProcessAffinityMask(pi.hProcess,
- (DWORD_PTR)AffinityMask->getData().front());
- ::ResumeThread(pi.hThread);
- }
-
return true;
}
diff --git a/llvm/lib/Support/Windows/Threading.inc b/llvm/lib/Support/Windows/Threading.inc
index 6448bb478d0c..296e87b77695 100644
--- a/llvm/lib/Support/Windows/Threading.inc
+++ b/llvm/lib/Support/Windows/Threading.inc
@@ -195,27 +195,14 @@ static ArrayRef<ProcessorGroup> getProcessorGroups() {
if (!IterateProcInfo(RelationProcessorCore, HandleProc))
return std::vector<ProcessorGroup>();
- // If there's an affinity mask set, assume the user wants to constrain the
- // current process to only a single CPU group. On Windows, it is not
- // possible for affinity masks to cross CPU group boundaries.
- DWORD_PTR ProcessAffinityMask = 0, SystemAffinityMask = 0;
- if (::GetProcessAffinityMask(GetCurrentProcess(), &ProcessAffinityMask,
- &SystemAffinityMask) &&
- ProcessAffinityMask != SystemAffinityMask) {
- // We don't expect more that 4 CPU groups on Windows (256 processors).
- USHORT GroupCount = 4;
- USHORT GroupArray[4]{};
- if (::GetProcessGroupAffinity(GetCurrentProcess(), &GroupCount,
- GroupArray)) {
- assert(GroupCount == 1 &&
- "On startup, a program is expected to be assigned only to "
- "one processor group!");
- unsigned CurrentGroupID = GroupArray[0];
- ProcessorGroup NewG{Groups[CurrentGroupID]};
- NewG.Affinity = ProcessAffinityMask;
- NewG.UsableThreads = countPopulation(ProcessAffinityMask);
+ // If there's an affinity mask set on one of the CPUs, then assume the user
+ // wants to constrain the current process to only a single CPU.
+ for (auto &G : Groups) {
+ if (G.UsableThreads != G.AllThreads) {
+ ProcessorGroup NewG{G};
Groups.clear();
Groups.push_back(NewG);
+ break;
}
}
diff --git a/llvm/unittests/Support/ThreadPool.cpp b/llvm/unittests/Support/ThreadPool.cpp
index d9520a9855df..43882d0f3cee 100644
--- a/llvm/unittests/Support/ThreadPool.cpp
+++ b/llvm/unittests/Support/ThreadPool.cpp
@@ -8,13 +8,11 @@
#include "llvm/Support/ThreadPool.h"
+#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/Triple.h"
-#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Host.h"
-#include "llvm/Support/Program.h"
#include "llvm/Support/TargetSelect.h"
#include "llvm/Support/Threading.h"
@@ -73,7 +71,7 @@ class ThreadPoolTest : public testing::Test {
void SetUp() override { MainThreadReady = false; }
- std::vector<llvm::BitVector> RunOnAllSockets(ThreadPoolStrategy S);
+ void RunOnAllSockets(ThreadPoolStrategy S);
std::condition_variable WaitMainThread;
std::mutex WaitMainThreadMutex;
@@ -171,16 +169,15 @@ TEST_F(ThreadPoolTest, PoolDestruction) {
#if LLVM_ENABLE_THREADS == 1
-std::vector<llvm::BitVector>
-ThreadPoolTest::RunOnAllSockets(ThreadPoolStrategy S) {
+void ThreadPoolTest::RunOnAllSockets(ThreadPoolStrategy S) {
// FIXME: Skip these tests on non-Windows because multi-socket system were not
// tested on Unix yet, and llvm::get_thread_affinity_mask() isn't implemented
// for Unix.
Triple Host(Triple::normalize(sys::getProcessTriple()));
if (!Host.isOSWindows())
- return {};
+ return;
- llvm::SetVector<llvm::BitVector> ThreadsUsed;
+ llvm::DenseSet<llvm::BitVector> ThreadsUsed;
std::mutex Lock;
{
std::condition_variable AllThreads;
@@ -201,7 +198,7 @@ ThreadPoolTest::RunOnAllSockets(ThreadPoolStrategy S) {
ThreadsUsed.insert(Mask);
});
}
- EXPECT_EQ(true, ThreadsUsed.empty());
+ ASSERT_EQ(true, ThreadsUsed.empty());
{
std::unique_lock<std::mutex> Guard(AllThreadsLock);
AllThreads.wait(Guard,
@@ -209,67 +206,17 @@ ThreadPoolTest::RunOnAllSockets(ThreadPoolStrategy S) {
}
setMainThreadReady();
}
- return ThreadsUsed.takeVector();
+ ASSERT_EQ(llvm::get_cpus(), ThreadsUsed.size());
}
TEST_F(ThreadPoolTest, AllThreads_UseAllRessources) {
CHECK_UNSUPPORTED();
- std::vector<llvm::BitVector> ThreadsUsed = RunOnAllSockets({});
- ASSERT_EQ(llvm::get_cpus(), ThreadsUsed.size());
+ RunOnAllSockets({});
}
TEST_F(ThreadPoolTest, AllThreads_OneThreadPerCore) {
CHECK_UNSUPPORTED();
- std::vector<llvm::BitVector> ThreadsUsed =
- RunOnAllSockets(llvm::heavyweight_hardware_concurrency());
- ASSERT_EQ(llvm::get_cpus(), ThreadsUsed.size());
+ RunOnAllSockets(llvm::heavyweight_hardware_concurrency());
}
-#if defined(_WIN32) // FIXME: implement AffinityMask in Support/Unix/Program.inc
-
-// From TestMain.cpp.
-extern const char *TestMainArgv0;
-
-// Just a reachable symbol to ease resolving of the executable's path.
-static cl::opt<std::string> ThreadPoolTestStringArg1("thread-pool-string-arg1");
-
-#ifdef _MSC_VER
-#define setenv(name, var, ignore) _putenv_s(name, var)
#endif
-
-TEST_F(ThreadPoolTest, AffinityMask) {
- CHECK_UNSUPPORTED();
-
- // Skip this test if less than 4 threads are available.
- if (llvm::hardware_concurrency().compute_thread_count() < 4)
- return;
-
- using namespace llvm::sys;
- if (getenv("LLVM_THREADPOOL_AFFINITYMASK")) {
- std::vector<llvm::BitVector> ThreadsUsed = RunOnAllSockets({});
- // Ensure the threads only ran on CPUs 0-3.
- for (auto &It : ThreadsUsed)
- ASSERT_LT(It.getData().front(), 16UL);
- return;
- }
- std::string Executable =
- sys::fs::getMainExecutable(TestMainArgv0, &ThreadPoolTestStringArg1);
- StringRef argv[] = {Executable, "--gtest_filter=ThreadPoolTest.AffinityMask"};
-
- // Add environment variable to the environment of the child process.
- int Res = setenv("LLVM_THREADPOOL_AFFINITYMASK", "1", false);
- ASSERT_EQ(Res, 0);
-
- std::string Error;
- bool ExecutionFailed;
- BitVector Affinity;
- Affinity.resize(4);
- Affinity.set(0, 4); // Use CPUs 0,1,2,3.
- int Ret = sys::ExecuteAndWait(Executable, argv, {}, {}, 0, 0, &Error,
- &ExecutionFailed, nullptr, &Affinity);
- ASSERT_EQ(0, Ret);
-}
-
-#endif // #if _WIN32
-
-#endif // #if LLVM_ENABLE_THREADS == 1
More information about the llvm-branch-commits
mailing list