[Exegesis][RISCV] Support C_LDSP for llvm-exegesis

[sunshaoce]

Fix error:

*** Bad machine code: Illegal physical register for instruction ***
- function:    foo
- basic block: %bb.0  (0x5e2262bd3f20)
- instruction: $x10 = C_LDSP $x10, 0
- operand 1:   $x10
$x10 is not a SP register.
llvm-exegesis error: The machine function failed verification.

[llvmbot]

@llvm/pr-subscribers-backend-risc-v

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

Author: Shaoce SUN (sunshaoce)

Changes

Fix error:

*** Bad machine code: Illegal physical register for instruction ***
- function:    foo
- basic block: %bb.0  (0x5e2262bd3f20)
- instruction: $x10 = C_LDSP $x10, 0
- operand 1:   $x10
$x10 is not a SP register.
llvm-exegesis error: The machine function failed verification.

---

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

2 Files Affected:

• (modified) llvm/test/tools/llvm-exegesis/RISCV/latency-by-extension-C.s (+9)
• (modified) llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp (+8)

diff --git a/llvm/test/tools/llvm-exegesis/RISCV/latency-by-extension-C.s b/llvm/test/tools/llvm-exegesis/RISCV/latency-by-extension-C.s
index 9e94f024ed116..8189d252cd96c 100644
--- a/llvm/test/tools/llvm-exegesis/RISCV/latency-by-extension-C.s
+++ b/llvm/test/tools/llvm-exegesis/RISCV/latency-by-extension-C.s
@@ -46,3 +46,12 @@ C_SRLI-NEXT: key:
 C_SRLI-NEXT:   instructions:
 C_SRLI-NEXT:     - 'C_SRLI [[REG101:X[0-9]+]] [[REG102:X[0-9]+]] [[IMM10:i_0x[0-9]+]]'
 C_SRLI-DAG: ...
+
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux-gnu --mcpu=generic --benchmark-phase=assemble-measured-code -opcode-name=C_LDSP -mattr=+c | FileCheck --check-prefix=C_LDSP %s
+
+C_LDSP:      ---
+C_LDSP-NEXT: mode: latency
+C_LDSP-NEXT: key:
+C_LDSP-NEXT:   instructions:
+C_LDSP-NEXT:     - 'C_LDSP [[REG101:X[0-9]+]] [[REG102:X[0-9]+]] [[IMM10:i_0x[0-9]+]]'
+C_LDSP-DAG: ...
diff --git a/llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp b/llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp
index ea830bd5f753d..9460743e34ce8 100644
--- a/llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp
+++ b/llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp
@@ -819,6 +819,14 @@ void ExegesisRISCVTarget::fillMemoryOperands(InstructionTemplate &IT,
 
   assert(MemOp.isReg() && "Memory operand expected to be register");
 
+  unsigned Opcode = I.getOpcode();
+  if (Opcode == RISCV::C_LDSP || Opcode == RISCV::C_LWSP ||
+      Opcode == RISCV::C_SDSP || Opcode == RISCV::C_SWSP) {
+    // Force base register to SP (X2)
+    IT.getValueFor(MemOp) = MCOperand::createReg(RISCV::X2);
+    return;
+  }
+
   IT.getValueFor(MemOp) = MCOperand::createReg(Reg);
 }
 

[topperc]

Do we need to do the FP versions too?

[sunshaoce]

I tested some instructions, but none of them can be measured:

C_FLDSP: No strategy found to make the execution serial
C_FSDSP: No strategy found to make the execution serial
C_FLWSP: No strategy found to make the execution serial
C_FSWSP: No strategy found to make the execution serial

[mshockwave]

In C_LDSP's case, it'll try to create an alias between the destination register of the previous load to the base register of the following instruction, which is always X2. But when it comes to C_FLDSP the same trick doesn't work because now the desination register is a floating point register, which is incompatible with X2.

[topperc]

Does the rest of llvm-exegesis need to know that we're accessing stack instead of the scratch memory it set up? If the offset is too large, can't it cause the stack access to page fault?

[mshockwave]

so this will effectively generate sequence like

C_LDSP X10, X2, 0
C_LDSP X10, X2, 0
C_LDSP X10, X2, 0
...

But as you might also notice: this is not serial. Ideally it'll be great if we can generate

C_LDSP X2, X2, 0
C_LDSP X2, X2, 0
C_LDSP X2, X2, 0
...

To do this, we might need to teach it to use a different scratch memory register instead of X10.

[boomanaiden154]

Changing the scratch register should be doable (with a register to register move in the snippet setup code), but ideally we would move snippet generation involving memory operands to use memory definitions and mappings. That requires support for subprocess execution mode though, which is a larger effort.

[boomanaiden154]

Does the rest of llvm-exegesis need to know that we're accessing stack instead of the scratch memory it set up? If the offset is too large, can't it cause the stack access to page fault?

It doesn't need to "know" for the purchases of benchmarking, given it's just a load, but yes, it could definitely cause faults. The proper way to do this is to implement subprocess execution mode and then you can set the stack to a exegesis controlled section of memory.

[sunshaoce]

Thanks everyone for the guidance. I've now implemented the change to replace rd with X2. I think subprocess support can be implemented later.

[boomanaiden154]

I guess RISC-V is actually RISC so you do not get instructions like pushq or popq on X86 that also modify the value of the SP register, which is what typically makes supporting stack operations really difficult. Using the stack of the parent process is a bit of a hack though.

Overall, I think I'm probably fine with this patch. Please have someone who actually knows RISC-V sign off on this before landing.

[mshockwave]

LGTM w/ a minor comment

[mshockwave]

I guess RISC-V is actually RISC so you do not get instructions like pushq or popq

tangent to this PR: the Zcmp extension actually has stack push / pop, they're called cm.push and cm.pop