Skip to content

[MC] Account for AcquireAtCycle in getReciprocalThroughput #132653

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Mar 24, 2025
Merged

[MC] Account for AcquireAtCycle in getReciprocalThroughput #132653

merged 2 commits into from
Mar 24, 2025

Conversation

mshockwave
Copy link
Member

@mshockwave mshockwave commented Mar 24, 2025 •
edited
Loading

Previously MCSchedModel::getReciprocalThroughput ignored AcquireAtCycle completey, this patch fixes it by using the largest (ReleaseAtCycle - AcquireAtCycle) / NumUnits as inverse throughput.

Here are some technical explanations: https://myhsu.xyz/llvm-sched-interval-throughput

@mshockwave @JulienVillette
[MC] Account for AcquireAtCycle in getReciprocalThroughput
6fde28d 
Co-Authored-By: Julien Villette <julien.villette@sipearl.com>
@mshockwave mshockwave mentioned this pull request Mar 24, 2025
Merged
@mshockwave mshockwave marked this pull request as ready for review March 24, 2025 02:10
@llvmbot llvmbot added the mc Machine (object) code label Mar 24, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 24, 2025

@llvm/pr-subscribers-mc

Author: Min-Yih Hsu (mshockwave)

Changes

Previously MCSchedModel::getReciprocalThroughput ignored AcquireAtCycle completey, this patch fixes it by using the largest (ReleaseAtCycle - AcquireAtCycle) / NumUnits as inverse throughput.

Here are some technical explanations: https://myhsu.xyz/llvm-sched-interval-throughput

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

26 Files Affected:

  • (modified) llvm/lib/MC/MCSchedule.cpp (+9-6)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/different-lmul-instruments.s (+2-2)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/different-sew-instruments.s (+2-2)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/disable-im.s (+3-3)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/fractional-lmul-data.s (+2-2)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-at-start.s (+1-1)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-in-middle.s (+2-2)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-in-region.s (+1-1)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-straddles-region.s (+1-1)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/multiple-same-lmul-instruments.s (+5-5)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/multiple-same-sew-instruments.s (+5-5)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/needs-sew-but-only-lmul.s (+2-2)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/no-vsetvli-to-start.s (+2-2)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/reductions.s (+103-103)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/sew-instrument-at-start.s (+1-1)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/sew-instrument-in-middle.s (+2-2)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/sew-instrument-in-region.s (+1-1)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/sew-instrument-straddles-region.s (+1-1)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/strided-load-store.s (+78-78)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/strided-load-x0.s (+24-24)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/vector-integer-arithmetic.s (+381-381)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/vle-vse.s (+200-200)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/vsetivli-lmul-instrument.s (+2-2)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/vsetivli-lmul-sew-instrument.s (+2-2)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/vsetvli-lmul-instrument.s (+2-2)
  • (modified) llvm/test/tools/llvm-mca/RISCV/SiFiveX280/vsetvli-lmul-sew-instrument.s (+2-2)
