[llvm] 63d1d37 - RegAllocGreedy: Avoid overflowing priority bitfields
Matt Arsenault via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 15 07:38:49 PDT 2022
Author: Matt Arsenault
Date: 2022-09-15T10:38:40-04:00
New Revision: 63d1d37d35e513632e9d4fcc1910ed4e9e3525d0
URL: https://github.com/llvm/llvm-project/commit/63d1d37d35e513632e9d4fcc1910ed4e9e3525d0
DIFF: https://github.com/llvm/llvm-project/commit/63d1d37d35e513632e9d4fcc1910ed4e9e3525d0.diff
LOG: RegAllocGreedy: Avoid overflowing priority bitfields
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.
Added:
Modified:
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
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
index 8439093210a19..d55f88dd50e57 100644
--- a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
@@ -55,10 +55,12 @@ class TargetRegisterClass {
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.
diff --git a/llvm/include/llvm/Target/Target.td b/llvm/include/llvm/Target/Target.td
index d865704f8b34e..55d9ff7b858c0 100644
--- a/llvm/include/llvm/Target/Target.td
+++ b/llvm/include/llvm/Target/Target.td
@@ -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;
diff --git a/llvm/lib/CodeGen/RegAllocGreedy.cpp b/llvm/lib/CodeGen/RegAllocGreedy.cpp
index dd6e20558b162..e745056758b3b 100644
--- a/llvm/lib/CodeGen/RegAllocGreedy.cpp
+++ b/llvm/lib/CodeGen/RegAllocGreedy.cpp
@@ -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
diff --git a/llvm/lib/Target/PowerPC/PPCRegisterInfoMMA.td b/llvm/lib/Target/PowerPC/PPCRegisterInfoMMA.td
index 0b6305f95a0af..57f4545516a00 100644
--- a/llvm/lib/Target/PowerPC/PPCRegisterInfoMMA.td
+++ b/llvm/lib/Target/PowerPC/PPCRegisterInfoMMA.td
@@ -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;
}
diff --git a/llvm/utils/TableGen/CodeGenRegisters.cpp b/llvm/utils/TableGen/CodeGenRegisters.cpp
index a35c2afaab9c5..999c720e6c303 100644
--- a/llvm/utils/TableGen/CodeGenRegisters.cpp
+++ b/llvm/utils/TableGen/CodeGenRegisters.cpp
@@ -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;
diff --git a/llvm/utils/TableGen/CodeGenRegisters.h b/llvm/utils/TableGen/CodeGenRegisters.h
index f269a52689ec7..55ac47e51a309 100644
--- a/llvm/utils/TableGen/CodeGenRegisters.h
+++ b/llvm/utils/TableGen/CodeGenRegisters.h
@@ -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;
diff --git a/llvm/utils/TableGen/RegisterInfoEmitter.cpp b/llvm/utils/TableGen/RegisterInfoEmitter.cpp
index 3f221884a26f5..208a89ab5549a 100644
--- a/llvm/utils/TableGen/RegisterInfoEmitter.cpp
+++ b/llvm/utils/TableGen/RegisterInfoEmitter.cpp
@@ -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 ";
More information about the llvm-commits
mailing list