[llvm] [llvm-exegesis] Set stack pointer register after starting perf counter (PR #72489)

Aiden Grossman via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 29 01:35:57 PST 2023


https://github.com/boomanaiden154 updated https://github.com/llvm/llvm-project/pull/72489

>From 58f3d25e7bd57915048121967435af894fadfb0a Mon Sep 17 00:00:00 2001
From: Aiden Grossman <agrossman154 at yahoo.com>
Date: Wed, 15 Nov 2023 23:51:20 -0800
Subject: [PATCH 1/3] [llvm-exegesis] Set stack pointer register after starting
 perf counter

Before this patch, in subprocess mode, llvm-exegesis setup the stack
pointer register with the rest of the registers when it was requested by
the user. This would cause a segfault when the instructions to start the
perf counter ran as they use the stack to preserve the three registers
needed to make the syscall. This patch moves the setup of the stack
register to after the configuration of the perf counter to fix this
issue so that we have a valid stack pointer for all the preceeding
operations.

Regression test added.
---
 .../X86/latency/subprocess-rsp.s              | 22 +++++++++++++++++++
 llvm/tools/llvm-exegesis/lib/Assembler.cpp    | 19 ++++++++++++++++
 llvm/tools/llvm-exegesis/lib/Target.h         |  6 +++++
 llvm/tools/llvm-exegesis/lib/X86/Target.cpp   |  4 ++++
 4 files changed, 51 insertions(+)
 create mode 100644 llvm/test/tools/llvm-exegesis/X86/latency/subprocess-rsp.s

diff --git a/llvm/test/tools/llvm-exegesis/X86/latency/subprocess-rsp.s b/llvm/test/tools/llvm-exegesis/X86/latency/subprocess-rsp.s
new file mode 100644
index 000000000000000..2eac0a661a270ce
--- /dev/null
+++ b/llvm/test/tools/llvm-exegesis/X86/latency/subprocess-rsp.s
@@ -0,0 +1,22 @@
+# REQUIRES: exegesis-can-execute-x86_64
+
+# RUN: llvm-exegesis -mtriple=x86_64-unknown-unknown -mode=latency -snippets-file=%s -execution-mode=subprocess | FileCheck %s
+
+# See comment in ./subprocess-abnormal-exit-code.s on the transient
+# PTRACE_ATTACH failure.
+# ALLOW_RETRIES: 2
+
+# Check that we can set the value of RSP in subprocess mode without
+# segfaulting as we need to restore it after the rest of the setup is
+# complete to prevent loading from the stack where we set it instead
+# of where the stack actuall is.
+
+# LLVM-EXEGESIS-MEM-DEF test1 4096 2147483647
+# LLVM-EXEGESIS-MEM-MAP test1 1048576
+# LLVM-EXEGESIS-DEFREG RAX 100000
+# LLVM-EXEGESIS-DEFREG R14 100000
+# LLVM-EXEGESIS-DEFREG RSP 100000
+
+movq %r14, (%rax)
+
+# CHECK-NOT: error:           'The benchmarking subprocess sent unexpected signal: Segmentation fault'
diff --git a/llvm/tools/llvm-exegesis/lib/Assembler.cpp b/llvm/tools/llvm-exegesis/lib/Assembler.cpp
index 167fb6373377c28..43ab730eba90a98 100644
--- a/llvm/tools/llvm-exegesis/lib/Assembler.cpp
+++ b/llvm/tools/llvm-exegesis/lib/Assembler.cpp
@@ -60,6 +60,13 @@ static bool generateSnippetSetupCode(
     BBF.addInstructions(ET.setStackRegisterToAuxMem());
   }
   for (const RegisterValue &RV : RegisterInitialValues) {
+    if (GenerateMemoryInstructions) {
+      // If we're generating memory instructions, don't load in the value for
+      // the register with the stack pointer as it will be used later to finish
+      // the setup.
+      if (RV.Register == ET.getStackRegister())
+        continue;
+    }
     // Load a constant in the register.
     const auto SetRegisterCode = ET.setRegTo(*MSI, RV.Register, RV.Value);
     if (SetRegisterCode.empty())
@@ -70,6 +77,18 @@ static bool generateSnippetSetupCode(
 #ifdef HAVE_LIBPFM
     BBF.addInstructions(ET.configurePerfCounter(PERF_EVENT_IOC_RESET, true));
 #endif // HAVE_LIBPFM
+    for (const RegisterValue &RV : RegisterInitialValues) {
+      // Load in the stack register now as we're done using it elsewhere
+      // and need to set the value in preparation for executing the
+      // snippet.
+      if (RV.Register != ET.getStackRegister())
+        continue;
+      const auto SetRegisterCode = ET.setRegTo(*MSI, RV.Register, RV.Value);
+      if (SetRegisterCode.empty())
+        IsSnippetSetupComplete = false;
+      BBF.addInstructions(SetRegisterCode);
+      break;
+    }
   }
   return IsSnippetSetupComplete;
 }
diff --git a/llvm/tools/llvm-exegesis/lib/Target.h b/llvm/tools/llvm-exegesis/lib/Target.h
index 6de5b3c1065f1aa..b4fae7bff70f097 100644
--- a/llvm/tools/llvm-exegesis/lib/Target.h
+++ b/llvm/tools/llvm-exegesis/lib/Target.h
@@ -170,6 +170,12 @@ class ExegesisTarget {
         "configurePerfCounter is not implemented on the current architecture");
   }
 