diff --git a/llvm/lib/MC/MCSchedule.cpp b/llvm/lib/MC/MCSchedule.cpp
index ed243cecabb76..bc7a426d5f0dc 100644
--- a/llvm/lib/MC/MCSchedule.cpp
+++ b/llvm/lib/MC/MCSchedule.cpp
@@ -96,19 +96,22 @@ int MCSchedModel::computeInstrLatency(const MCSubtargetInfo &STI,
 double
 MCSchedModel::getReciprocalThroughput(const MCSubtargetInfo &STI,
                                       const MCSchedClassDesc &SCDesc) {
-  std::optional<double> Throughput;
+  std::optional<double> MinThroughput;
   const MCSchedModel &SM = STI.getSchedModel();
   const MCWriteProcResEntry *I = STI.getWriteProcResBegin(&SCDesc);
   const MCWriteProcResEntry *E = STI.getWriteProcResEnd(&SCDesc);
   for (; I != E; ++I) {
-    if (!I->ReleaseAtCycle)
+    if (!I->ReleaseAtCycle || I->ReleaseAtCycle == I->AcquireAtCycle)
       continue;
+    assert(I->ReleaseAtCycle > I->AcquireAtCycle);
     unsigned NumUnits = SM.getProcResource(I->ProcResourceIdx)->NumUnits;
-    double Temp = NumUnits * 1.0 / I->ReleaseAtCycle;
-    Throughput = Throughput ? std::min(*Throughput, Temp) : Temp;
+    double Throughput =
+        double(NumUnits) / double(I->ReleaseAtCycle - I->AcquireAtCycle);
+    MinThroughput =
+        MinThroughput ? std::min(*MinThroughput, Throughput) : Throughput;
   }
-  if (Throughput)
-    return 1.0 / *Throughput;
+  if (MinThroughput)
+    return 1.0 / *MinThroughput;
 
   // If no throughput value was calculated, assume that we can execute at the
   // maximum issue width scaled by number of micro-ops for the schedule class.
diff --git a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/different-lmul-instruments.s b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/different-lmul-instruments.s
index 29148092882d8..0e7284fb550af 100644
--- a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/different-lmul-instruments.s
+++ b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/different-lmul-instruments.s
@@ -28,9 +28,9 @@ vadd.vv v12, v12, v12
 
 # CHECK:      [1]    [2]    [3]    [4]    [5]    [6]    Instructions:
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, a0, e8, m1, tu, mu
-# CHECK-NEXT:  1      4     3.00                        vadd.vv	v12, v12, v12
+# CHECK-NEXT:  1      4     2.00                        vadd.vv	v12, v12, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, a0, e8, m8, tu, mu
-# CHECK-NEXT:  1      4     17.00                       vadd.vv	v12, v12, v12
+# CHECK-NEXT:  1      4     16.00                       vadd.vv	v12, v12, v12
 
 # CHECK:      Resources:
 # CHECK-NEXT: [0]   - SiFive7FDiv
diff --git a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/different-sew-instruments.s b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/different-sew-instruments.s
index 44d6c442f52d7..b445855ef3c11 100644
--- a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/different-sew-instruments.s
+++ b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/different-sew-instruments.s
@@ -29,9 +29,9 @@ vdiv.vv v8, v8, v12
 
 # CHECK:      [1]    [2]    [3]    [4]    [5]    [6]    Instructions:
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, a0, e8, m1, tu, mu
-# CHECK-NEXT:  1      240   241.00                      vdiv.vv	v8, v8, v12
+# CHECK-NEXT:  1      240   240.00                      vdiv.vv	v8, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, a0, e64, m1, tu, mu
-# CHECK-NEXT:  1      114   115.00                      vdiv.vv	v8, v8, v12
+# CHECK-NEXT:  1      114   114.00                      vdiv.vv	v8, v8, v12
 
 # CHECK:      Resources:
 # CHECK-NEXT: [0]   - SiFive7FDiv
diff --git a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/disable-im.s b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/disable-im.s
index 9885d4accc44b..0cd96701fd8f8 100644
--- a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/disable-im.s
+++ b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/disable-im.s
@@ -31,11 +31,11 @@ vadd.vv v12, v12, v12
 
 # CHECK:      [1]    [2]    [3]    [4]    [5]    [6]    Instructions:
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, a0, e8, m2, tu, mu
-# CHECK-NEXT:  1      4     17.00                       vadd.vv	v12, v12, v12
+# CHECK-NEXT:  1      4     16.00                       vadd.vv	v12, v12, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, a0, e8, m1, tu, mu
-# CHECK-NEXT:  1      4     17.00                       vadd.vv	v12, v12, v12
+# CHECK-NEXT:  1      4     16.00                       vadd.vv	v12, v12, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, a0, e8, m8, tu, mu
-# CHECK-NEXT:  1      4     17.00                       vadd.vv	v12, v12, v12
+# CHECK-NEXT:  1      4     16.00                       vadd.vv	v12, v12, v12
 
 # CHECK:      Resources:
 # CHECK-NEXT: [0]   - SiFive7FDiv
diff --git a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/fractional-lmul-data.s b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/fractional-lmul-data.s
index 0b5a557170358..ac4b138da98ac 100644
--- a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/fractional-lmul-data.s
+++ b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/fractional-lmul-data.s
@@ -29,9 +29,9 @@ vdiv.vv v12, v12, v12
 
 # CHECK:      [1]    [2]    [3]    [4]    [5]    [6]    Instructions:
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e32, mf2, tu, mu
-# CHECK-NEXT:  1      56    57.00                       vdiv.vv	v12, v12, v12
+# CHECK-NEXT:  1      56    56.00                       vdiv.vv	v12, v12, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e8, mf8, tu, mu
-# CHECK-NEXT:  1      30    31.00                       vdiv.vv	v12, v12, v12
+# CHECK-NEXT:  1      30    30.00                       vdiv.vv	v12, v12, v12
 
 # CHECK:      Resources:
 # CHECK-NEXT: [0]   - SiFive7FDiv
diff --git a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-at-start.s b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-at-start.s
index 9a47f3fc4f8d8..53d106a327411 100644
--- a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-at-start.s
+++ b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-at-start.s
@@ -25,7 +25,7 @@ vadd.vv v12, v12, v12
 
 # CHECK:      [1]    [2]    [3]    [4]    [5]    [6]    Instructions:
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, a0, e8, m1, tu, mu
-# CHECK-NEXT:  1      4     3.00                        vadd.vv	v12, v12, v12
+# CHECK-NEXT:  1      4     2.00                        vadd.vv	v12, v12, v12
 
 # CHECK:      Resources:
 # CHECK-NEXT: [0]   - SiFive7FDiv
diff --git a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-in-middle.s b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-in-middle.s
index b7525f5114be9..2b0e3fa1b8af4 100644
--- a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-in-middle.s
+++ b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-in-middle.s
@@ -25,9 +25,9 @@ vadd.vv v12, v12, v12
 # CHECK-NEXT: [6]: HasSideEffects (U)
 
 # CHECK:      [1]    [2]    [3]    [4]    [5]    [6]    Instructions:
-# CHECK-NEXT:  1      4     17.00                       vadd.vv	v12, v12, v12
+# CHECK-NEXT:  1      4     16.00                       vadd.vv	v12, v12, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, a0, e8, mf8, tu, mu
-# CHECK-NEXT:  1      4     2.00                        vadd.vv	v12, v12, v12
+# CHECK-NEXT:  1      4     1.00                        vadd.vv	v12, v12, v12
 
 # CHECK:      Resources:
 # CHECK-NEXT: [0]   - SiFive7FDiv
diff --git a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-in-region.s b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-in-region.s
index 918d03cb503aa..308994116ed26 100644
--- a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-in-region.s
+++ b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-in-region.s
@@ -29,7 +29,7 @@ vadd.vv v12, v12, v12
 
 # CHECK:      [1]    [2]    [3]    [4]    [5]    [6]    Instructions:
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, a0, e8, m1, tu, mu
-# CHECK-NEXT:  1      4     3.00                        vadd.vv	v12, v12, v12
+# CHECK-NEXT:  1      4     2.00                        vadd.vv	v12, v12, v12
 
 # CHECK:      Resources:
 # CHECK-NEXT: [0]   - SiFive7FDiv
diff --git a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-straddles-region.s b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-straddles-region.s
index 2b500ab9d014c..7c596b16109fb 100644
--- a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-straddles-region.s
+++ b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/lmul-instrument-straddles-region.s
@@ -30,7 +30,7 @@ vadd.vv v12, v12, v12
 
 # CHECK:      [1]    [2]    [3]    [4]    [5]    [6]    Instructions:
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, a0, e8, m1, tu, mu
-# CHECK-NEXT:  1      4     3.00                        vadd.vv	v12, v12, v12
+# CHECK-NEXT:  1      4     2.00                        vadd.vv	v12, v12, v12
 
 # CHECK:      Resources:
 # CHECK-NEXT: [0]   - SiFive7FDiv
diff --git a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/multiple-same-lmul-instruments.s b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/multiple-same-lmul-instruments.s
index 6cb19048665fb..680d84e9dc226 100644
--- a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/multiple-same-lmul-instruments.s
+++ b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/multiple-same-lmul-instruments.s
@@ -33,13 +33,13 @@ vsub.vv v12, v12, v12
 
 # CHECK:      [1]    [2]    [3]    [4]    [5]    [6]    Instructions:
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, a0, e8, m1, tu, mu
-# CHECK-NEXT:  1      4     3.00                        vadd.vv	v12, v12, v12
+# CHECK-NEXT:  1      4     2.00                        vadd.vv	v12, v12, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, a0, e8, m1, tu, mu
-# CHECK-NEXT:  1      4     3.00                        vadd.vv	v12, v12, v12
-# CHECK-NEXT:  1      4     3.00                        vsub.vv	v12, v12, v12
+# CHECK-NEXT:  1      4     2.00                        vadd.vv	v12, v12, v12
+# CHECK-NEXT:  1      4     2.00                        vsub.vv	v12, v12, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, a0, e8, m4, tu, mu
-# CHECK-NEXT:  1      4     9.00                        vadd.vv	v12, v12, v12
-# CHECK-NEXT:  1      4     9.00                        vsub.vv	v12, v12, v12
+# CHECK-NEXT:  1      4     8.00                        vadd.vv	v12, v12, v12
+# CHECK-NEXT:  1      4     8.00                        vsub.vv	v12, v12, v12
 
 # CHECK:      Resources:
 # CHECK-NEXT: [0]   - SiFive7FDiv
diff --git a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/multiple-same-sew-instruments.s b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/multiple-same-sew-instruments.s
index ace5eac406bfb..02e1fd72bceea 100644
--- a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/multiple-same-sew-instruments.s
+++ b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/multiple-same-sew-instruments.s
@@ -34,13 +34,13 @@ vdivu.vv v8, v8, v12
 
 # CHECK:      [1]    [2]    [3]    [4]    [5]    [6]    Instructions:
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, a0, e64, m1, tu, mu
-# CHECK-NEXT:  1      114   115.00                      vdiv.vv	v8, v8, v12
+# CHECK-NEXT:  1      114   114.00                      vdiv.vv	v8, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, a0, e64, m1, tu, mu
-# CHECK-NEXT:  1      114   115.00                      vdiv.vv	v8, v8, v12
-# CHECK-NEXT:  1      114   115.00                      vdivu.vv	v8, v8, v12
+# CHECK-NEXT:  1      114   114.00                      vdiv.vv	v8, v8, v12
+# CHECK-NEXT:  1      114   114.00                      vdivu.vv	v8, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, a0, e32, m1, tu, mu
-# CHECK-NEXT:  1      112   113.00                      vdiv.vv	v8, v8, v12
-# CHECK-NEXT:  1      112   113.00                      vdivu.vv	v8, v8, v12
+# CHECK-NEXT:  1      112   112.00                      vdiv.vv	v8, v8, v12
+# CHECK-NEXT:  1      112   112.00                      vdivu.vv	v8, v8, v12
 
 # CHECK:      Resources:
 # CHECK-NEXT: [0]   - SiFive7FDiv
diff --git a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/needs-sew-but-only-lmul.s b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/needs-sew-but-only-lmul.s
index c20200bd536a7..2a58ea7962ae8 100644
--- a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/needs-sew-but-only-lmul.s
+++ b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/needs-sew-but-only-lmul.s
@@ -28,8 +28,8 @@ vdiv.vv v8, v8, v12
 
 # CHECK:      [1]    [2]    [3]    [4]    [5]    [6]    Instructions:
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, a0, e8, m1, tu, mu
-# CHECK-NEXT:  1      240   241.00                      vdiv.vv	v8, v8, v12
-# CHECK-NEXT:  1      240   241.00                      vdiv.vv	v8, v8, v12
+# CHECK-NEXT:  1      240   240.00                      vdiv.vv	v8, v8, v12
+# CHECK-NEXT:  1      240   240.00                      vdiv.vv	v8, v8, v12
 
 # CHECK:      Resources:
 # CHECK-NEXT: [0]   - SiFive7FDiv
diff --git a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/no-vsetvli-to-start.s b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/no-vsetvli-to-start.s
index a8d25ed271f74..e16b8f982b9c9 100644
--- a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/no-vsetvli-to-start.s
+++ b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/no-vsetvli-to-start.s
@@ -24,9 +24,9 @@ vadd.vv v12, v12, v12
 # CHECK-NEXT: [6]: HasSideEffects (U)
 
 # CHECK:      [1]    [2]    [3]    [4]    [5]    [6]    Instructions:
-# CHECK-NEXT:  1      4     17.00                       vadd.vv	v12, v12, v12
+# CHECK-NEXT:  1      4     16.00                       vadd.vv	v12, v12, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, a0, e8, m1, tu, mu
-# CHECK-NEXT:  1      4     3.00                        vadd.vv	v12, v12, v12
+# CHECK-NEXT:  1      4     2.00                        vadd.vv	v12, v12, v12
 
 # CHECK:      Resources:
 # CHECK-NEXT: [0]   - SiFive7FDiv
diff --git a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/reductions.s b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/reductions.s
index a6b756ba8151b..32e2d0e94305e 100644
--- a/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/reductions.s
+++ b/llvm/test/tools/llvm-mca/RISCV/SiFiveX280/reductions.s
@@ -241,211 +241,211 @@ vfredmin.vs  v4, v8, v12
 
 # CHECK:      [1]    [2]    [3]    [4]    [5]    [6]    Instructions:
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e8, mf8, tu, mu
-# CHECK-NEXT:  1      46    47.00                       vredsum.vs	v4, v8, v12
+# CHECK-NEXT:  1      46    46.00                       vredsum.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e8, mf4, tu, mu
-# CHECK-NEXT:  1      46    47.00                       vredsum.vs	v4, v8, v12
+# CHECK-NEXT:  1      46    46.00                       vredsum.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e8, mf2, tu, mu
-# CHECK-NEXT:  1      46    47.00                       vredsum.vs	v4, v8, v12
+# CHECK-NEXT:  1      46    46.00                       vredsum.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e8, m1, tu, mu
-# CHECK-NEXT:  1      47    48.00                       vredsum.vs	v4, v8, v12
+# CHECK-NEXT:  1      47    47.00                       vredsum.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e8, m2, tu, mu
-# CHECK-NEXT:  1      49    50.00                       vredsum.vs	v4, v8, v12
+# CHECK-NEXT:  1      49    49.00                       vredsum.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e8, m4, tu, mu
-# CHECK-NEXT:  1      53    54.00                       vredsum.vs	v4, v8, v12
+# CHECK-NEXT:  1      53    53.00                       vredsum.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e8, m8, tu, mu
-# CHECK-NEXT:  1      61    62.00                       vredsum.vs	v4, v8, v12
+# CHECK-NEXT:  1      61    61.00                       vredsum.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e16, mf4, tu, mu
-# CHECK-NEXT:  1      41    42.00                       vredand.vs	v4, v8, v12
+# CHECK-NEXT:  1      41    41.00                       vredand.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e16, mf2, tu, mu
-# CHECK-NEXT:  1      41    42.00                       vredand.vs	v4, v8, v12
+# CHECK-NEXT:  1      41    41.00                       vredand.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e16, m1, tu, mu
-# CHECK-NEXT:  1      42    43.00                       vredand.vs	v4, v8, v12
+# CHECK-NEXT:  1      42    42.00                       vredand.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e16, m2, tu, mu
-# CHECK-NEXT:  1      44    45.00                       vredand.vs	v4, v8, v12
+# CHECK-NEXT:  1      44    44.00                       vredand.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e16, m4, tu, mu
-# CHECK-NEXT:  1      48    49.00                       vredand.vs	v4, v8, v12
+# CHECK-NEXT:  1      48    48.00                       vredand.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e16, m8, tu, mu
-# CHECK-NEXT:  1      56    57.00                       vredand.vs	v4, v8, v12
+# CHECK-NEXT:  1      56    56.00                       vredand.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e32, mf2, tu, mu
-# CHECK-NEXT:  1      36    37.00                       vredor.vs	v4, v8, v12
+# CHECK-NEXT:  1      36    36.00                       vredor.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e32, m1, tu, mu
-# CHECK-NEXT:  1      37    38.00                       vredor.vs	v4, v8, v12
+# CHECK-NEXT:  1      37    37.00                       vredor.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e32, m2, tu, mu
-# CHECK-NEXT:  1      39    40.00                       vredor.vs	v4, v8, v12
+# CHECK-NEXT:  1      39    39.00                       vredor.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e32, m4, tu, mu
-# CHECK-NEXT:  1      43    44.00                       vredor.vs	v4, v8, v12
+# CHECK-NEXT:  1      43    43.00                       vredor.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e32, m8, tu, mu
-# CHECK-NEXT:  1      51    52.00                       vredor.vs	v4, v8, v12
+# CHECK-NEXT:  1      51    51.00                       vredor.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e64, m1, tu, mu
-# CHECK-NEXT:  1      32    33.00                       vredxor.vs	v4, v8, v12
+# CHECK-NEXT:  1      32    32.00                       vredxor.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e64, m2, tu, mu
-# CHECK-NEXT:  1      34    35.00                       vredxor.vs	v4, v8, v12
+# CHECK-NEXT:  1      34    34.00                       vredxor.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e64, m4, tu, mu
-# CHECK-NEXT:  1      38    39.00                       vredxor.vs	v4, v8, v12
+# CHECK-NEXT:  1      38    38.00                       vredxor.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e64, m8, tu, mu
-# CHECK-NEXT:  1      46    47.00                       vredxor.vs	v4, v8, v12
+# CHECK-NEXT:  1      46    46.00                       vredxor.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e8, mf8, tu, mu
-# CHECK-NEXT:  1      46    47.00                       vredmaxu.vs	v4, v8, v12
+# CHECK-NEXT:  1      46    46.00                       vredmaxu.vs	v4, v8, v12
 # CHECK-NEXT:  1      3     1.00                  U     vsetvli	zero, zero, e8, mf4, tu, mu
-# CHECK...
[truncated]

@mshockwave mshockwave requested a review from fpetrogalli March 24, 2025 02:19
RKSimon
RKSimon reviewed Mar 24, 2025
View reviewed changes
llvm/lib/MC/MCSchedule.cpp Outdated
continue;
assert(I->ReleaseAtCycle > I->AcquireAtCycle);
Copy link
Collaborator

Choose a reason for hiding this comment

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

(style) assert message

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

llvm/lib/MC/MCSchedule.cpp
const MCSchedModel &SM = STI.getSchedModel();
const MCWriteProcResEntry *I = STI.getWriteProcResBegin(&SCDesc);
const MCWriteProcResEntry *E = STI.getWriteProcResEnd(&SCDesc);
for (; I != E; ++I) {
if (!I->ReleaseAtCycle)
if (!I->ReleaseAtCycle || I->ReleaseAtCycle == I->AcquireAtCycle)
Copy link
Collaborator

Choose a reason for hiding this comment

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

does I->ReleaseAtCycle == I->AcquireAtCycle actually happen?

Copy link
Member Author

Choose a reason for hiding this comment

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

does I->ReleaseAtCycle == I->AcquireAtCycle actually happen?

It's allowed:

llvm-project/llvm/lib/CodeGen/MachineScheduler.cpp

Line 4647 in 70c325b

// Zero resource usage is allowed by TargetSchedule.td but we do not construct

And it indeed happens:

llvm-project/llvm/lib/Target/AArch64/AArch64SchedA510.td

Line 507 in 70c325b

def : InstRW<[CortexA510MCWrite<4, 0, CortexA510UnitVMC>], (instrs PMULLv1i64, PMULLv2i64)>;

(the second template argument is ReleaseAtCycle, with AcquireAtCycle left to be default value of 0)

It's not clear to me why they (CortexA510) want both ReleaseAtCycle and AcquireAtCycle to be zero. Maybe they want to exclude these instructions completely from MachineScheduler.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe its allowed in the case that the resource needs to be available but does not use it. This could happen in a scenario where some other instruction might make it busy.

michaelmaitland
michaelmaitland approved these changes Mar 24, 2025
View reviewed changes
Copy link
Contributor

@michaelmaitland michaelmaitland left a comment

Choose a reason for hiding this comment

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

LGTM, pending @RKSimon's comment on assert message.

@mshockwave
fixup! Address review comments
3ede14c 
Add assertion message
@mshockwave mshockwave merged commit 7764847 into llvm:main Mar 24, 2025
6 of 10 checks passed
@mshockwave mshockwave deleted the patch/sched-rthroughput branch March 24, 2025 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RKSimon RKSimon RKSimon approved these changes

@michaelmaitland michaelmaitland michaelmaitland approved these changes

@fpetrogalli fpetrogalli Awaiting requested review from fpetrogalli

Assignees
No one assigned
Labels
mc Machine (object) code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mshockwave @llvmbot @RKSimon @michaelmaitland