[llvm] cd921e0 - [MISched] Do not erase resource booking history for subunits.

[Francesco Petrogalli via llvm-commits]

Author: Francesco Petrogalli
Date: 2023-08-01T14:00:37+02:00
New Revision: cd921e0fd79cd9bd2ab2ee1820476810847f5516

URL: https://github.com/llvm/llvm-project/commit/cd921e0fd79cd9bd2ab2ee1820476810847f5516
DIFF: https://github.com/llvm/llvm-project/commit/cd921e0fd79cd9bd2ab2ee1820476810847f5516.diff

LOG: [MISched] Do not erase resource booking history for subunits.

When dealing with the subunits of a resource group, we should reset
the subunits availability at the first avaiable cycle of the resource
that contains the subunits. Previously, the reset operation was
returning cycle 0, effectively erasing the booking history of the
subunits.

Without this change, when using intervals for models have make use of
subunits, the erasing of resource booking for subunits can raise the
assertion "A resource is being overwritten" in
`ResourceSegments::add`. The test added in the patch is one of such
cases.

Reviewed By: andreadb

Differential Revision: https://reviews.llvm.org/D156530

Added: 
    llvm/test/CodeGen/ARM/misched-prevent-erase-history-of-subunits.mir

Modified: 
    llvm/lib/CodeGen/MachineScheduler.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/MachineScheduler.cpp b/llvm/lib/CodeGen/MachineScheduler.cpp
index ba5432459d1282..556f0036bed3ec 100644
--- a/llvm/lib/CodeGen/MachineScheduler.cpp
+++ b/llvm/lib/CodeGen/MachineScheduler.cpp
@@ -2331,20 +2331,24 @@ SchedBoundary::getNextResourceCycle(const MCSchedClassDesc *SC, unsigned PIdx,
          "Cannot have zero instances of a ProcResource");
 
   if (isUnbufferedGroup(PIdx)) {
-    // If any subunits are used by the instruction, report that the resource
-    // group is available at 0, effectively removing the group record from
-    // hazarding and basing the hazarding decisions on the subunit records.
-    // Otherwise, choose the first available instance from among the subunits.
-    // Specifications which assign cycles to both the subunits and the group or
-    // which use an unbuffered group with buffered subunits will appear to
-    // schedule strangely. In the first case, the additional cycles for the
-    // group will be ignored.  In the second, the group will be ignored
-    // entirely.
+    // If any subunits are used by the instruction, report that the
+    // subunits of the resource group are available at the first cycle
+    // in which the unit is available, effectively removing the group
+    // record from hazarding and basing the hazarding decisions on the
+    // subunit records. Otherwise, choose the first available instance
+    // from among the subunits.  Specifications which assign cycles to
+    // both the subunits and the group or which use an unbuffered
+    // group with buffered subunits will appear to schedule
+    // strangely. In the first case, the additional cycles for the
+    // group will be ignored.  In the second, the group will be
+    // ignored entirely.
     for (const MCWriteProcResEntry &PE :
          make_range(SchedModel->getWriteProcResBegin(SC),
                     SchedModel->getWriteProcResEnd(SC)))
       if (ResourceGroupSubUnitMasks[PIdx][PE.ProcResourceIdx])
-        return std::make_pair(0u, StartIndex);
+        return std::make_pair(
+            getNextResourceCycleByInstance(StartIndex, Cycles, StartAtCycle),
+            StartIndex);
 
     auto SubUnits = SchedModel->getProcResource(PIdx)->SubUnitsIdxBegin;
     for (unsigned I = 0, End = NumberOfInstances; I < End; ++I) {

diff  --git a/llvm/test/CodeGen/ARM/misched-prevent-erase-history-of-subunits.mir b/llvm/test/CodeGen/ARM/misched-prevent-erase-history-of-subunits.mir
new file mode 100644
index 00000000000000..7134703851d28d
--- /dev/null
+++ b/llvm/test/CodeGen/ARM/misched-prevent-erase-history-of-subunits.mir
@@ -0,0 +1,37 @@
+# RUN: llc -mtriple=thumbv7em-unknown-unknown -mcpu=cortex-m7 \
+# RUN:    -sched-model-force-enable-intervals \
+# RUN:    %s -run-pass=machine-scheduler -o - 2>&1 -debug | FileCheck %s
+# REQUIRES: arm-registered-target, asserts
+
+# NOTE: Without the bugfix introduced in this code, the test below
+# would raise the assertion "A resource is being overwritten" from
+# `ResourceSegments::add` when enforcing the use of resource intervals
+# via -sched-model-force-enable-intervals with the scheduling model
+# for -mcpu=cortex-m7.
+
+# CHECK-LABEL: *** Final schedule for %bb.0 ***
+# CHECK: SU(0):   %0:dpr = COPY $d0
+# CHECK: SU(1):   %1:dpr = VABSD %0:dpr, 14, $noreg
+# CHECK: SU(2):   %2:dpr = VLDRD %const.0, 0, 14, $noreg :: (load (s64) from constant-pool)
+# CHECK: SU(4):   %3:rgpr = t2MOVi 0, 14, $noreg, $noreg
+# CHECK: SU(3):   VCMPD %1:dpr, %2:dpr, 14, $noreg, implicit-def $fpscr_nzcv
+# CHECK: SU(5):   $r0 = COPY %3:rgpr
+---
+name:            test
+tracksRegLiveness: true
+constants:
+  - id:              0
+    value:           double 0x7FF0000000000000
+body:             |
+  bb.0:
+    liveins: $d0
+
+    %0:dpr = COPY $d0
+    %1:dpr = VABSD %0, 14 /* CC::al */, $noreg
+    %2:dpr = VLDRD %const.0, 0, 14 /* CC::al */, $noreg :: (load (s64) from constant-pool)
+    VCMPD %1, %2, 14 /* CC::al */, $noreg, implicit-def $fpscr_nzcv
+    %4:rgpr = t2MOVi 0, 14 /* CC::al */, $noreg, $noreg
+    $r0 = COPY %4
+    tBX_RET 14 /* CC::al */, $noreg, implicit killed $r0
+
+...