+  // Returns the stack register for the platform.
+  virtual unsigned getStackRegister() const {
+    report_fatal_error(
+        "getStackRegister is not implemented on the current architecture");
+  }
+
   // Gets the ABI dependent registers that are used to pass arguments in a
   // function call.
   virtual std::vector<unsigned> getArgumentRegisters() const {
diff --git a/llvm/tools/llvm-exegesis/lib/X86/Target.cpp b/llvm/tools/llvm-exegesis/lib/X86/Target.cpp
index ac99e98cc851f34..33a248110d8d63e 100644
--- a/llvm/tools/llvm-exegesis/lib/X86/Target.cpp
+++ b/llvm/tools/llvm-exegesis/lib/X86/Target.cpp
@@ -745,6 +745,8 @@ class ExegesisX86Target : public ExegesisTarget {
 
   std::vector<MCInst> configurePerfCounter(long Request, bool SaveRegisters) const override;
 
+  unsigned getStackRegister() const override;
+
   std::vector<unsigned> getArgumentRegisters() const override;
 
   std::vector<unsigned> getRegistersNeedSaving() const override;
@@ -1211,6 +1213,8 @@ ExegesisX86Target::configurePerfCounter(long Request, bool SaveRegisters) const
   return ConfigurePerfCounterCode;
 }
 
+unsigned ExegesisX86Target::getStackRegister() const { return X86::RSP; }
+
 std::vector<unsigned> ExegesisX86Target::getArgumentRegisters() const {
   return {X86::RDI, X86::RSI};
 }

>From fd2ecdfc68905138b871fab1df4f27934d2aceda Mon Sep 17 00:00:00 2001
From: Aiden Grossman <agrossman154 at yahoo.com>
Date: Tue, 28 Nov 2023 23:46:32 -0800
Subject: [PATCH 2/3] Address reviewer feedback

---
 .../tools/llvm-exegesis/X86/latency/subprocess-rsp.s     | 4 ----
 llvm/tools/llvm-exegesis/lib/Assembler.cpp               | 9 +++++++--
 llvm/tools/llvm-exegesis/lib/Target.h                    | 6 ------
 llvm/tools/llvm-exegesis/lib/X86/Target.cpp              | 4 ----
 4 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/llvm/test/tools/llvm-exegesis/X86/latency/subprocess-rsp.s b/llvm/test/tools/llvm-exegesis/X86/latency/subprocess-rsp.s
index 2eac0a661a270ce..63274c43879951c 100644
--- a/llvm/test/tools/llvm-exegesis/X86/latency/subprocess-rsp.s
+++ b/llvm/test/tools/llvm-exegesis/X86/latency/subprocess-rsp.s
@@ -2,10 +2,6 @@
 
 # RUN: llvm-exegesis -mtriple=x86_64-unknown-unknown -mode=latency -snippets-file=%s -execution-mode=subprocess | FileCheck %s
 
-# See comment in ./subprocess-abnormal-exit-code.s on the transient
-# PTRACE_ATTACH failure.
-# ALLOW_RETRIES: 2
-
 # Check that we can set the value of RSP in subprocess mode without
 # segfaulting as we need to restore it after the rest of the setup is
 # complete to prevent loading from the stack where we set it instead
diff --git a/llvm/tools/llvm-exegesis/lib/Assembler.cpp b/llvm/tools/llvm-exegesis/lib/Assembler.cpp
index 43ab730eba90a98..194f13656c73f5c 100644
--- a/llvm/tools/llvm-exegesis/lib/Assembler.cpp
+++ b/llvm/tools/llvm-exegesis/lib/Assembler.cpp
@@ -19,6 +19,7 @@
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/TargetInstrInfo.h"
+#include "llvm/CodeGen/TargetLowering.h"
 #include "llvm/CodeGen/TargetPassConfig.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
 #include "llvm/ExecutionEngine/SectionMemoryManager.h"
@@ -64,7 +65,9 @@ static bool generateSnippetSetupCode(
       // If we're generating memory instructions, don't load in the value for
       // the register with the stack pointer as it will be used later to finish
       // the setup.
-      if (RV.Register == ET.getStackRegister())
+      if (RV.Register == BBF.MF.getSubtarget()
+                             .getTargetLowering()
+                             ->getStackPointerRegisterToSaveRestore())
         continue;
     }
     // Load a constant in the register.
@@ -81,7 +84,9 @@ static bool generateSnippetSetupCode(
       // Load in the stack register now as we're done using it elsewhere
       // and need to set the value in preparation for executing the
       // snippet.
-      if (RV.Register != ET.getStackRegister())
+      if (RV.Register == BBF.MF.getSubtarget()
+                             .getTargetLowering()
+                             ->getStackPointerRegisterToSaveRestore())
         continue;
       const auto SetRegisterCode = ET.setRegTo(*MSI, RV.Register, RV.Value);
       if (SetRegisterCode.empty())
diff --git a/llvm/tools/llvm-exegesis/lib/Target.h b/llvm/tools/llvm-exegesis/lib/Target.h
index b4fae7bff70f097..6de5b3c1065f1aa 100644
--- a/llvm/tools/llvm-exegesis/lib/Target.h
+++ b/llvm/tools/llvm-exegesis/lib/Target.h
@@ -170,12 +170,6 @@ class ExegesisTarget {
         "configurePerfCounter is not implemented on the current architecture");
   }
 
-  // Returns the stack register for the platform.
-  virtual unsigned getStackRegister() const {
-    report_fatal_error(
-        "getStackRegister is not implemented on the current architecture");
-  }
-
   // Gets the ABI dependent registers that are used to pass arguments in a
   // function call.
   virtual std::vector<unsigned> getArgumentRegisters() const {
diff --git a/llvm/tools/llvm-exegesis/lib/X86/Target.cpp b/llvm/tools/llvm-exegesis/lib/X86/Target.cpp
index 33a248110d8d63e..ac99e98cc851f34 100644
--- a/llvm/tools/llvm-exegesis/lib/X86/Target.cpp
+++ b/llvm/tools/llvm-exegesis/lib/X86/Target.cpp
@@ -745,8 +745,6 @@ class ExegesisX86Target : public ExegesisTarget {
 
   std::vector<MCInst> configurePerfCounter(long Request, bool SaveRegisters) const override;
 
-  unsigned getStackRegister() const override;
-
   std::vector<unsigned> getArgumentRegisters() const override;
 
   std::vector<unsigned> getRegistersNeedSaving() const override;
@@ -1213,8 +1211,6 @@ ExegesisX86Target::configurePerfCounter(long Request, bool SaveRegisters) const
   return ConfigurePerfCounterCode;
 }
 
-unsigned ExegesisX86Target::getStackRegister() const { return X86::RSP; }
-
 std::vector<unsigned> ExegesisX86Target::getArgumentRegisters() const {
   return {X86::RDI, X86::RSI};
 }

>From ccc9f112cab74eb36108f78faee6ccccb7b698a2 Mon Sep 17 00:00:00 2001
From: Aiden Grossman <agrossman154 at yahoo.com>
Date: Wed, 29 Nov 2023 01:35:38 -0800
Subject: [PATCH 3/3] Address reviewer feedback

---
 llvm/tools/llvm-exegesis/lib/Assembler.cpp | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/llvm/tools/llvm-exegesis/lib/Assembler.cpp b/llvm/tools/llvm-exegesis/lib/Assembler.cpp
index 194f13656c73f5c..707cfac1635c22d 100644
--- a/llvm/tools/llvm-exegesis/lib/Assembler.cpp
+++ b/llvm/tools/llvm-exegesis/lib/Assembler.cpp
@@ -60,14 +60,15 @@ static bool generateSnippetSetupCode(
     }
     BBF.addInstructions(ET.setStackRegisterToAuxMem());
   }
+  Register StackPointerRegister = BBF.MF.getSubtarget()
+                                      .getTargetLowering()
+                                      ->getStackPointerRegisterToSaveRestore();
   for (const RegisterValue &RV : RegisterInitialValues) {
     if (GenerateMemoryInstructions) {
       // If we're generating memory instructions, don't load in the value for
       // the register with the stack pointer as it will be used later to finish
       // the setup.
-      if (RV.Register == BBF.MF.getSubtarget()
-                             .getTargetLowering()
-                             ->getStackPointerRegisterToSaveRestore())
+      if (RV.Register == StackPointerRegister)
         continue;
     }
     // Load a constant in the register.
@@ -84,9 +85,7 @@ static bool generateSnippetSetupCode(
       // Load in the stack register now as we're done using it elsewhere
       // and need to set the value in preparation for executing the
       // snippet.
-      if (RV.Register == BBF.MF.getSubtarget()
-                             .getTargetLowering()
-                             ->getStackPointerRegisterToSaveRestore())
+      if (RV.Register == StackPointerRegister)
         continue;
       const auto SetRegisterCode = ET.setRegTo(*MSI, RV.Register, RV.Value);
       if (SetRegisterCode.empty())



More information about the llvm-commits mailing list