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

[fpetrogalli]

Summary

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.

[fpetrogalli]

Update: once D156540 is in, I will update this change and add a test.

[fpetrogalli]

Add regression tests for the bugfix.

[andreadb]

LGTM

Ciao!

[fpetrogalli]

NFC. Describe the intent of the test in the patch with a comment.

llvm/lib/CodeGen/MachineScheduler.cpp

SchedBoundary::getNextResourceCycle(const MCSchedClassDesc *SC, unsigned PIdx,
   unsigned MinNextUnreserved = InvalidCycle;
   unsigned InstanceIdx = 0;
   unsigned StartIndex = ReservedCyclesIndex[PIdx];
   unsigned NumberOfInstances = SchedModel->getProcResource(PIdx)->NumUnits;
   assert(NumberOfInstances > 0 &&
          "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) {
       unsigned NextUnreserved, NextInstanceIdx;
       std::tie(NextUnreserved, NextInstanceIdx) =
           getNextResourceCycle(SC, SubUnits[I], Cycles, StartAtCycle);
       if (MinNextUnreserved > NextUnreserved) {
         InstanceIdx = NextInstanceIdx;

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

+# 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
+
+...