Skip to content

Commit

Permalink
Revert "[llvm-exegesis] Add thread IDs to subprocess memory names (#8…
Browse files Browse the repository at this point in the history
…4451)"

This reverts commit 6bbe8a2.

This breaks building LLVM on macOS, failing with

    llvm/tools/llvm-exegesis/lib/SubprocessMemory.cpp:146:33: error: out-of-line definition of 'setupAuxiliaryMemoryInSubprocess' does not match any declaration in 'llvm::exegesis::SubprocessMemory'
    Expected<int> SubprocessMemory::setupAuxiliaryMemoryInSubprocess(
  • Loading branch information
fhahn committed Mar 12, 2024
1 parent 6bbe8a2 commit aefad27
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 33 deletions.
9 changes: 4 additions & 5 deletions llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,6 @@ class SubProcessFunctionExecutorImpl
if (AddMemDefError)
return AddMemDefError;

long ParentTID = SubprocessMemory::getCurrentTID();
pid_t ParentOrChildPID = fork();

if (ParentOrChildPID == -1) {
Expand All @@ -315,7 +314,7 @@ class SubProcessFunctionExecutorImpl
// Unregister handlers, signal handling is now handled through ptrace in
// the host process.
sys::unregisterHandlers();
prepareAndRunBenchmark(PipeFiles[0], Key, ParentTID);
prepareAndRunBenchmark(PipeFiles[0], Key);
// The child process terminates in the above function, so we should never
// get to this point.
llvm_unreachable("Child process didn't exit when expected.");
Expand Down Expand Up @@ -416,8 +415,8 @@ class SubProcessFunctionExecutorImpl
setrlimit(RLIMIT_CORE, &rlim);
}

[[noreturn]] void prepareAndRunBenchmark(int Pipe, const BenchmarkKey &Key,
long ParentTID) const {
[[noreturn]] void prepareAndRunBenchmark(int Pipe,
const BenchmarkKey &Key) const {
// Disable core dumps in the child process as otherwise everytime we
// encounter an execution failure like a segmentation fault, we will create
// a core dump. We report the information directly rather than require the
Expand Down Expand Up @@ -474,7 +473,7 @@ class SubProcessFunctionExecutorImpl

Expected<int> AuxMemFDOrError =
SubprocessMemory::setupAuxiliaryMemoryInSubprocess(
Key.MemoryValues, ParentPID, ParentTID, CounterFileDescriptor);
Key.MemoryValues, ParentPID, CounterFileDescriptor);
if (!AuxMemFDOrError)
exit(ChildProcessExitCodeE::AuxiliaryMemorySetupFailed);

Expand Down
28 changes: 8 additions & 20 deletions llvm/tools/llvm-exegesis/lib/SubprocessMemory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,11 @@
#include "SubprocessMemory.h"
#include "Error.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FormatVariadic.h"
#include <cerrno>

#ifdef __linux__
#include <fcntl.h>
#include <sys/mman.h>
#include <sys/syscall.h>
#include <unistd.h>
#endif

Expand All @@ -24,21 +22,12 @@ namespace exegesis {

#if defined(__linux__) && !defined(__ANDROID__)

long SubprocessMemory::getCurrentTID() {
// We're using the raw syscall here rather than the gettid() function provided
// by most libcs for compatibility as gettid() was only added to glibc in
// version 2.30.
return syscall(SYS_gettid);
}

Error SubprocessMemory::initializeSubprocessMemory(pid_t ProcessID) {
// Add the PID to the shared memory name so that if we're running multiple
// processes at the same time, they won't interfere with each other.
// This comes up particularly often when running the exegesis tests with
// llvm-lit. Additionally add the TID so that downstream consumers
// using multiple threads don't run into conflicts.
std::string AuxiliaryMemoryName =
formatv("/{0}auxmem{1}", getCurrentTID(), ProcessID);
// llvm-lit
std::string AuxiliaryMemoryName = "/auxmem" + std::to_string(ProcessID);
int AuxiliaryMemoryFD = shm_open(AuxiliaryMemoryName.c_str(),
O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
if (AuxiliaryMemoryFD == -1)
Expand All @@ -58,8 +47,8 @@ Error SubprocessMemory::addMemoryDefinition(
pid_t ProcessPID) {
SharedMemoryNames.reserve(MemoryDefinitions.size());
for (auto &[Name, MemVal] : MemoryDefinitions) {
std::string SharedMemoryName =
formatv("/{0}t{1}memdef{2}", ProcessPID, getCurrentTID(), MemVal.Index);
std::string SharedMemoryName = "/" + std::to_string(ProcessPID) + "memdef" +
std::to_string(MemVal.Index);
SharedMemoryNames.push_back(SharedMemoryName);
int SharedMemoryFD =
shm_open(SharedMemoryName.c_str(), O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
Expand Down Expand Up @@ -93,9 +82,8 @@ Error SubprocessMemory::addMemoryDefinition(

Expected<int> SubprocessMemory::setupAuxiliaryMemoryInSubprocess(
std::unordered_map<std::string, MemoryValue> MemoryDefinitions,
pid_t ParentPID, long ParentTID, int CounterFileDescriptor) {
std::string AuxiliaryMemoryName =
formatv("/{0}auxmem{1}", ParentTID, ParentPID);
pid_t ParentPID, int CounterFileDescriptor) {
std::string AuxiliaryMemoryName = "/auxmem" + std::to_string(ParentPID);
int AuxiliaryMemoryFileDescriptor =
shm_open(AuxiliaryMemoryName.c_str(), O_RDWR, S_IRUSR | S_IWUSR);
if (AuxiliaryMemoryFileDescriptor == -1)
Expand All @@ -109,8 +97,8 @@ Expected<int> SubprocessMemory::setupAuxiliaryMemoryInSubprocess(
return make_error<Failure>("Mapping auxiliary memory failed");
AuxiliaryMemoryMapping[0] = CounterFileDescriptor;
for (auto &[Name, MemVal] : MemoryDefinitions) {
std::string MemoryValueName =
formatv("/{0}t{1}memdef{2}", ParentPID, ParentTID, MemVal.Index);
std::string MemoryValueName = "/" + std::to_string(ParentPID) + "memdef" +
std::to_string(MemVal.Index);
AuxiliaryMemoryMapping[AuxiliaryMemoryOffset + MemVal.Index] =
shm_open(MemoryValueName.c_str(), O_RDWR, S_IRUSR | S_IWUSR);
if (AuxiliaryMemoryMapping[AuxiliaryMemoryOffset + MemVal.Index] == -1)
Expand Down
5 changes: 1 addition & 4 deletions llvm/tools/llvm-exegesis/lib/SubprocessMemory.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@ class SubprocessMemory {
static constexpr const size_t AuxiliaryMemoryOffset = 1;
static constexpr const size_t AuxiliaryMemorySize = 4096;

// Gets the thread ID for the calling thread.
static long getCurrentTID();

Error initializeSubprocessMemory(pid_t ProcessID);

// The following function sets up memory definitions. It creates shared
Expand All @@ -57,7 +54,7 @@ class SubprocessMemory {
// section.
static Expected<int> setupAuxiliaryMemoryInSubprocess(
std::unordered_map<std::string, MemoryValue> MemoryDefinitions,
pid_t ParentPID, long ParentTID, int CounterFileDescriptor);
pid_t ParentPID, int CounterFileDescriptor);

~SubprocessMemory();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include <endian.h>
#include <fcntl.h>
#include <sys/mman.h>
#include <sys/syscall.h>
#include <unistd.h>
#endif // __linux__

Expand Down Expand Up @@ -50,9 +49,7 @@ class SubprocessMemoryTest : public X86TestBase {

std::string getSharedMemoryName(const unsigned TestNumber,
const unsigned DefinitionNumber) {
long CurrentTID = syscall(SYS_gettid);
return "/" + std::to_string(getSharedMemoryNumber(TestNumber)) + "t" +
std::to_string(CurrentTID) + "memdef" +
return "/" + std::to_string(getSharedMemoryNumber(TestNumber)) + "memdef" +
std::to_string(DefinitionNumber);
}

Expand Down

0 comments on commit aefad27

Please sign in to comment.