[llvm] d40dd6b - Revert "[llvm-exegesis] Introduce SubprocessMemory Utility Class"

Aiden Grossman via llvm-commits llvm-commits at lists.llvm.org
Sun Jun 25 20:17:01 PDT 2023


Author: Aiden Grossman
Date: 2023-06-26T03:15:43Z
New Revision: d40dd6be00d881a13a881f02ffa10b48d1441615

URL: https://github.com/llvm/llvm-project/commit/d40dd6be00d881a13a881f02ffa10b48d1441615
DIFF: https://github.com/llvm/llvm-project/commit/d40dd6be00d881a13a881f02ffa10b48d1441615.diff

LOG: Revert "[llvm-exegesis] Introduce SubprocessMemory Utility Class"

This reverts commit 0b6b400b98b921279fc08c63a2a68ebfcb12a3e2.

The sporadic test failures were fixed during this land, but I forgot to
fix the build failures on certain platforms (seems to mostly be
AArch64/PPC) that result from them not being able to find the symbols
for shm_open and shm_unlink.

Added: 
    

Modified: 
    llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
    llvm/tools/llvm-exegesis/lib/CMakeLists.txt
    llvm/unittests/tools/llvm-exegesis/X86/CMakeLists.txt

Removed: 
    llvm/tools/llvm-exegesis/lib/SubprocessMemory.cpp
    llvm/tools/llvm-exegesis/lib/SubprocessMemory.h
    llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp


