Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[llvm-exegesis] Preserve rcx and r11 around system call #72807

Merged

Conversation

boomanaiden154
Copy link
Contributor

Currently, when making the ioctl system call, we're not preserving rcx and r11. The system call will clobber these registers, meaning that the values of the registers in the snippet will be different than expected. This patch fixes that be preserving the registers around the system call, similar to how the other registers involved in the making the system call get preserved.

Fixes #72741.

Currently, when making the ioctl system call, we're not preserving rcx
and r11. The system call will clobber these registers, meaning that the
values of the registers in the snippet will be different than expected.
This patch fixes that be preserving the registers around the system
call, similar to how the other registers involved in the making the
system call get preserved.
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 19, 2023

@llvm/pr-subscribers-tools-llvm-exegesis

Author: Aiden Grossman (boomanaiden154)

Changes

Currently, when making the ioctl system call, we're not preserving rcx and r11. The system call will clobber these registers, meaning that the values of the registers in the snippet will be different than expected. This patch fixes that be preserving the registers around the system call, similar to how the other registers involved in the making the system call get preserved.

Fixes #72741.


Full diff: https://github.com/llvm/llvm-project/pull/72807.diff

2 Files Affected:

  • (modified) llvm/test/tools/llvm-exegesis/X86/latency/subprocess-preserved-registers.s (+19-13)
  • (modified) llvm/tools/llvm-exegesis/lib/X86/Target.cpp (+9-2)
diff --git a/llvm/test/tools/llvm-exegesis/X86/latency/subprocess-preserved-registers.s b/llvm/test/tools/llvm-exegesis/X86/latency/subprocess-preserved-registers.s
index 320d1d7db38f422..11ae00dacda2eee 100644
--- a/llvm/test/tools/llvm-exegesis/X86/latency/subprocess-preserved-registers.s
+++ b/llvm/test/tools/llvm-exegesis/X86/latency/subprocess-preserved-registers.s
@@ -9,21 +9,27 @@
 # Check that the value of the registers preserved in subprocess mode while
 # making the ioctl system call are actually preserved correctly.
 
-# LLVM-EXEGESIS-DEFREG RAX 11
-# LLVM-EXEGESIS-DEFREG RDI 13
-# LLVM-EXEGESIS-DEFREG RSI 17
-# LLVM-EXEGESIS-DEFREG R13 0
-# LLVM-EXEGESIS-DEFREG R12 127
-
-cmpq $0x11, %rax
-cmovneq %r12, %r13
-cmpq $0x13, %rdi
-cmovneq %r12, %r13
-cmpq $0x17, %rsi
-cmovneq %r12, %r13
+# LLVM-EXEGESIS-DEFREG RAX 3
+# LLVM-EXEGESIS-DEFREG RCX 5
+# LLVM-EXEGESIS-DEFREG RDI 7
+# LLVM-EXEGESIS-DEFREG RSI B
+# LLVM-EXEGESIS-DEFREG R11 D
+# LLVM-EXEGESIS-DEFREG R14 127
+# LLVM-EXEGESIS-DEFREG R15 0
+
+cmpq $0x3, %rax
+cmovneq %r14, %r15
+cmpq $0x5, %rcx
+cmovneq %r14, %r15
+cmpq $0x7, %rdi
+cmovneq %r14, %r15
+cmpq $0xB, %rsi
+cmovneq %r14, %r15
+cmpq $0xD, %r11
+cmovneq %r14, %r15
 
 movq $60, %rax
-movq %r13, %rdi
+movq %r15, %rdi
 syscall
 
 # CHECK-NOT: error:           'Child benchmarking process exited with non-zero exit code: Child process returned with unknown exit code'
diff --git a/llvm/tools/llvm-exegesis/lib/X86/Target.cpp b/llvm/tools/llvm-exegesis/lib/X86/Target.cpp
index bf8fd9ec7066478..d025fe955be5157 100644
--- a/llvm/tools/llvm-exegesis/lib/X86/Target.cpp
+++ b/llvm/tools/llvm-exegesis/lib/X86/Target.cpp
@@ -1185,10 +1185,14 @@ std::vector<MCInst>
 ExegesisX86Target::configurePerfCounter(long Request, bool SaveRegisters) const {
   std::vector<MCInst> ConfigurePerfCounterCode;
   if(SaveRegisters) {
-    // Preservie RAX, RDI, and RSI by pushing them to the stack.
+    // Preserve RAX, RDI, and RSI by pushing them to the stack.
     generateRegisterStackPush(X86::RAX, ConfigurePerfCounterCode);
     generateRegisterStackPush(X86::RDI, ConfigurePerfCounterCode);
     generateRegisterStackPush(X86::RSI, ConfigurePerfCounterCode);
+    // RCX and R11 will get clobbered by the syscall instruction, so save them
+    // as well.
+    generateRegisterStackPush(X86::RCX, ConfigurePerfCounterCode);
+    generateRegisterStackPush(X86::R11, ConfigurePerfCounterCode);
   }
   ConfigurePerfCounterCode.push_back(
       loadImmediate(X86::RDI, 64, APInt(64, getAuxiliaryMemoryStartAddress())));
@@ -1203,6 +1207,9 @@ ExegesisX86Target::configurePerfCounter(long Request, bool SaveRegisters) const
       loadImmediate(X86::RSI, 64, APInt(64, Request)));
   generateSyscall(SYS_ioctl, ConfigurePerfCounterCode);
   if(SaveRegisters) {
+    // Restore R11 then RCX
+    generateRegisterStackPop(X86::R11, ConfigurePerfCounterCode);
+    generateRegisterStackPop(X86::RCX, ConfigurePerfCounterCode);
     // Restore RAX, RDI, and RSI, in reverse order.
     generateRegisterStackPop(X86::RSI, ConfigurePerfCounterCode);
     generateRegisterStackPop(X86::RDI, ConfigurePerfCounterCode);
@@ -1216,7 +1223,7 @@ std::vector<unsigned> ExegesisX86Target::getArgumentRegisters() const {
 }
 
 std::vector<unsigned> ExegesisX86Target::getRegistersNeedSaving() const {
-  return {X86::RAX, X86::RDI, X86::RSI};
+  return {X86::RAX, X86::RDI, X86::RSI, X86::RCX, X86::R11};
 }
 
 #endif // __linux__

@boomanaiden154 boomanaiden154 merged commit 27c9895 into llvm:main Nov 20, 2023
4 checks passed
sr-tream pushed a commit to sr-tream/llvm-project that referenced this pull request Nov 20, 2023
Currently, when making the ioctl system call, we're not preserving rcx
and r11. The system call will clobber these registers, meaning that the
values of the registers in the snippet will be different than expected.
This patch fixes that be preserving the registers around the system
call, similar to how the other registers involved in the making the
system call get preserved.

Fixes llvm#72741.
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
Currently, when making the ioctl system call, we're not preserving rcx
and r11. The system call will clobber these registers, meaning that the
values of the registers in the snippet will be different than expected.
This patch fixes that be preserving the registers around the system
call, similar to how the other registers involved in the making the
system call get preserved.

Fixes llvm#72741.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[llvm-exegesis] Values of registers r11 and rcx not set properly
3 participants