RegAllocGreedy: Avoid overflowing priority bitfields
authorMatt Arsenault <Matthew.Arsenault@amd.com>
Sat, 23 Jul 2022 14:13:25 +0000 (10:13 -0400)
committerMatt Arsenault <Matthew.Arsenault@amd.com>
Thu, 15 Sep 2022 14:38:40 +0000 (10:38 -0400)
The class priority is expected to be at most 5 bits before it starts
clobbering bits used for other fields. Also clamp the instruction
distance in case we have millions of instructions.

AMDGPU was accidentally overflowing into the global priority bit in
some cases. I think in principal we would have wanted this, but in the
cases I've looked at, it had the counter intuitive effect and
de-prioritized the large register tuple.

Avoid using weird bit hack PPC uses for global priority. The
AllocationPriority field is really 5 bits, and PPC was relying on
overflowing this to 6-bits to forcibly set the global priority
bit. Split this out as a separate flag to avoid having magic behavior
for values above 31.

llvm/include/llvm/CodeGen/TargetRegisterInfo.h
llvm/include/llvm/Target/Target.td
llvm/lib/CodeGen/RegAllocGreedy.cpp
llvm/lib/Target/PowerPC/PPCRegisterInfoMMA.td
llvm/utils/TableGen/CodeGenRegisters.cpp
llvm/utils/TableGen/CodeGenRegisters.h
llvm/utils/TableGen/RegisterInfoEmitter.cpp

index 8439093..d55f88d 100644 (file)
@@ -55,10 +55,12 @@ public:
   const uint16_t *SuperRegIndices;
   const LaneBitmask LaneMask;
   /// Classes with a higher priority value are assigned first by register
-  /// allocators using a greedy heuristic. The value is in the range [0,63].
-  /// Values >= 32 should be used with care since they may overlap with other
-  /// fields in the allocator's priority heuristics.
+  /// allocators using a greedy heuristic. The value is in the range [0,31].
   const uint8_t AllocationPriority;
+
+  // Change allocation priority heuristic used by greedy.
+  const bool GlobalPriority;
+
   /// Configurable target specific flags.
   const uint8_t TSFlags;
   /// Whether the class supports two (or more) disjunct subregister indices.
index d865704..55d9ff7 100644 (file)
@@ -282,11 +282,14 @@ class RegisterClass<string namespace, list<ValueType> regTypes, int alignment,
   // Specify allocation priority for register allocators using a greedy
   // heuristic. Classes with higher priority values are assigned first. This is
   // useful as it is sometimes beneficial to assign registers to highly
-  // constrained classes first. The value has to be in the range [0,63].
-  // Values >= 32 should be used with care since they may overlap with other
-  // fields in the allocator's priority heuristics.
+  // constrained classes first. The value has to be in the range [0,31].
   int AllocationPriority = 0;
 
+  // Force register class to use greedy's global heuristic for all
+  // registers in this class. This should more aggressively try to
+  // avoid spilling in pathological cases.
+  bit GlobalPriority = false;
+
   // Generate register pressure set for this register class and any class
   // synthesized from it. Set to 0 to inhibit unneeded pressure sets.
   bit GeneratePressureSet = true;