################################################################################
diff  --git a/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h b/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
index eb3b839ea09fb..0ad18cee0e725 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
@@ -43,22 +43,6 @@ enum class BenchmarkPhaseSelectorE {
 
 enum class BenchmarkFilter { All, RegOnly, WithMem };
 
-struct MemoryValue {
-  // The arbitrary bit width constant that defines the value.
-  APInt Value;
-  // The size of the value in bytes.
-  size_t SizeBytes;
-  // The index of the memory value.
-  size_t Index;
-};
-
-struct MemoryMapping {
-  // The address to place the mapping at.
-  intptr_t Address;
-  // The name of the value that should be mapped.
-  std::string MemoryValueName;
-};
-
 struct BenchmarkKey {
   // The LLVM opcode name.
   std::vector<MCInst> Instructions;

diff  --git a/llvm/tools/llvm-exegesis/lib/CMakeLists.txt b/llvm/tools/llvm-exegesis/lib/CMakeLists.txt
index 4960fee182762..92894601d39f6 100644
--- a/llvm/tools/llvm-exegesis/lib/CMakeLists.txt
+++ b/llvm/tools/llvm-exegesis/lib/CMakeLists.txt
@@ -66,7 +66,6 @@ add_llvm_library(LLVMExegesis
   SnippetFile.cpp
   SnippetGenerator.cpp
   SnippetRepetitor.cpp
-  SubprocessMemory.cpp
   Target.cpp
   UopsBenchmarkRunner.cpp
 

diff  --git a/llvm/tools/llvm-exegesis/lib/SubprocessMemory.cpp b/llvm/tools/llvm-exegesis/lib/SubprocessMemory.cpp
deleted file mode 100644
index 28dfa06283e89..0000000000000
--- a/llvm/tools/llvm-exegesis/lib/SubprocessMemory.cpp
+++ /dev/null
@@ -1,144 +0,0 @@
-//===-- SubprocessMemory.cpp ------------------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "SubprocessMemory.h"
-#include "Error.h"
-#include "llvm/Support/Error.h"
-
-#ifdef __linux__
-#include <fcntl.h>
-#include <sys/mman.h>
-#include <unistd.h>
-#endif
-
-namespace llvm {
-namespace exegesis {
-
-#ifdef __linux__
-
-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
-  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)
-    return make_error<Failure>(
-        "Failed to create shared memory object for auxiliary memory: " +
-        Twine(strerror(errno)));
-  if (ftruncate(AuxiliaryMemoryFD, AuxiliaryMemorySize) != 0) {
-    return make_error<Failure>("Truncating the auxiliary memory failed: " +
-                               Twine(strerror(errno)));
-  }
-  return Error::success();
-}
-
-Error SubprocessMemory::addMemoryDefinition(
-    std::unordered_map<std::string, MemoryValue> MemoryDefinitions,
-    pid_t ProcessPID) {
-  SharedMemoryNames.reserve(MemoryDefinitions.size());
-  for (auto &[Name, MemVal] : MemoryDefinitions) {
-    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);
-    if (ftruncate(SharedMemoryFD, MemVal.SizeBytes) != 0) {
-      return make_error<Failure>("Truncating a memory definiton failed: " +
-                                 Twine(strerror(errno)));
-    }
-
-    char *SharedMemoryMapping =
-        (char *)mmap(NULL, MemVal.SizeBytes, PROT_READ | PROT_WRITE, MAP_SHARED,
-                     SharedMemoryFD, 0);
-    // fill the buffer with the specified value
-    size_t CurrentByte = 0;
-    const size_t ValueWidthBytes = MemVal.Value.getBitWidth() / 8;
-    while (CurrentByte < MemVal.SizeBytes - ValueWidthBytes) {
-      memcpy(SharedMemoryMapping + CurrentByte, MemVal.Value.getRawData(),
-             ValueWidthBytes);
-      CurrentByte += ValueWidthBytes;
-    }
-    // fill the last section
-    memcpy(SharedMemoryMapping + CurrentByte, MemVal.Value.getRawData(),
-           MemVal.SizeBytes - CurrentByte);
-    if (munmap(SharedMemoryMapping, MemVal.SizeBytes) != 0) {
-      return make_error<Failure>(
-          "Unmapping a memory definition in the parent failed: " +
-          Twine(strerror(errno)));
-    }
-  }
-  return Error::success();
-}
-
-Expected<int> SubprocessMemory::setupAuxiliaryMemoryInSubprocess(
-    std::unordered_map<std::string, MemoryValue> MemoryDefinitions,
-    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)
-    return make_error<Failure>(
-        "Getting file descriptor for auxiliary memory failed");
-  // set up memory value file descriptors
-  int *AuxiliaryMemoryMapping =
-      (int *)mmap(NULL, 4096, PROT_READ | PROT_WRITE, MAP_SHARED,
-                  AuxiliaryMemoryFileDescriptor, 0);
-  if ((intptr_t)AuxiliaryMemoryMapping == -1)
-    return make_error<Failure>("Mapping auxiliary memory failed");
-  AuxiliaryMemoryMapping[0] = CounterFileDescriptor;
-  for (auto &[Name, MemVal] : MemoryDefinitions) {
-    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)
-      return make_error<Failure>("Mapping shared memory failed");
-  }
-  if (munmap(AuxiliaryMemoryMapping, 4096) == -1)
-    return make_error<Failure>("Unmapping auxiliary memory failed");
-  return AuxiliaryMemoryFileDescriptor;
-}
-
-SubprocessMemory::~SubprocessMemory() {
-  for (std::string SharedMemoryName : SharedMemoryNames) {
-    if (shm_unlink(SharedMemoryName.c_str()) != 0) {
-      errs() << "Failed to unlink shared memory section: " << strerror(errno)
-             << "\n";
-    }
-  }
-}
-
-#else
-
-Error SubprocessMemory::initializeSubprocessMemory(pid_t ProcessPID) {
-  return make_error<Failure>(
-      "initializeSubprocessMemory is only supported on Linux");
-}
-
-Error SubprocessMemory::addMemoryDefinition(
-    std::unordered_map<std::string, MemoryValue> MemoryDefinitions,
-    pid_t ProcessPID) {
-  return make_error<Failure>("addMemoryDefinitions is only supported on Linux");
-}
-
-Expected<int> SubprocessMemory::setupAuxiliaryMemoryInSubprocess(
-    std::unordered_map<std::string, MemoryValue> MemoryDefinitions,
-    pid_t ParentPID, int CounterFileDescriptor) {
-  return make_error<Failure>(
-      "setupAuxiliaryMemoryInSubprocess is only supported on Linux");
-}
-
-SubprocessMemory::~SubprocessMemory() {}
-
-#endif // __linux__
-
-} // namespace exegesis
-} // namespace llvm

