[Exegesis][RISCV] Add RISCV support for llvm-exegesis #89047

AnastasiyaChernikova · 2024-04-17T10:35:21Z

Llvm-exegesis RISCV port is a result of team effort. Below everyone involved listed.
Co-authored-by: Konstantin Vladimirov konstantin.vladimirov@syntacore.com
Co-authored-by: Dmitrii Petrov dmitrii.petrov@syntacore.com
Co-authored-by: Dmitry Bushev dmitry.bushev@syntacore.com
Co-authored-by: Mark Goncharov mark.goncharov@syntacore.com
Co-authored-by: Anastasiya Chernikova anastasiya.chernikova@syntacore.com

github-actions · 2024-04-17T10:35:39Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2024-04-17T10:36:07Z

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

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

Author: None (AnastasiyaChernikova)

Changes

Llvm-exegesis RISCV port is a result of team effort. Below everyone involved listed.
Co-authored-by: Konstantin Vladimirov <konstantin.vladimirov@syntacore.com>
Co-authored-by: Dmitrii Petrov <dmitrii.petrov@syntacore.com>
Co-authored-by: Dmitry Bushev <dmitry.bushev@syntacore.com>
Co-authored-by: Mark Goncharov <mark.goncharov@syntacore.com>
Co-authored-by: Anastasiya Chernikova <anastasiya.chernikova@syntacore.com>

Patch is 58.39 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/89047.diff

27 Files Affected:

(modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h (+3)
(modified) llvm/lib/Target/RISCV/RISCVInstrInfo.td (+2-2)
(modified) llvm/lib/Target/RISCV/RISCVInstrInfoF.td (+4)
(added) llvm/test/tools/llvm-exegesis/RISCV/latency-by-extension-A.s (+59)
(added) llvm/test/tools/llvm-exegesis/RISCV/latency-by-extension-C.s (+65)
(added) llvm/test/tools/llvm-exegesis/RISCV/latency-by-load.s (+60)
(added) llvm/test/tools/llvm-exegesis/RISCV/latency-by-opcode-name-FADD_D.s (+11)
(added) llvm/test/tools/llvm-exegesis/RISCV/latency-pre-assigned-register.s (+4)
(modified) llvm/tools/llvm-exegesis/lib/Assembler.cpp (+9-1)
(modified) llvm/tools/llvm-exegesis/lib/BenchmarkCode.h (+5)
(modified) llvm/tools/llvm-exegesis/lib/CMakeLists.txt (+3)
(modified) llvm/tools/llvm-exegesis/lib/CodeTemplate.cpp (+8)
(modified) llvm/tools/llvm-exegesis/lib/CodeTemplate.h (+6)
(modified) llvm/tools/llvm-exegesis/lib/LlvmState.cpp (+6)
(modified) llvm/tools/llvm-exegesis/lib/MCInstrDescView.cpp (+24-9)
(modified) llvm/tools/llvm-exegesis/lib/MCInstrDescView.h (+12-1)
(modified) llvm/tools/llvm-exegesis/lib/PerfHelper.h (+1-1)
(added) llvm/tools/llvm-exegesis/lib/RISCV/CMakeLists.txt (+22)
(added) llvm/tools/llvm-exegesis/lib/RISCV/RISCVCounters.cpp (+66)
(added) llvm/tools/llvm-exegesis/lib/RISCV/RISCVCounters.h (+31)
(added) llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp (+414)
(modified) llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp (+54-8)
(modified) llvm/tools/llvm-exegesis/lib/SnippetFile.cpp (+9-6)
(modified) llvm/tools/llvm-exegesis/lib/SnippetGenerator.cpp (+12-1)
(modified) llvm/tools/llvm-exegesis/lib/SnippetGenerator.h (+1)
(modified) llvm/tools/llvm-exegesis/lib/Target.h (+21)
(modified) llvm/tools/llvm-exegesis/llvm-exegesis.cpp (+53-15)

diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
index 92f405b5f6acbf..9e83add7c4c3d9 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
@@ -299,6 +299,9 @@ enum OperandType : unsigned {
   OPERAND_RVKRNUM_2_14,
   OPERAND_SPIMM,
   OPERAND_LAST_RISCV_IMM = OPERAND_SPIMM,
+  // Operand is a 3-bit rounding mode, '111' indicates FRM register.
+  // Represents 'frm' argument passing to floating-point operations.
+  OPERAND_FRMARG,
   // Operand is either a register or uimm5, this is used by V extension pseudo
   // instructions to represent a value that be passed as AVL to either vsetvli
   // or vsetivli.
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.td b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
index f9dadc6c0d4895..dddbeeddffa5b4 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -509,7 +509,7 @@ class BranchCC_rri<bits<3> funct3, string opcodestr>
   let isTerminator = 1;
 }
 
-let hasSideEffects = 0, mayLoad = 1, mayStore = 0 in {
+let hasSideEffects = 0, mayLoad = 1, mayStore = 0, UseNamedOperandTable = 1 in {
 class Load_ri<bits<3> funct3, string opcodestr>
     : RVInstI<funct3, OPC_LOAD, (outs GPR:$rd), (ins GPRMem:$rs1, simm12:$imm12),
               opcodestr, "$rd, ${imm12}(${rs1})">;
@@ -524,7 +524,7 @@ class HLoad_r<bits<7> funct7, bits<5> funct5, string opcodestr>
 // Operands for stores are in the order srcreg, base, offset rather than
 // reflecting the order these fields are specified in the instruction
 // encoding.
-let hasSideEffects = 0, mayLoad = 0, mayStore = 1 in {
+let hasSideEffects = 0, mayLoad = 0, mayStore = 1, UseNamedOperandTable = 1 in {
 class Store_rri<bits<3> funct3, string opcodestr>
     : RVInstS<funct3, OPC_STORE, (outs),
               (ins GPR:$rs2, GPRMem:$rs1, simm12:$imm12),
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoF.td b/llvm/lib/Target/RISCV/RISCVInstrInfoF.td
index 7d89608de1223f..846f90e83df809 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoF.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoF.td
@@ -130,6 +130,8 @@ def frmarg : Operand<XLenVT> {
   let ParserMatchClass = FRMArg;
   let PrintMethod = "printFRMArg";
   let DecoderMethod = "decodeFRMArg";
+  let OperandType = "OPERAND_FRMARG";
+  let OperandNamespace = "RISCVOp";
 }
 
 // Variants of the rounding mode operand that default to 'rne'. This is used
@@ -150,6 +152,8 @@ def frmarglegacy : Operand<XLenVT> {
   let ParserMatchClass = FRMArgLegacy;
   let PrintMethod = "printFRMArgLegacy";
   let DecoderMethod = "decodeFRMArg";
+  let OperandType = "OPERAND_FRMARG";
+  let OperandNamespace = "RISCVOp";
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/llvm/test/tools/llvm-exegesis/RISCV/latency-by-extension-A.s b/llvm/test/tools/llvm-exegesis/RISCV/latency-by-extension-A.s
new file mode 100644
index 00000000000000..e07452814c9096
--- /dev/null
+++ b/llvm/test/tools/llvm-exegesis/RISCV/latency-by-extension-A.s
@@ -0,0 +1,59 @@
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=AMOAND_D -mattr="+a" |& FileCheck --check-prefix=TEST1 %s
+
+TEST1:      ---
+TEST1-NEXT: mode: latency
+TEST1-NEXT: key:
+TEST1-NEXT:   instructions:
+TEST1-NEXT:     - 'AMOAND_D [[RE01:X[0-9]+]] X10 [[RE01:X[0-9]+]]'
+TEST1-NEXT: config: ''
+TEST1-NEXT: register_initial_values:
+TEST1-NEXT: - '[[RE01:X[0-9]+]]=0x0'
+TEST1-LAST: ...
+
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=AMOADD_W -mattr="+a" |& FileCheck --check-prefix=TEST2 %s
+
+TEST2:      ---
+TEST2-NEXT: mode: latency
+TEST2-NEXT: key:
+TEST2-NEXT:   instructions:
+TEST2-NEXT:     - 'AMOADD_W [[RE02:X[0-9]+]] X10 [[RE02:X[0-9]+]]'
+TEST2-NEXT: config: ''
+TEST2-NEXT: register_initial_values:
+TEST2-NEXT: - '[[RE02:X[0-9]+]]=0x0'
+TEST2-LAST: ...
+
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=AMOMAXU_D -mattr="+a" |& FileCheck --check-prefix=TEST3 %s
+
+TEST3:      ---
+TEST3-NEXT: mode: latency
+TEST3-NEXT: key:
+TEST3-NEXT:   instructions:
+TEST3-NEXT:     - 'AMOMAXU_D [[RE03:X[0-9]+]] X10 [[RE03:X[0-9]+]]'
+TEST3-NEXT: config: ''
+TEST3-NEXT: register_initial_values:
+TEST3-NEXT: - '[[RE03:X[0-9]+]]=0x0'
+TEST3-LAST: ...
+
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=AMOMIN_W -mattr="+a" |& FileCheck --check-prefix=TEST4 %s
+
+TEST4:      ---
+TEST4-NEXT: mode: latency
+TEST4-NEXT: key:
+TEST4-NEXT:   instructions:
+TEST4-NEXT:     - 'AMOMIN_W [[RE04:X[0-9]+]] X10 [[RE04:X[0-9]+]]'
+TEST4-NEXT: config: ''
+TEST4-NEXT: register_initial_values:
+TEST4-NEXT: - '[[RE04:X[0-9]+]]=0x0'
+TEST4-LAST: ...
+
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=AMOXOR_D -mattr="+a" |& FileCheck --check-prefix=TEST5 %s
+
+TEST5:      ---
+TEST5-NEXT: mode: latency
+TEST5-NEXT: key:
+TEST5-NEXT:   instructions:
+TEST5-NEXT:     - 'AMOXOR_D [[RE05:X[0-9]+]] X10 [[RE05:X[0-9]+]]'
+TEST5-NEXT: config: ''
+TEST5-NEXT: register_initial_values:
+TEST5-NEXT: - '[[RE05:X[0-9]+]]=0x0'
+TEST5-LAST: ...
\ No newline at end of file
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
new file mode 100644
index 00000000000000..14a517750f3462
--- /dev/null
+++ b/llvm/test/tools/llvm-exegesis/RISCV/latency-by-extension-C.s
@@ -0,0 +1,65 @@
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=C_ADDI -mattr=+c |& FileCheck --check-prefix=TEST1 %s
+
+TEST1:      ---
+TEST1-NEXT: mode: latency
+TEST1-NEXT: key:
+TEST1-NEXT:   instructions:
+TEST1-NEXT:     - 'C_ADDI [[REG01:X[0-9]+]] [[RE02:X[0-9]+]] [[IMM0:i_0x[0-9]+]]'
+
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=C_ADDIW -mattr=+c |& FileCheck --check-prefix=TEST2 %s
+
+TEST2:      ---
+TEST2-NEXT: mode: latency
+TEST2-NEXT: key:
+TEST2-NEXT:   instructions:
+TEST2-NEXT:     - 'C_ADDIW [[REG11:X[0-9]+]] [[RE12:X[0-9]+]] [[IMM1:i_0x[0-9]+]]'
+
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=C_ANDI -mattr=+c |& FileCheck --check-prefix=TEST3 %s
+
+TEST3:      ---
+TEST3-NEXT: mode: latency
+TEST3-NEXT: key:
+TEST3-NEXT:   instructions:
+TEST3-NEXT:     - 'C_ANDI [[REG31:X[0-9]+]] [[REG32:X[0-9]+]] [[IMM3:i_0x[0-9]+]]'
+
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=C_SLLI -mattr=+c |& FileCheck --check-prefix=TEST4 %s
+
+TEST4:      ---
+TEST4-NEXT: mode: latency
+TEST4-NEXT: key:
+TEST4-NEXT:   instructions:
+TEST4-NEXT:     - 'C_SLLI [[REG81:X[0-9]+]] [[REG82:X[0-9]+]] [[IMM8:i_0x[0-9]+]]'
+
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=C_SRAI -mattr=+c |& FileCheck --check-prefix=TEST5 %s
+
+TEST5:      ---
+TEST5-NEXT: mode: latency
+TEST5-NEXT: key:
+TEST5-NEXT:   instructions:
+TEST5-NEXT:     - 'C_SRAI [[REG91:X[0-9]+]] [[REG92:X[0-9]+]] [[IMM9:i_0x[0-9]+]]'
+
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=C_SRLI -mattr=+c |& FileCheck --check-prefix=TEST6 %s
+
+TEST6:      ---
+TEST6-NEXT: mode: latency
+TEST6-NEXT: key:
+TEST6-NEXT:   instructions:
+TEST6-NEXT:     - 'C_SRLI [[REG101:X[0-9]+]] [[REG102:X[0-9]+]] [[IMM10:i_0x[0-9]+]]'
+TEST6-LAST: ...
+
+
+# RUN: llvm-exegesis  -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=C_LD -mattr=+c |& FileCheck --check-prefix=TEST7 %s
+
+TEST7:      ---
+TEST7-NEXT: mode: latency
+TEST7-NEXT: key:
+TEST7-NEXT:   instructions:
+TEST7-NEXT:     - 'C_LD [[REG61:X[0-9]+]] [[REG62:X[0-9]+]] [[IMM6:i_0x[0-9]+]]'
+
+# RUN: llvm-exegesis  -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=C_LW -mattr=+c |& FileCheck --check-prefix=TEST8 %s
+
+TEST8:      ---
+TEST8-NEXT: mode: latency
+TEST8-NEXT: key:
+TEST8-NEXT:   instructions:
+TEST8-NEXT:     - 'C_LW [[REG71:X[0-9]+]] [[REG72:X[0-9]+]] [[IMM7:i_0x[0-9]+]]'
diff --git a/llvm/test/tools/llvm-exegesis/RISCV/latency-by-load.s b/llvm/test/tools/llvm-exegesis/RISCV/latency-by-load.s
new file mode 100644
index 00000000000000..74ee7580c6054c
--- /dev/null
+++ b/llvm/test/tools/llvm-exegesis/RISCV/latency-by-load.s
@@ -0,0 +1,60 @@
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=LD |& FileCheck --check-prefix=TEST1 %s
+
+TEST1:      ---
+TEST1-NEXT: mode: latency
+TEST1-NEXT: key:
+TEST1-NEXT:   instructions:
+TEST1-NEXT:     - 'LD X10 X10 i_0x0'
+
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=LW |& FileCheck --check-prefix=TEST2 %s
+
+TEST2:      ---
+TEST2-NEXT: mode: latency
+TEST2-NEXT: key:
+TEST2-NEXT:   instructions:
+TEST2-NEXT:     - 'LW X10 X10 i_0x0'
+
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=LH |& FileCheck --check-prefix=TEST3 %s
+
+TEST3:      ---
+TEST3-NEXT: mode: latency
+TEST3-NEXT: key:
+TEST3-NEXT:   instructions:
+TEST3-NEXT:     - 'LH X10 X10 i_0x0'
+
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=LWU |& FileCheck --check-prefix=TEST4 %s
+
+TEST4:      ---
+TEST4-NEXT: mode: latency
+TEST4-NEXT: key:
+TEST4-NEXT:   instructions:
+TEST4-NEXT:     - 'LWU X10 X10 i_0x0'
+
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=LBU |& FileCheck --check-prefix=TEST5 %s
+
+TEST5:      ---
+TEST5-NEXT: mode: latency
+TEST5-NEXT: key:
+TEST5-NEXT:   instructions:
+TEST5-NEXT:     - 'LBU X10 X10 i_0x0'
+
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=LUI |& FileCheck --check-prefix=TEST6 %s
+
+TEST6: LUI: No strategy found to make the execution serial
+
+
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=LB |& FileCheck --check-prefix=TEST7 %s
+
+TEST7:      ---
+TEST7-NEXT: mode: latency
+TEST7-NEXT: key:
+TEST7-NEXT:   instructions:
+TEST7-NEXT:     - 'LB X10 X10 i_0x0'
+
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux --benchmark-phase=assemble-measured-code -opcode-name=LR_W_RL -mattr="+a" |& FileCheck --check-prefix=TEST8 %s
+
+TEST8:      ---
+TEST8-NEXT: mode: latency
+TEST8-NEXT: key:
+TEST8-NEXT:   instructions:
+TEST8-NEXT:     - 'LR_W_RL X10 X10'
\ No newline at end of file
diff --git a/llvm/test/tools/llvm-exegesis/RISCV/latency-by-opcode-name-FADD_D.s b/llvm/test/tools/llvm-exegesis/RISCV/latency-by-opcode-name-FADD_D.s
new file mode 100644
index 00000000000000..b6ee3266cae6ca
--- /dev/null
+++ b/llvm/test/tools/llvm-exegesis/RISCV/latency-by-opcode-name-FADD_D.s
@@ -0,0 +1,11 @@
+# RUN: llvm-exegesis -mtriple=riscv64-unknown-linux -mode=latency --benchmark-phase=assemble-measured-code -mattr=+d -opcode-name=FADD_D |& FileCheck %s
+
+CHECK:      ---
+CHECK-NEXT: mode: latency
+CHECK-NEXT: key:
+CHECK-NEXT:   instructions:
+CHECK-NEXT:     - 'FADD_D [[REG1:F[0-9]+_D]] [[REG2:F[0-9]+_D]] [[REG3:F[0-9]+_D]] i_0x7'
+CHECK-NEXT: config: ''
+CHECK-NEXT: register_initial_values:
+CHECK-DAG: - '[[REG1]]=0x0'
+CHECK-LAST: ...
diff --git a/llvm/test/tools/llvm-exegesis/RISCV/latency-pre-assigned-register.s b/llvm/test/tools/llvm-exegesis/RISCV/latency-pre-assigned-register.s
new file mode 100644
index 00000000000000..f0a114f648fa24
--- /dev/null
+++ b/llvm/test/tools/llvm-exegesis/RISCV/latency-pre-assigned-register.s
@@ -0,0 +1,4 @@
+RUN: llvm-exegesis -mode=latency --benchmark-phase=assemble-measured-code -opcode-name=LB -mtriple=riscv64-unknown-linux-gnu
+
+CHECK: Warning: Pre-assigned register prevented usage of self-aliasing strategy.
+
diff --git a/llvm/tools/llvm-exegesis/lib/Assembler.cpp b/llvm/tools/llvm-exegesis/lib/Assembler.cpp
index 92ab3a96d91e6b..2908ea80d0e471 100644
--- a/llvm/tools/llvm-exegesis/lib/Assembler.cpp
+++ b/llvm/tools/llvm-exegesis/lib/Assembler.cpp
@@ -168,7 +168,15 @@ void BasicBlockFiller::addInstruction(const MCInst &Inst, const DebugLoc &DL) {
     } else if (Op.isImm()) {
       Builder.addImm(Op.getImm());
     } else if (!Op.isValid()) {
-      llvm_unreachable("Operand is not set");
+      std::string Message;
+      llvm::raw_string_ostream MessageOut(Message);
+      MessageOut << "Operand is not set: Instr: ";
+      Inst.dump_pretty(MessageOut, MCII->getName(Inst.getOpcode()));
+      MessageOut << "; OpIndex: " << OpIndex;
+      const MCOperandInfo &OperandInfo = MCID.operands()[OpIndex];
+      MessageOut << "; OpInfo.OperandType: "
+                 << static_cast<uint32_t>(OperandInfo.OperandType);
+      report_fatal_error(Twine(Message));
     } else {
       llvm_unreachable("Not yet implemented");
     }
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkCode.h b/llvm/tools/llvm-exegesis/lib/BenchmarkCode.h
index 1db8472e99f7c9..4d5fc241fde0d9 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkCode.h
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkCode.h
@@ -17,6 +17,11 @@
 namespace llvm {
 namespace exegesis {
 
+struct ScratchMemoryStore {
+  unsigned Reg;
+  unsigned Offset;
+};
+
 // A collection of instructions that are to be assembled, executed and measured.
 struct BenchmarkCode {
   BenchmarkKey Key;
diff --git a/llvm/tools/llvm-exegesis/lib/CMakeLists.txt b/llvm/tools/llvm-exegesis/lib/CMakeLists.txt
index 414b49e5e021c2..bb201ed89e6c57 100644
--- a/llvm/tools/llvm-exegesis/lib/CMakeLists.txt
+++ b/llvm/tools/llvm-exegesis/lib/CMakeLists.txt
@@ -12,6 +12,9 @@ endif()
 if (LLVM_TARGETS_TO_BUILD MATCHES "Mips")
   list(APPEND LLVM_EXEGESIS_TARGETS "Mips")
 endif()
+if(LLVM_TARGETS_TO_BUILD MATCHES "RISCV" OR LLVM_EXPERIMENTAL_TARGETS_TO_BUILD MATCHES "RISCV")
+  list(APPEND LLVM_EXEGESIS_TARGETS "RISCV")
+endif()
 
 set(LLVM_EXEGESIS_TARGETS ${LLVM_EXEGESIS_TARGETS} PARENT_SCOPE)
 
diff --git a/llvm/tools/llvm-exegesis/lib/CodeTemplate.cpp b/llvm/tools/llvm-exegesis/lib/CodeTemplate.cpp
index fd156ee01e7ce5..9b5bfe3196dcda 100644
--- a/llvm/tools/llvm-exegesis/lib/CodeTemplate.cpp
+++ b/llvm/tools/llvm-exegesis/lib/CodeTemplate.cpp
@@ -57,6 +57,14 @@ const MCOperand &InstructionTemplate::getValueFor(const Operand &Op) const {
   return getValueFor(Instr->Variables[Op.getVariableIndex()]);
 }
 
+MCOperand &InstructionTemplate::getValueFor(unsigned OpIdx) {
+  return getValueFor(Instr->Variables[OpIdx]);
+}
+
+const MCOperand &InstructionTemplate::getValueFor(unsigned OpIdx) const {
+  return getValueFor(Instr->Variables[OpIdx]);
+}
+
 bool InstructionTemplate::hasImmediateVariables() const {
   return any_of(Instr->Variables, [this](const Variable &Var) {
     return Instr->getPrimaryOperand(Var).isImmediate();
diff --git a/llvm/tools/llvm-exegesis/lib/CodeTemplate.h b/llvm/tools/llvm-exegesis/lib/CodeTemplate.h
index 7aca224302a1ff..85fcb4e908f017 100644
--- a/llvm/tools/llvm-exegesis/lib/CodeTemplate.h
+++ b/llvm/tools/llvm-exegesis/lib/CodeTemplate.h
@@ -35,6 +35,8 @@ struct InstructionTemplate {
   const MCOperand &getValueFor(const Variable &Var) const;
   MCOperand &getValueFor(const Operand &Op);
   const MCOperand &getValueFor(const Operand &Op) const;
+  MCOperand &getValueFor(unsigned OpIdx);
+  const MCOperand &getValueFor(unsigned OpIdx) const;
   bool hasImmediateVariables() const;
   const Instruction &getInstr() const { return *Instr; }
   ArrayRef<MCOperand> getVariableValues() const { return VariableValues; }
@@ -133,6 +135,10 @@ struct CodeTemplate {
   // the pointer to this memory is passed in to the function.
   unsigned ScratchSpacePointerInReg = 0;
 
+  // Require to pre-store value of a given register (fisrt)
+  // to scratch memory with given offset (second)
+  SmallVector<std::pair<unsigned, unsigned>, 2> PreinitScratchMemory;
+
 #if defined(__GNUC__) && (defined(__clang__) || LLVM_GNUC_PREREQ(8, 0, 0))
   // FIXME: GCC7 bug workaround. Drop #if after GCC7 no longer supported.
 private:
diff --git a/llvm/tools/llvm-exegesis/lib/LlvmState.cpp b/llvm/tools/llvm-exegesis/lib/LlvmState.cpp
index 17d09a1ec0cf34..d387251a687e6a 100644
--- a/llvm/tools/llvm-exegesis/lib/LlvmState.cpp
+++ b/llvm/tools/llvm-exegesis/lib/LlvmState.cpp
@@ -45,6 +45,12 @@ Expected<LLVMState> LLVMState::Create(std::string TripleName,
   if (CpuName == "native")
     CpuName = std::string(sys::getHostCPUName());
 
+  if (CpuName.empty()) {
+    std::unique_ptr<MCSubtargetInfo> Empty_STI(
+        TheTarget->createMCSubtargetInfo(TripleName, "", ""));
+    CpuName = Empty_STI->getAllProcessorDescriptions().begin()->Key;
+  }
+
   std::unique_ptr<MCSubtargetInfo> STI(
       TheTarget->createMCSubtargetInfo(TripleName, CpuName, ""));
   assert(STI && "Unable to create subtarget info!");
diff --git a/llvm/tools/llvm-exegesis/lib/MCInstrDescView.cpp b/llvm/tools/llvm-exegesis/lib/MCInstrDescView.cpp
index 9c926d1fc61124..7705d9af4057e8 100644
--- a/llvm/tools/llvm-exegesis/lib/MCInstrDescView.cpp
+++ b/llvm/tools/llvm-exegesis/lib/MCInstrDescView.cpp
@@ -89,17 +89,17 @@ const BitVector *BitVectorCache::getUnique(BitVector &&BV) const {
   return Entry.get();
 }
 
-Instruction::Instruction(const MCInstrDesc *Description, StringRef Name,
-                         SmallVector<Operand, 8> Operands,
-                         SmallVector<Variable, 4> Variables,
-                         const BitVector *ImplDefRegs,
-                         const BitVector *ImplUseRegs,
-                         const BitVector *AllDefRegs,
-                         const BitVector *AllUseRegs)
+Instruction::Instruction(
+    const MCInstrDesc *Description, StringRef Name,
+    SmallVector<Operand, 8> Operands, SmallVector<Variable, 4> Variables,
+    const BitVector *ImplDefRegs, const BitVector *ImplUseRegs,
+    const BitVector *AllDefRegs, const BitVector *AllUseRegs,
+    const BitVector *MemoryRegs, const BitVector *NotMemoryRegs)
     : Description(*Description), Name(Name), Operands(std::move(Operands)),
       Variables(std::move(Variables)), ImplDefRegs(*ImplDefRegs),
       ImplUseRegs(*ImplUseRegs), AllDefRegs(*AllDefRegs),
-      AllUseRegs(*AllUseRegs) {}
+      AllUseRegs(*AllUseRegs), MemoryRegs(*MemoryRegs),
+      NotMemoryRegs(*NotMemoryRegs) {}
 
 std::unique_ptr<Instruction>
 Instruction::create(const MCInstrInfo &InstrInfo,
@@ -166,6 +166,9 @@ Instruction::create(const MCInstrInfo &InstrInfo,
   BitVector ImplUseRegs = RATC.emptyRegisters();
   BitVector AllDefRegs = RATC.emptyRegisters();
   BitVector AllUseRegs = RATC.emptyRegisters();
+  BitVector MemoryRegs = RATC.emptyRegisters();
+  BitVector NotMemoryRegs = RATC.emptyRegisters();
+
   for (const auto &Op : Operands) {
     if (Op.isReg()) {
       const auto &AliasingBits = Op.getRegisterAliasing().aliasedBits();
@@ -177,6 +180,10 @@ Instruction::create(const MCInstrInfo &InstrInfo,
         ImplDefRegs |= AliasingBits;
       if (Op.isUse() && Op.isImplicit())
         ImplUseRegs |= AliasingBits;
+      if (Op.isUse() && Op.isMemory())
+        MemoryRegs |= AliasingBits;
+      if (Op.isUse() && !Op.isMemory())
+        NotMemoryRegs |= AliasingBits;
     }
   }
   // Can't use make_unique because constructor is private.
@@ -185,7 +192,9 @@ Instruction::create(const MCInstrInfo &InstrInfo,
       std::move(Variables), BVC.getUnique(std::move(ImplDefRegs)),
       BVC.getUnique(std::move(ImplUseRegs)),
       BVC.getUnique(std::move(AllDefRegs)),
-      BVC.getUnique(std::move(AllUseRegs))));
+      BVC.getUnique(std::move(AllUseRegs)),
+      BVC.getUnique(std::move(MemoryRegs)),
+      BVC.getUnique(std::move(NotMemoryRegs))));
 }
 
 const Operand &Instruction::getPrimaryOperand(const Variable &Var) const {
@@ -240,6 +249,12 @@ bool Instruction::hasAliasingRegisters(
              ...
[truncated]

github-actions · 2024-04-17T13:04:05Z

✅ With the latest revision this PR passed the C/C++ code formatter.

mshockwave · 2024-04-17T16:22:47Z

llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp

+namespace {
+
+static cl::OptionCategory
+    RISCVBenchmarkOptions("llvm-exegesis benchmark RISCV options");


Is this used anywhere?

Now it really isn't used. We made it with expansion in mind. Deleted in this commit

mshockwave · 2024-04-17T16:25:23Z

llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp

+static std::vector<MCInst> loadFPRegBits(const MCSubtargetInfo &STI,
+                                         unsigned Reg, const APInt &Bits,
+                                         unsigned FmvOpcode) {
+  std::vector<MCInst> Instrs = loadIntReg(STI, ScratchIntReg, Bits);


Could we use FLI instruction here when possible?

Nowadays there are a few common used RISC-V boards which support Zfa extension. Thanks for your suggestion, I think that this instruction usage also could be added with a separate patch

mshockwave · 2024-04-17T16:31:50Z

llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp

+      Instr.getPrimaryOperand(Var).getExplicitOperandInfo().OperandType;
+
+  switch (OperandType) {
+  case RISCVOp::OPERAND_FRMARG:


I think we can use RISCV::getNamedOperandIdx(<opcode>, RISCV::OpName::frm) to get the operand index of rounding mode operand, rather than creating a new operand type.

I am not sure how it would help here. In this function we don't need an operand index. What we need is a legal value for specified MCOperand.

The rationale behind my suggestion is that I'm still not really convinced we should add a new OPERAND_FRMARG operand type to RISC-V. As we have always been able to locate rounding mode operand with RISCV::getNamedOperandIdx. Therefore, I think we can get the index of exegesis::Operand we're dealing here through Instr.getPrimaryOperand(Var) and check if that index matches what returned from RISCV::getNamedOperandIdx to know if it's a rounding mode operand.

In this case, it seems to me that it is unnecessary. Now the code has one structure and it is clear which cases are processed. In case of replacement, we will have a separate branch for frm operands, which will make the code a bit less readable

mshockwave · 2024-04-17T16:35:22Z

llvm/tools/llvm-exegesis/llvm-exegesis.cpp

@@ -235,7 +237,7 @@ static cl::opt<std::string>
 static cl::opt<std::string>
    MCPU("mcpu",
         cl::desc("Target a specific cpu type (-mcpu=help for details)"),
-         cl::value_desc("cpu-name"), cl::cat(Options), cl::init("native"));
+         cl::value_desc("cpu-name"), cl::cat(Options), cl::init(""));


I kind of knowing why you made this change here, but I think it's better to leave it native.

mshockwave · 2024-04-17T16:37:31Z

llvm/tools/llvm-exegesis/llvm-exegesis.cpp

@@ -291,6 +297,22 @@ T ExitOnFileError(const Twine &FileName, Expected<T> &&E) {
  return std::move(*E);
 }

+static const char *getIgnoredOpcodeReasonOrNull(const LLVMState &State,


Is there any reason to duplicating this checking logics here? I thought we already done it somewhere in the snippet generator.

I can't find a function with similar logic. Can you please tell me its name?

Here:

llvm-project/llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp

Line 58 in 7964356

if (OtherInstrDesc.isPseudo() || OtherInstrDesc.usesCustomInsertionHook() ||

Removed this check from tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp, since the program always first goes to generateSnippets in the file tools/llvm-exegesis/llvm-exegesis.cpp, where the same check already done and returns an error if it fails

mshockwave · 2024-04-17T16:46:26Z

llvm/tools/llvm-exegesis/llvm-exegesis.cpp

-    else
+      continue;
+    }
+    // Not a known opcode name; should be an opcode name range.


Could we implement the parsing logics here as a custom command line parser for OpcodeName? https://llvm.org/docs/CommandLine.html#writing-a-custom-parser

topperc · 2024-04-17T16:34:00Z

llvm/tools/llvm-exegesis/lib/CMakeLists.txt

@@ -12,6 +12,9 @@ endif()
 if (LLVM_TARGETS_TO_BUILD MATCHES "Mips")
  list(APPEND LLVM_EXEGESIS_TARGETS "Mips")
 endif()
+if(LLVM_TARGETS_TO_BUILD MATCHES "RISCV" OR LLVM_EXPERIMENTAL_TARGETS_TO_BUILD MATCHES "RISCV")


Why would RISCV be in LLVM_EXPERIMENTAL_TARGETS_TO_BUILD? It's not an experimental target.

topperc · 2024-04-17T16:37:56Z

llvm/lib/Target/RISCV/RISCVInstrInfo.td

@@ -509,7 +509,7 @@ class BranchCC_rri<bits<3> funct3, string opcodestr>
  let isTerminator = 1;
 }

-let hasSideEffects = 0, mayLoad = 1, mayStore = 0 in {
+let hasSideEffects = 0, mayLoad = 1, mayStore = 0, UseNamedOperandTable = 1 in {


What is this needed for?

'UseNamedOperandTable = 1' builds a table that's used to find instruction operand by its name in td

topperc · 2024-04-17T16:38:05Z

llvm/lib/Target/RISCV/RISCVInstrInfo.td

@@ -524,7 +524,7 @@ class HLoad_r<bits<7> funct7, bits<5> funct5, string opcodestr>
 // Operands for stores are in the order srcreg, base, offset rather than
 // reflecting the order these fields are specified in the instruction
 // encoding.
-let hasSideEffects = 0, mayLoad = 0, mayStore = 1 in {
+let hasSideEffects = 0, mayLoad = 0, mayStore = 1, UseNamedOperandTable = 1 in {


What is this needed for?

'UseNamedOperandTable = 1' builds a table that's used to find instruction operand by its name in td

topperc · 2024-04-17T16:41:21Z

llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp

+  std::vector<MCInst> MatIntInstrs;
+  MatIntInstrs.reserve(InstSeq.size());
+  for (const RISCVMatInt::Inst &Inst : InstSeq) {
+    if (Inst.getOpcode() == RISCV::LUI) {


Use a switch over getKind() instead of an if/else chain based on opcode. That will give us a build warning if a new kind is added.

topperc · 2024-04-17T16:44:20Z

llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp

+  auto &I = IT.getInstr();
+
+  auto MemOpIt =
+      find_if(I.Operands, [](Operand const &op) { return op.isMemory(); });


Capitalize variable names

topperc · 2024-04-17T16:52:16Z

llvm/tools/llvm-exegesis/lib/LlvmState.cpp

+  if (CpuName.empty()) {
+    std::unique_ptr<MCSubtargetInfo> Empty_STI(
+        TheTarget->createMCSubtargetInfo(TripleName, "", ""));
+    CpuName = Empty_STI->getAllProcessorDescriptions().begin()->Key;


This behavior seems weird. Is this guaranteed to be a basic CPU or could it be one with lots of features?

It's not even guaranteed that we can run code on the host when jitting for this first CPU, so it feels weird to automatically use this.

Removed this code

topperc · 2024-04-17T16:56:11Z

llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp

+  case RISCV::C_ADDI_NOP:
+  case RISCV::C_ADD_HINT:
+  case RISCV::C_SLLI_HINT:
+    AssignedValue = MCOperand::createReg(RISCV::X0);


Why does this need special cases when they already have a register class that says the register must be X0?

In comment above the function described, that some function from RISC-V C extension require special registers, registers for such instructions described in register class, but X0 - reserved register (set in RISCVRegisterInfo.cpp), and here we need to set it explicitly.

Can you use the register class information to detect that instead of hardcoding a list of instructions? Why are only some of the _HINT instructions here? Should C_LUI_HINT, C_LI_HINT, C_MV_HINT be listed here?

The process of determining the operand for instructions generally occurs in the randomizeMCOperand function. This function receives ForbiddenRegs, common to all platform instructions. The list of allowed registers (AllowedRegs) excludes ForbiddenRegs. X0 belongs to ForbiddenRegs (since it is a special register), so it can never be used in generation. In the case of HINT instructions, we use non-standard operands that are generally prohibited. Therefore, it is necessary to set operands for these instructions in a special function.
Added processing of additional cases from llvm/lib/Target/RISCV/RISCVInstrInfoC.td

boomanaiden154

This looks exciting. Thanks for putting patches up.

A couple comments.

llvm/test/tools/llvm-exegesis/RISCV/latency-by-extension-A.s

llvm/test/tools/llvm-exegesis/RISCV/latency-by-load.s

boomanaiden154 · 2024-04-18T09:01:04Z

llvm/tools/llvm-exegesis/lib/Assembler.cpp

@@ -168,7 +168,15 @@ void BasicBlockFiller::addInstruction(const MCInst &Inst, const DebugLoc &DL) {
    } else if (Op.isImm()) {
      Builder.addImm(Op.getImm());
    } else if (!Op.isValid()) {
-      llvm_unreachable("Operand is not set");
+      std::string Message;


This addition here looks unrelated (directly at least) to the RISCV support? Maybe pull this out into a separate patch?

I agree with you, thanks. Addressed

boomanaiden154 · 2024-04-18T09:05:03Z

llvm/tools/llvm-exegesis/lib/SnippetGenerator.cpp

-      assert(AssignedValue.isReg() && AssignedValue.getReg() == ROV.Reg);
+      // TODO don't re-assign register operands which are already "locked"
+      //  by Target in corresponding InstructionTemplate
+      // assert(AssignedValue.isReg() && AssignedValue.getReg() == ROV.Reg);


The TODO here should be fine, but leaving the assert in as a comment is odd. It should probably be removed with additional information added to the todo if necessary.

boomanaiden154 · 2024-04-18T09:09:02Z

llvm/tools/llvm-exegesis/lib/SnippetFile.cpp

@@ -207,14 +208,16 @@ class BenchmarkCodeStreamer : public MCStreamer, public AsmCommentConsumer {
                    Align ByteAlignment, SMLoc Loc) override {}

  unsigned findRegisterByName(const StringRef RegName) const {
+    if (unsigned Reg = Target.findRegisterByName(RegName))
+      return Reg;


Why is this just tacked on here? If there's registers missing, they should probably be added to the RegNameToRegNo map by modifying LlvmState::createRegNameToRegNoMapping. If this works better and covers all/more cases though, it might be something we want to do cleanup on, but that can probably be done in a separate PR along with the additional cleanup.

That's a good idea, thanks for the suggestion. But I propose to make this in a separate pull request, since it does not provide functionality now.

boomanaiden154 · 2024-04-18T09:10:25Z

llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp

+static cl::OptionCategory
+    RISCVBenchmarkOptions("llvm-exegesis benchmark RISCV options");
+
+// TODO move perf counter data to td files (although it looks like an overkill


This should probably be done rather than leaving it as a TODO just so that all the targets are consistent.

This should probably be done rather than leaving it as a TODO just so that all the targets are consistent.

Second this, I think you only need to add a couple lines.

boomanaiden154 · 2024-04-18T09:12:52Z

llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp

+  if (CounterName == RISCVPfmCounterNames[0]) {
+    return createRISCVCpuCyclesCounter(pfm::PerfEvent(CounterName));
+  }
+  return make_error<Failure>(Twine("Unsupported performance counter '")


Some of this code seems duplicated with ExegesisTarget::createCounter?

We create counter in specific way createRISCVCpuCyclesCounter(pfm::PerfEvent(CounterName));. So we need to leave it here

boomanaiden154 · 2024-04-18T09:16:06Z

llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp

+    StringRef CounterName, const LLVMState &State,
+    ArrayRef<const char *> ValidationCounters, const pid_t ProcessID) const {
+  if (CounterName == RISCVPfmCounterNames[0]) {
+    return createRISCVCpuCyclesCounter(pfm::PerfEvent(CounterName));


I'm not too familiar with perf support on RISCV (so please correct me if I'm mistaken here), but it seems like some things (like a cycles counter) might be supported? It might be reasonable to allow for using the perf subsystem in addition to the custom counter similar to how X86 supports the perf subsystem and LBR.

I also think we should try to use as much Linux Perf as possible. Granted, reading mcycle from CSR register is more generic, but llvm-exegesis already puts an assumption that it has to run on Linux so we might as well just use Perf. Libpfm hasn't supported RISC-V yet but we can use Perf to read the CYCLES event.

We'd still have to go through libpfm to get the event ID/description to do a call to perf_event_open with how things are structured currently, but there should be generic Linux perf events that can be used (see here). Probably will want to run this example though to see what events are there as it isn't super clear from the header files.

Thanks for the offer. We will definitely look into this, but I suggest putting it in a separate pool request. Since it does not violate the functionality of exegesis, and the changes are significant.

boomanaiden154 · 2024-04-18T09:17:44Z

llvm/tools/llvm-exegesis/lib/RISCV/RISCVCounters.cpp

+  EndValue = getRISCVCpuCyclesCount();
+  MeasurementCycles = EndValue - StartValue;
+  if (MeasurementCycles == 0) {
+    report_fatal_error("MeasurementCycles == 0, "


Maybe add a comment here explaining this check? Not sure it's super obvious from first glance.

boomanaiden154 · 2024-04-18T09:19:21Z

llvm/tools/llvm-exegesis/lib/RISCV/RISCVCounters.cpp

+
+Expected<SmallVector<int64_t, 4>>
+RISCVCpuCyclesCounter::readOrError(StringRef FunctionBytes) const {
+  uint64_t Counter = EndValue - StartValue - MeasurementCycles;


I would warn against subtracting MeasurementCycles from the counter value. There are going to be quite a few overheads that aren't really avoidable within llvm-exegesis, and trying to subtract them out in places like this doesn't really seem ideal to me. To subtract out all differences during a measurement, we have the middle-half-duplicate and middle-half-loop repetition modes that are able to systematically subtract out overheads and anecdotally work quite well.

It seems like not so huge overhead, but it also can give us more precision during measurements, so subtracting cost looks affordable for improved precision

michaelmaitland · 2024-04-22T18:28:25Z

llvm/tools/llvm-exegesis/lib/BenchmarkCode.h

@@ -17,6 +17,11 @@
 namespace llvm {
 namespace exegesis {

+struct ScratchMemoryStore {


Can you add docstring to this struct and its fields?

Removed this structure, because it is not used

michaelmaitland · 2024-04-22T18:31:41Z

llvm/tools/llvm-exegesis/lib/MCInstrDescView.cpp

@@ -166,6 +166,9 @@ Instruction::create(const MCInstrInfo &InstrInfo,
  BitVector ImplUseRegs = RATC.emptyRegisters();
  BitVector AllDefRegs = RATC.emptyRegisters();
  BitVector AllUseRegs = RATC.emptyRegisters();
+  BitVector MemoryRegs = RATC.emptyRegisters();


It seems that MemoryRegs is not used. Do we need it?

Not currently used, it was added in addition to NotMemoryRegs since they are all aliased registers together. Could be used in future

I think we should remove it until it is used.

llvm/tools/llvm-exegesis/lib/RISCV/RISCVCounters.cpp

llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp

michaelmaitland · 2024-04-22T18:38:00Z

llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp

+      CodeTemplates.push_back(std::move(CT));
+    }
+
+    // TODO: implement more cases


What are the other cases besides !I.Description.mayLoad()? What was the reason we chose to implement only the I.Description.mayLoad() case for RISC-V support?

Now case for I.Description.mayLoad() is enough to provide aliasing strategy for load operation in RISC-V. In future, maybe, we add some more cases for other strategies.

michaelmaitland · 2024-04-22T18:38:30Z

llvm/tools/llvm-exegesis/lib/SnippetGenerator.h

@@ -55,6 +55,7 @@ class SnippetGenerator {
 public:
  struct Options {
    unsigned MaxConfigsPerOpcode = 1;
+    std::optional<int64_t> InitImmValue;


Is this used anywhere?

Now it really isn't used. We made it with expansion in mind. Deleted in this commit

llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp

michaelmaitland · 2024-04-22T18:43:39Z

llvm/tools/llvm-exegesis/lib/Target.h

+
+  // Find register by name, 0 if not found.
+  virtual unsigned findRegisterByName(const StringRef RegName) const {
+    return 0;


Is it possible for a register to have the value 0? Why not RISCV::NoRegister for RISC-V? Thats what RISCVAsmParser::matchRegisterNameHelper returns when there is no match.

michaelmaitland · 2024-05-03T15:07:20Z

llvm/test/tools/llvm-exegesis/RISCV/latency-pre-assigned-register.s

@@ -0,0 +1,3 @@
+RUN: llvm-exegesis -mode=latency --benchmark-phase=assemble-measured-code -opcode-name=LB -mtriple=riscv64-unknown-linux-gnu
+
+CHECK: Warning: Pre-assigned register prevented usage of self-aliasing strategy.


I am a bit confused by this test. What is the pre-assigned register that prevented usage of self-aliasing? I would expect a warning that no instructions were passed.

This test was used previously, during implementing, now it is not used anymore, removed it, thanks for the catch.

topperc · 2024-05-03T16:15:45Z

llvm/tools/llvm-exegesis/lib/X86/Target.cpp

@@ -1009,6 +1011,10 @@ static std::vector<MCInst> loadImmediateSegmentRegister(unsigned Reg,
 #endif // defined(__x86_64__) && defined(__linux__)
 }

+unsigned ExegesisX86Target::findRegisterByName(const StringRef RegName) const {


Can we just have a default implementation that returns MCRegister::NoRegister instead of having a function for every target?

topperc · 2024-05-03T16:23:53Z

llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp

+  MCOperand &AssignedValue = IT.getValueFor(0);
+
+  switch (IT.getOpcode()) {
+  case RISCV::C_ADDI16SP:


What about C_ADDI4SPN?

mshockwave · 2024-05-13T16:16:22Z

llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp

+      Instr.getPrimaryOperand(Var).getExplicitOperandInfo().OperandType;
+
+  switch (OperandType) {
+  case RISCVOp::OPERAND_FRMARG:


The rationale behind my suggestion is that I'm still not really convinced we should add a new OPERAND_FRMARG operand type to RISC-V. As we have always been able to locate rounding mode operand with RISCV::getNamedOperandIdx. Therefore, I think we can get the index of exegesis::Operand we're dealing here through Instr.getPrimaryOperand(Var) and check if that index matches what returned from RISCV::getNamedOperandIdx to know if it's a rounding mode operand.

mshockwave · 2024-05-13T16:23:38Z

llvm/test/tools/llvm-exegesis/RISCV/latency-by-load.s

@@ -0,0 +1,60 @@
+# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux-gnu --benchmark-phase=assemble-measured-code -opcode-name=LD | FileCheck --check-prefix=TEST1 %s


nit: usually all RUN lines would be put at the very top of the file. Also, could you use the opcode you're testing as FileCheck prefix rather than TESTX?

Changed prefix in FileCheck to opcode name, thanks. I prefer to leave RUN near the CHECK lines. It is more logical and easy to read for me

mshockwave · 2024-05-13T16:27:27Z

llvm/tools/llvm-exegesis/lib/MCInstrDescView.h

+  // Repeating this instruction may execute sequentially by picking aliasing
+  // Def and Not Memory Use registers. It may also execute in parallel by
+  // picking non aliasing Def and Not Memory Use registers.
+  bool hasAliasingNotMemoryRegisters(const BitVector &ForbiddenRegisters) const;


general question: it seems like you made quite some improvements on the infrastructure for memory instructions, which is nice. But could you add tests for these potentially corner cases so that we don't regress in the future.

This function use to enable alternative strategy for instructions with memory operands. We have tests to cheek this behavior in test/tools/llvm-exegesis/RISCV/latency-by-extension-A.s

mshockwave · 2024-05-13T16:28:40Z

llvm/tools/llvm-exegesis/lib/MCInstrDescView.h

@@ -160,12 +166,17 @@ struct Instruction {
  const BitVector &ImplUseRegs; // The set of aliased implicit use registers.
  const BitVector &AllDefRegs;  // The set of all aliased def registers.
  const BitVector &AllUseRegs;  // The set of all aliased use registers.
+  const BitVector &MemoryRegs;  // The set of all aliased memory use registers.
+  const BitVector


This formatting looks odd... maybe we could just put the comment above the line so that clang-format won't do anything funny?

mshockwave · 2024-05-13T16:39:20Z

llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp

+static cl::OptionCategory
+    RISCVBenchmarkOptions("llvm-exegesis benchmark RISCV options");
+
+// TODO move perf counter data to td files (although it looks like an overkill


This should probably be done rather than leaving it as a TODO just so that all the targets are consistent.

Second this, I think you only need to add a couple lines.

mshockwave · 2024-05-13T16:43:48Z

llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp

+    StringRef CounterName, const LLVMState &State,
+    ArrayRef<const char *> ValidationCounters, const pid_t ProcessID) const {
+  if (CounterName == RISCVPfmCounterNames[0]) {
+    return createRISCVCpuCyclesCounter(pfm::PerfEvent(CounterName));


I also think we should try to use as much Linux Perf as possible. Granted, reading mcycle from CSR register is more generic, but llvm-exegesis already puts an assumption that it has to run on Linux so we might as well just use Perf. Libpfm hasn't supported RISC-V yet but we can use Perf to read the CYCLES event.

mshockwave · 2024-05-13T16:46:23Z

llvm/tools/llvm-exegesis/llvm-exegesis.cpp

@@ -291,6 +297,22 @@ T ExitOnFileError(const Twine &FileName, Expected<T> &&E) {
  return std::move(*E);
 }

+static const char *getIgnoredOpcodeReasonOrNull(const LLVMState &State,


Here:

llvm-project/llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp

Line 58 in 7964356

if (OtherInstrDesc.isPseudo() || OtherInstrDesc.usesCustomInsertionHook() ||

mshockwave · 2024-05-13T16:48:40Z

Note: please don't forced push to update PR unless it's really necessary: https://llvm.org/docs/GitHub.html#updating-pull-requests

legrosbuffle · 2024-05-14T11:53:50Z

llvm/tools/llvm-exegesis/lib/BenchmarkCode.h

@@ -17,6 +17,11 @@
 namespace llvm {
 namespace exegesis {

+struct ScratchMemoryStore {


This looks unused, did you mean to use it in PreinitScratchMemory ?

It is really unused. Thanks for the catch!

legrosbuffle · 2024-05-14T11:57:09Z

llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp

+      if (DefOpIt == I.Operands.end())
+        return;
+
+      auto &DefOp = *DefOpIt;


avoid auto here for readability. getValueFor has a few overloads, it' not obvious which one is called.

legrosbuffle · 2024-05-14T12:00:28Z

llvm/tools/llvm-exegesis/lib/CodeTemplate.h

@@ -35,6 +35,8 @@ struct InstructionTemplate {
  const MCOperand &getValueFor(const Variable &Var) const;
  MCOperand &getValueFor(const Operand &Op);
  const MCOperand &getValueFor(const Operand &Op) const;
+  MCOperand &getValueFor(unsigned OpIdx);


Having the same name for this overload is confusing: the two overloads for Variable and Operand are unambiguous thanks to the types, but this one is unclear as to whether we're talking about an operand or variable. getValueForOperandIdx ?

legrosbuffle · 2024-05-14T12:02:12Z

llvm/tools/llvm-exegesis/lib/LlvmState.cpp

+  if (CpuName.empty()) {
+    std::unique_ptr<MCSubtargetInfo> Empty_STI(
+        TheTarget->createMCSubtargetInfo(TripleName, "", ""));
+    CpuName = Empty_STI->getAllProcessorDescriptions().begin()->Key;


It's not even guaranteed that we can run code on the host when jitting for this first CPU, so it feels weird to automatically use this.

legrosbuffle · 2024-05-14T12:07:22Z

llvm/tools/llvm-exegesis/lib/Target.h

+  }
+
+  // Find register by name, NoRegister if not found.
+  virtual unsigned findRegisterByName(const StringRef RegName) const = 0;


Why can't this be non-virtual and use RegNameToRegNoMapping ?

Create default implementation for all platforms. Thank you!

Llvm-exegesis RISCV port is a result of team effort. Below everyone involved listed. Co-authored-by: Konstantin Vladimirov <konstantin.vladimirov@syntacore.com> Co-authored-by: Dmitrii Petrov <dmitrii.petrov@syntacore.com> Co-authored-by: Dmitry Bushev <dmitry.bushev@syntacore.com> Co-authored-by: Mark Goncharov <mark.goncharov@syntacore.com> Co-authored-by: Anastasiya Chernikova <anastasiya.chernikova@syntacore.com>

mshockwave · 2024-05-21T20:22:33Z

llvm/test/tools/llvm-exegesis/RISCV/latency-by-extension-A.s

+AMOAND_D-NEXT: config: ''
+AMOAND_D-NEXT: register_initial_values:
+AMOAND_D-NEXT: - '[[RE01:X[0-9]+]]=0x0'
+AMOAND_D-LAST: ...


I don't think there is a -LAST directive suffix in FileCheck. (You didn't get an error because FileCheck thought "AMOAND_D-LAST" as a whole is a prefix, which it then gladly ignored)

Thank you for the catch. I fixed this in tests under RISCV. But we have the same problem in some tests in other platforms. Created separate pull request for other platforms #93222

…pliant way Fixed some FileChecks in tests. Firstly found in PR89047 (llvm#89047 (comment))

…#93222) Fixed some FileChecks in tests. Firstly found in PR89047 (#89047 (comment))

…llvm#93222) Fixed some FileChecks in tests. Firstly found in PR89047 (llvm#89047 (comment))

AnastasiyaChernikova · 2024-05-31T17:54:14Z

@legrosbuffle @mshockwave @michaelmaitland @topperc @boomanaiden154 @gchatelet I answered all the comments, please take a look

legrosbuffle · 2024-06-04T12:05:13Z

llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp

@@ -113,7 +109,51 @@ static void appendCodeTemplates(const LLVMState &State,
  }
  case ExecutionMode::SERIAL_VIA_MEMORY_INSTR: {
    // Select back-to-back memory instruction.
-    // TODO: Implement me.
+


It would be nice to submit this as a separate PR given that it's really independent from the riscv support.

It would make the current PR much easier to review.

This part of code is necessary to implemented strategy for load operation in RISCV.
Example with LD instruction without this part of code:

mode: latency key: instructions: - 'LD X12 X10 i_0x0' - 'SLL X17 X12 X28' config: '' register_initial_values: - 'X28=0x0'

And with this code:

mode: latency key: instructions: - 'LD X10 X10 i_0x0' config: '' register_initial_values: []

This test from latency-by-load.s

llvmbot added backend:RISC-V tools:llvm-exegesis labels Apr 17, 2024

RKSimon requested review from boomanaiden154, legrosbuffle and gchatelet April 17, 2024 13:00

4vtomat requested a review from mshockwave April 17, 2024 13:40

AnastasiyaChernikova force-pushed the riscv_support branch from 0855ea1 to 9d921e8 Compare April 17, 2024 15:46

mshockwave reviewed Apr 17, 2024

View reviewed changes

topperc reviewed Apr 17, 2024

View reviewed changes

boomanaiden154 reviewed Apr 18, 2024

View reviewed changes

michaelmaitland reviewed Apr 22, 2024

View reviewed changes

AnastasiyaChernikova force-pushed the riscv_support branch 3 times, most recently from e058716 to c870765 Compare April 24, 2024 09:59

AnastasiyaChernikova force-pushed the riscv_support branch from c870765 to 6fa0e18 Compare May 1, 2024 20:29

michaelmaitland reviewed May 3, 2024

View reviewed changes

topperc reviewed May 3, 2024

View reviewed changes

AnastasiyaChernikova force-pushed the riscv_support branch from 6fa0e18 to 90d57cd Compare May 3, 2024 21:19

mshockwave reviewed May 13, 2024

View reviewed changes

legrosbuffle reviewed May 14, 2024

View reviewed changes

AnastasiyaChernikova added 2 commits May 17, 2024 15:56

Addressing review comments

10d4f37

AnastasiyaChernikova force-pushed the riscv_support branch from 5c7be16 to 10d4f37 Compare May 17, 2024 12:56

AnastasiyaChernikova added 2 commits May 21, 2024 16:29

Addressing review comments

45dbe40

Addressing review comments

8088b2b

mshockwave reviewed May 21, 2024

View reviewed changes

AnastasiyaChernikova mentioned this pull request May 23, 2024

[Exegesis] Changing non-standard CHECK in tests to more compliant way #93222

Merged

AnastasiyaChernikova added 2 commits May 23, 2024 20:59

Fixed tests

6608293

Addressing review comments

fe13a8e

dybv-sc pushed a commit that referenced this pull request May 29, 2024

[Exegesis] Changing non-standard CHECK in tests to more compliant way (…

4ad2f41

…#93222) Fixed some FileChecks in tests. Firstly found in PR89047 (#89047 (comment))

vg0204 pushed a commit to vg0204/llvm-project that referenced this pull request May 29, 2024

[Exegesis] Changing non-standard CHECK in tests to more compliant way (…

0a9fc0e

…llvm#93222) Fixed some FileChecks in tests. Firstly found in PR89047 (llvm#89047 (comment))

Addressing review comments

fec059e

AnastasiyaChernikova requested review from rupprecht and keith as code owners May 31, 2024 16:14

llvmbot added the bazel "Peripheral" support tier build system: utils/bazel label May 31, 2024

AnastasiyaChernikova requested review from legrosbuffle, mshockwave, topperc and michaelmaitland June 4, 2024 11:27

legrosbuffle reviewed Jun 4, 2024

View reviewed changes

AnastasiyaChernikova requested a review from legrosbuffle June 5, 2024 14:22

		@@ -0,0 +1,3 @@
		RUN: llvm-exegesis -mode=latency --benchmark-phase=assemble-measured-code -opcode-name=LB -mtriple=riscv64-unknown-linux-gnu

		CHECK: Warning: Pre-assigned register prevented usage of self-aliasing strategy.

		@@ -0,0 +1,60 @@
		# RUN: llvm-exegesis -mode=latency -mtriple=riscv64-unknown-linux-gnu --benchmark-phase=assemble-measured-code -opcode-name=LD \| FileCheck --check-prefix=TEST1 %s

[Exegesis][RISCV] Add RISCV support for llvm-exegesis #89047

Are you sure you want to change the base?

[Exegesis][RISCV] Add RISCV support for llvm-exegesis #89047

Conversation

AnastasiyaChernikova commented Apr 17, 2024

github-actions bot commented Apr 17, 2024

llvmbot commented Apr 17, 2024 • edited

github-actions bot commented Apr 17, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boomanaiden154 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AnastasiyaChernikova May 24, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AnastasiyaChernikova May 21, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

llvmbot commented Apr 17, 2024 •

edited

github-actions bot commented Apr 17, 2024 •

edited

AnastasiyaChernikova May 24, 2024 •

edited

AnastasiyaChernikova May 21, 2024 •

edited