Skip to content

Conversation

r-belenov
Copy link
Contributor

Recently added latency customization (PR) does not work on RISCV since it has target-specific InstrumentManager that overrides default functionality. Added calls to base class to ensure that common instruments (including latency customizer) are available.

@llvmbot
Copy link
Member

llvmbot commented Sep 22, 2025

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

Author: Roman Belenov (r-belenov)

Changes

Recently added latency customization (PR) does not work on RISCV since it has target-specific InstrumentManager that overrides default functionality. Added calls to base class to ensure that common instruments (including latency customizer) are available.


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.cpp (+5-3)
  • (added) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/latency-instrument.s (+44)
diff --git a/llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.cpp b/llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.cpp
index ae44306170758..9b6f3504cf7ff 100644
--- a/llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.cpp
+++ b/llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.cpp
@@ -86,7 +86,8 @@ uint8_t RISCVSEWInstrument::getSEW() const {
 bool RISCVInstrumentManager::supportsInstrumentType(
     llvm::StringRef Type) const {
   return Type == RISCVLMULInstrument::DESC_NAME ||
-         Type == RISCVSEWInstrument::DESC_NAME;
+         Type == RISCVSEWInstrument::DESC_NAME ||
+         InstrumentManager::supportsInstrumentType(Type);;
 }
 
 UniqueInstrument
@@ -110,8 +111,9 @@ RISCVInstrumentManager::createInstrument(llvm::StringRef Desc,
     return std::make_unique<RISCVSEWInstrument>(Data);
   }
 
-  LLVM_DEBUG(dbgs() << "RVCB: Unknown instrumentation Desc: " << Desc << '\n');
-  return nullptr;
+  LLVM_DEBUG(dbgs() << "RVCB: Creating default instrument for Desc: "
+                    << Desc << '\n');
+  return InstrumentManager::createInstrument(Desc, Data);
 }
 
 SmallVector<UniqueInstrument>
diff --git a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/latency-instrument.s b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/latency-instrument.s
new file mode 100644
index 0000000000000..0ac183be85d54
--- /dev/null
+++ b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/latency-instrument.s
@@ -0,0 +1,44 @@
+# NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
+# RUN: llvm-mca -mtriple=riscv64 -mcpu=sifive-x280 -iterations=1 < %s | FileCheck %s
+
+# LLVM-MCA-LATENCY 100
+add a0, a0, a0
+
+# CHECK:      Iterations:        1
+# CHECK-NEXT: Instructions:      1
+# CHECK-NEXT: Total Cycles:      101
+# CHECK-NEXT: Total uOps:        1
+
+# CHECK:      Dispatch Width:    2
+# CHECK-NEXT: uOps Per Cycle:    0.01
+# CHECK-NEXT: IPC:               0.01
+# CHECK-NEXT: Block RThroughput: 0.5
+
+# CHECK:      Instruction Info:
+# CHECK-NEXT: [1]: #uOps
+# CHECK-NEXT: [2]: Latency
+# CHECK-NEXT: [3]: RThroughput
+# CHECK-NEXT: [4]: MayLoad
+# CHECK-NEXT: [5]: MayStore
+# CHECK-NEXT: [6]: HasSideEffects (U)
+
+# CHECK:      [1]    [2]    [3]    [4]    [5]    [6]    Instructions:
+# CHECK-NEXT:  1      3     0.50                        add     a0, a0, a0
+
+# CHECK:      Resources:
+# CHECK-NEXT: [0]   - VLEN512SiFive7FDiv
+# CHECK-NEXT: [1]   - VLEN512SiFive7IDiv
+# CHECK-NEXT: [2]   - VLEN512SiFive7PipeA
+# CHECK-NEXT: [3]   - VLEN512SiFive7PipeB
+# CHECK-NEXT: [4]   - VLEN512SiFive7VA
+# CHECK-NEXT: [5]   - VLEN512SiFive7VCQ
+# CHECK-NEXT: [6]   - VLEN512SiFive7VL
+# CHECK-NEXT: [7]   - VLEN512SiFive7VS
+
+# CHECK:      Resource pressure per iteration:
+# CHECK-NEXT: [0]    [1]    [2]    [3]    [4]    [5]    [6]    [7]
+# CHECK-NEXT:  -      -      -     1.00    -      -      -      -
+
+# CHECK:      Resource pressure by instruction:
+# CHECK-NEXT: [0]    [1]    [2]    [3]    [4]    [5]    [6]    [7]    Instructions:
+# CHECK-NEXT:  -      -      -     1.00    -      -      -      -     add       a0, a0, a0

Copy link

github-actions bot commented Sep 22, 2025

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

@r-belenov
Copy link
Contributor Author

@mshockwave - please check

Copy link
Contributor

@wangpc-pp wangpc-pp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Good catch!

Copy link
Member

@mshockwave mshockwave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks

@mshockwave mshockwave merged commit 51a86e7 into llvm:main Sep 23, 2025
9 checks passed
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.

4 participants