diff  --git a/llvm/tools/llvm-exegesis/lib/SubprocessMemory.h b/llvm/tools/llvm-exegesis/lib/SubprocessMemory.h
deleted file mode 100644
index 9b55bc40272b3..0000000000000
--- a/llvm/tools/llvm-exegesis/lib/SubprocessMemory.h
+++ /dev/null
@@ -1,65 +0,0 @@
-//===-- SubprocessMemory.h --------------------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-///
-/// \file
-/// Defines a class that automatically handles auxiliary memory and the
-/// underlying shared memory backings for memory definitions
-///
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_TOOLS_LLVM_EXEGESIS_SUBPROCESSMEMORY_H
-#define LLVM_TOOLS_LLVM_EXEGESIS_SUBPROCESSMEMORY_H
-
-#include "BenchmarkResult.h"
-#include <string>
-#include <unordered_map>
-#include <vector>
-
-#ifndef __linux__
-typedef int pid_t;
-#endif // __linux__
-
-namespace llvm {
-namespace exegesis {
-
-class SubprocessMemory {
-public:
-  static constexpr const size_t AuxiliaryMemoryOffset = 1;
-  static constexpr const size_t AuxiliaryMemorySize = 4096;
-
-  Error initializeSubprocessMemory(pid_t ProcessID);
-
-  // The following function sets up memory definitions. It creates shared
-  // memory objects for the definitions and fills them with the specified
-  // values. Arguments: MemoryDefinitions - A map from memory value names to
-  // MemoryValues, ProcessID - The ID of the current process.
-  Error addMemoryDefinition(
-      std::unordered_map<std::string, MemoryValue> MemoryDefinitions,
-      pid_t ProcessID);
-
-  // The following function sets up the auxiliary memory by opening shared
-  // memory objects backing memory definitions and putting file descriptors
-  // into appropriate places. Arguments: MemoryDefinitions - A map from memory
-  // values names to Memoryvalues, ParentPID - The ID of the process that
-  // setup the memory definitions, CounterFileDescriptor - The file descriptor
-  // for the performance counter that will be placed in the auxiliary memory
-  // section.
-  static Expected<int> setupAuxiliaryMemoryInSubprocess(
-      std::unordered_map<std::string, MemoryValue> MemoryDefinitions,
-      pid_t ParentPID, int CounterFileDescriptor);
-
-  ~SubprocessMemory();
-
-private:
-  std::vector<std::string> SharedMemoryNames;
-};
-
-} // namespace exegesis
-} // namespace llvm
-
-#endif

diff  --git a/llvm/unittests/tools/llvm-exegesis/X86/CMakeLists.txt b/llvm/unittests/tools/llvm-exegesis/X86/CMakeLists.txt
index bf8951f3b1148..2ac3a1ae4dea4 100644
--- a/llvm/unittests/tools/llvm-exegesis/X86/CMakeLists.txt
+++ b/llvm/unittests/tools/llvm-exegesis/X86/CMakeLists.txt
@@ -11,7 +11,6 @@ add_llvm_exegesis_unittest_sources(
   SnippetFileTest.cpp
   SnippetGeneratorTest.cpp
   SnippetRepetitorTest.cpp
-  SubprocessMemoryTest.cpp
   TargetTest.cpp
   )
 