index dd6e205..e745056 100644 (file)
@@ -319,9 +319,10 @@ unsigned DefaultPriorityAdvisor::getPriority(const LiveInterval &LI) const {
     // Giant live ranges fall back to the global assignment heuristic, which
     // prevents excessive spilling in pathological cases.
     const TargetRegisterClass &RC = *MRI->getRegClass(Reg);
-    bool ForceGlobal = !ReverseLocalAssignment &&
-                       (Size / SlotIndex::InstrDist) >
-                           (2 * RegClassInfo.getNumAllocatableRegs(&RC));
+    bool ForceGlobal = RC.GlobalPriority ||
+                       (!ReverseLocalAssignment &&
+                        (Size / SlotIndex::InstrDist) >
+                            (2 * RegClassInfo.getNumAllocatableRegs(&RC)));
     unsigned GlobalBit = 0;
 
     if (Stage == RS_Assign && !ForceGlobal && !LI.empty() &&
@@ -344,6 +345,22 @@ unsigned DefaultPriorityAdvisor::getPriority(const LiveInterval &LI) const {
       Prio = Size;
       GlobalBit = 1;
     }
+
+    // Priority bit layout:
+    // 31 RS_Assign priority
+    // 30 Preference priority
+    // if (RegClassPriorityTrumpsGlobalness)
+    //   29-25 AllocPriority
+    //   24 GlobalBit
+    // else
+    //   29 Global bit
+    //   28-24 AllocPriority
+    // 0-23 Size/Instr distance
+
+    // Clamp the size to fit with the priority masking scheme
+    Prio = std::min(Prio, (unsigned)maxUIntN(24));
+    assert(isUInt<5>(RC.AllocationPriority) && "allocation priority overflow");
+
     if (RegClassPriorityTrumpsGlobalness)
       Prio |= RC.AllocationPriority << 25 | GlobalBit << 24;
     else
index 0b6305f..57f4545 100644 (file)
@@ -47,13 +47,16 @@ let SubRegIndices = [sub_pair0, sub_pair1] in {
 }
 def ACCRC : RegisterClass<"PPC", [v512i1], 128, (add ACC0, ACC1, ACC2, ACC3,
                                                       ACC4, ACC5, ACC6, ACC7)> {
-  // The AllocationPriority is in the range [0, 63]. Assigned the ACC registers
+  // The AllocationPriority is in the range [0, 31]. Assigned the ACC registers
   // the highest possible priority in this range to force the register allocator
   // to assign these registers first. This is done because the ACC registers
   // must represent 4 advacent vector registers. For example ACC1 must be
-  // VS4 - VS7. The value here must be at least 32 as we want to allocate
-  // these registers even before we allocate global ranges.
-  let AllocationPriority = 63;
+  // VS4 - VS7.
+  let AllocationPriority = 31;
+
+  // We want to allocate these registers even before we allocate
+  // global ranges.
+  let GlobalPriority = true;
   let Size = 512;
 }
 
@@ -74,7 +77,8 @@ def UACCRC : RegisterClass<"PPC", [v512i1], 128,
   // least 32 as we want to allocate these registers before we allocate other
   // global ranges. The value must be less than the AllocationPriority of the
   // ACC registers.
-  let AllocationPriority = 36;
+  let AllocationPriority = 4;
+  let GlobalPriority = true;
   let Size = 512;
 }
 
index a35c2af..999c720 100644 (file)
@@ -803,10 +803,12 @@ CodeGenRegisterClass::CodeGenRegisterClass(CodeGenRegBank &RegBank, Record *R)
   Allocatable = R->getValueAsBit("isAllocatable");
   AltOrderSelect = R->getValueAsString("AltOrderSelect");
   int AllocationPriority = R->getValueAsInt("AllocationPriority");
-  if (AllocationPriority < 0 || AllocationPriority > 63)
-    PrintFatalError(R->getLoc(), "AllocationPriority out of range [0,63]");
+  if (!isUInt<5>(AllocationPriority))
+    PrintFatalError(R->getLoc(), "AllocationPriority out of range [0,31]");
   this->AllocationPriority = AllocationPriority;
 
+  GlobalPriority = R->getValueAsBit("GlobalPriority");
+
   BitsInit *TSF = R->getValueAsBitsInit("TSFlags");
   for (unsigned I = 0, E = TSF->getNumBits(); I != E; ++I) {
     BitInit *Bit = cast<BitInit>(TSF->getBit(I));
@@ -821,7 +823,8 @@ CodeGenRegisterClass::CodeGenRegisterClass(CodeGenRegBank &RegBank,
                                            StringRef Name, Key Props)
     : Members(*Props.Members), TheDef(nullptr), Name(std::string(Name)),
       TopoSigs(RegBank.getNumTopoSigs()), EnumValue(-1), RSI(Props.RSI),
-      CopyCost(0), Allocatable(true), AllocationPriority(0), TSFlags(0) {
+      CopyCost(0), Allocatable(true), AllocationPriority(0),
+      GlobalPriority(false), TSFlags(0) {
   Artificial = true;
   GeneratePressureSet = false;
   for (const auto R : Members) {
@@ -849,6 +852,7 @@ void CodeGenRegisterClass::inheritProperties(CodeGenRegBank &RegBank) {
   });
   AltOrderSelect = Super.AltOrderSelect;
   AllocationPriority = Super.AllocationPriority;
+  GlobalPriority = Super.GlobalPriority;
   TSFlags = Super.TSFlags;
   GeneratePressureSet |= Super.GeneratePressureSet;
 
index f269a52..55ac47e 100644 (file)
@@ -332,6 +332,7 @@ namespace llvm {
     bool Allocatable;
     StringRef AltOrderSelect;
     uint8_t AllocationPriority;
+    bool GlobalPriority;
     uint8_t TSFlags;
     /// Contains the combination of the lane masks of all subregisters.
     LaneBitmask LaneMask;
index 3f22188..208a89a 100644 (file)
@@ -1428,10 +1428,11 @@ RegisterInfoEmitter::runTargetDesc(raw_ostream &OS, CodeGenTarget &Target,
          << SuperRegIdxSeqs.get(SuperRegIdxLists[RC.EnumValue]) << ",\n    ";
       printMask(OS, RC.LaneMask);
       OS << ",\n    " << (unsigned)RC.AllocationPriority << ",\n    "
+         << (RC.GlobalPriority ? "true" : "false") << ",\n    "
          << format("0x%02x", RC.TSFlags) << ", /* TSFlags */\n    "
-         << (RC.HasDisjunctSubRegs?"true":"false")
+         << (RC.HasDisjunctSubRegs ? "true" : "false")
          << ", /* HasDisjunctSubRegs */\n    "
-         << (RC.CoveredBySubRegs?"true":"false")
+         << (RC.CoveredBySubRegs ? "true" : "false")
          << ", /* CoveredBySubRegs */\n    ";
       if (RC.getSuperClasses().empty())
         OS << "NullRegClasses,\n    ";