diff  --git a/llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp b/llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp
deleted file mode 100644
index a7ddd7596825c..0000000000000
--- a/llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp
+++ /dev/null
@@ -1,114 +0,0 @@
-//===-- SubprocessMemoryTest.cpp --------------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "SubprocessMemory.h"
-
-#include "X86/TestBase.h"
-#include "gtest/gtest.h"
-#include <unordered_map>
-
-#ifdef __linux__
-#include <endian.h>
-#include <fcntl.h>
-#include <sys/mman.h>
-#endif // __linux__
-
-namespace llvm {
-namespace exegesis {
-
-#ifdef __linux__
-
-class SubprocessMemoryTest : public X86TestBase {
-protected:
-  void
-  testCommon(std::unordered_map<std::string, MemoryValue> MemoryDefinitions,
-             const int MainProcessPID) {
-    EXPECT_FALSE(SM.initializeSubprocessMemory(MainProcessPID));
-    EXPECT_FALSE(SM.addMemoryDefinition(MemoryDefinitions, MainProcessPID));
-  }
-
-  void checkSharedMemoryDefinition(const std::string &DefinitionName,
-                                   size_t DefinitionSize,
-                                   std::vector<uint8_t> ExpectedValue) {
-    int SharedMemoryFD =
-        shm_open(DefinitionName.c_str(), O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
-    uint8_t *SharedMemoryMapping = (uint8_t *)mmap(
-        NULL, DefinitionSize, PROT_READ, MAP_SHARED, SharedMemoryFD, 0);
-    EXPECT_NE((intptr_t)SharedMemoryMapping, -1);
-    for (size_t I = 0; I < ExpectedValue.size(); ++I) {
-      EXPECT_EQ(SharedMemoryMapping[I], ExpectedValue[I]);
-    }
-    munmap(SharedMemoryMapping, DefinitionSize);
-  }
-
-  SubprocessMemory SM;
-};
-
-TEST_F(SubprocessMemoryTest, OneDefinition) {
-  testCommon({{"test1", {APInt(8, 0xff), 4096, 0}}}, 0);
-  checkSharedMemoryDefinition("/0memdef0", 4096, {0xff});
-}
-
-TEST_F(SubprocessMemoryTest, MultipleDefinitions) {
-  testCommon({{"test1", {APInt(8, 0xaa), 4096, 0}},
-              {"test2", {APInt(8, 0xbb), 4096, 1}},
-              {"test3", {APInt(8, 0xcc), 4096, 2}}},
-             1);
-  checkSharedMemoryDefinition("/1memdef0", 4096, {0xaa});
-  checkSharedMemoryDefinition("/1memdef1", 4096, {0xbb});
-  checkSharedMemoryDefinition("/1memdef2", 4096, {0xcc});
-}
-
-TEST_F(SubprocessMemoryTest, DefinitionFillsCompletely) {
-  testCommon({{"test1", {APInt(8, 0xaa), 4096, 0}},
-              {"test2", {APInt(16, 0xbbbb), 4096, 1}},
-              {"test3", {APInt(24, 0xcccccc), 4096, 2}}},
-             2);
-  std::vector<uint8_t> Test1Expected(512, 0xaa);
-  std::vector<uint8_t> Test2Expected(512, 0xbb);
-  std::vector<uint8_t> Test3Expected(512, 0xcc);
-  checkSharedMemoryDefinition("/2memdef0", 4096, Test1Expected);
-  checkSharedMemoryDefinition("/2memdef1", 4096, Test2Expected);
-  checkSharedMemoryDefinition("/2memdef2", 4096, Test3Expected);
-}
-
-// The test below only works on little endian systems.
-#ifdef __ORDER_LITTLE_ENDIAN__
-TEST_F(SubprocessMemoryTest, DefinitionEndTruncation) {
-  testCommon({{"test1", {APInt(48, 0xaabbccddeeff), 4096, 0}}}, 3);
-  std::vector<uint8_t> Test1Expected(512, 0);
-  // order is reversed since we're assuming a little endian system.
-  for (size_t I = 0; I < Test1Expected.size(); ++I) {
-    switch (I % 6) {
-    case 0:
-      Test1Expected[I] = 0xff;
-      break;
-    case 1:
-      Test1Expected[I] = 0xee;
-      break;
-    case 2:
-      Test1Expected[I] = 0xdd;
-      break;
-    case 3:
-      Test1Expected[I] = 0xcc;
-      break;
-    case 4:
-      Test1Expected[I] = 0xbb;
-      break;
-    case 5:
-      Test1Expected[I] = 0xaa;
-    }
-  }
-  checkSharedMemoryDefinition("/3memdef0", 4096, Test1Expected);
-}
-#endif // __ORDER_LITTLE_ENDIAN__
-
-#endif // __linux__
-
-} // namespace exegesis
-} // namespace llvm


        


More information about the llvm-commits mailing list