Skip to content

Commit

Permalink
[CodeGen] Define ABI breaking class members correctly
Browse files Browse the repository at this point in the history
Non-static class members declared under #ifndef NDEBUG should be declared
under #if LLVM_ENABLE_ABI_BREAKING_CHECKS to make headers library-friendly and
allow cross-linking, as discussed in D120714.

Differential Revision: https://reviews.llvm.org/D121549
  • Loading branch information
kovdan01 committed Mar 24, 2022
1 parent 828b63c commit c53cbce
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 31 deletions.
12 changes: 9 additions & 3 deletions llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class GISelWorkList {
SmallVector<MachineInstr *, N> Worklist;
DenseMap<MachineInstr *, unsigned> WorklistMap;

#ifndef NDEBUG
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
bool Finalized = true;
#endif

Expand All @@ -49,7 +49,7 @@ class GISelWorkList {
// of most passes.
void deferred_insert(MachineInstr *I) {
Worklist.push_back(I);
#ifndef NDEBUG
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
Finalized = false;
#endif
}
Expand All @@ -65,21 +65,25 @@ class GISelWorkList {
for (unsigned i = 0; i < Worklist.size(); ++i)
if (!WorklistMap.try_emplace(Worklist[i], i).second)
llvm_unreachable("Duplicate elements in the list");
#ifndef NDEBUG
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
Finalized = true;
#endif
}

/// Add the specified instruction to the worklist if it isn't already in it.
void insert(MachineInstr *I) {
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
assert(Finalized && "GISelWorkList used without finalizing");
#endif
if (WorklistMap.try_emplace(I, Worklist.size()).second)
Worklist.push_back(I);
}

/// Remove I from the worklist if it exists.
void remove(const MachineInstr *I) {
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
assert((Finalized || WorklistMap.empty()) && "Neither finalized nor empty");
#endif
auto It = WorklistMap.find(I);
if (It == WorklistMap.end())
return; // Not in worklist.
Expand All @@ -96,7 +100,9 @@ class GISelWorkList {
}

MachineInstr *pop_back_val() {
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
assert(Finalized && "GISelWorkList used without finalizing");
#endif
MachineInstr *I;
do {
I = Worklist.pop_back_val();
Expand Down
4 changes: 2 additions & 2 deletions llvm/include/llvm/CodeGen/MachineScheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ class ScheduleDAGMI : public ScheduleDAGInstrs {
const SUnit *NextClusterPred = nullptr;
const SUnit *NextClusterSucc = nullptr;

#ifndef NDEBUG
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
/// The number of instructions scheduled so far. Used to cut off the
/// scheduler at the point determined by misched-cutoff.
unsigned NumInstrsScheduled = 0;
Expand Down Expand Up @@ -679,7 +679,7 @@ class SchedBoundary {
// For each PIdx, stores the resource group IDs of its subunits
SmallVector<APInt, 16> ResourceGroupSubUnitMasks;

#ifndef NDEBUG
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
// Remember the greatest possible stall as an upper bound on the number of
// times we should retry the pending queue because of a hazard.
unsigned MaxObservedStall;
Expand Down
2 changes: 1 addition & 1 deletion llvm/include/llvm/CodeGen/PBQP/ReductionRules.h
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ namespace PBQP {

RawVector v = G.getNodeCosts(NId);

#ifndef NDEBUG
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
// Although a conservatively allocatable node can be allocated to a register,
// spilling it may provide a lower cost solution. Assert here that spilling
// is done by choice, not because there were no register available.
Expand Down
17 changes: 9 additions & 8 deletions llvm/include/llvm/CodeGen/RegAllocPBQP.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,12 @@ class NodeMetadata {
NodeMetadata() = default;

NodeMetadata(const NodeMetadata &Other)
: RS(Other.RS), NumOpts(Other.NumOpts), DeniedOpts(Other.DeniedOpts),
OptUnsafeEdges(new unsigned[NumOpts]), VReg(Other.VReg),
AllowedRegs(Other.AllowedRegs)
#ifndef NDEBUG
, everConservativelyAllocatable(Other.everConservativelyAllocatable)
: RS(Other.RS), NumOpts(Other.NumOpts), DeniedOpts(Other.DeniedOpts),
OptUnsafeEdges(new unsigned[NumOpts]), VReg(Other.VReg),
AllowedRegs(Other.AllowedRegs)
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
,
everConservativelyAllocatable(Other.everConservativelyAllocatable)
#endif
{
if (NumOpts > 0) {
Expand Down Expand Up @@ -217,7 +218,7 @@ class NodeMetadata {
assert(RS >= this->RS && "A node's reduction state can not be downgraded");
this->RS = RS;

#ifndef NDEBUG
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
// Remember this state to assert later that a non-infinite register
// option was available.
if (RS == ConservativelyAllocatable)
Expand Down Expand Up @@ -247,7 +248,7 @@ class NodeMetadata {
&OptUnsafeEdges[NumOpts]);
}

#ifndef NDEBUG
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
bool wasConservativelyAllocatable() const {
return everConservativelyAllocatable;
}
Expand All @@ -261,7 +262,7 @@ class NodeMetadata {
Register VReg;
GraphMetadata::AllowedRegVecRef AllowedRegs;

#ifndef NDEBUG
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
bool everConservativelyAllocatable = false;
#endif
};
Expand Down
2 changes: 1 addition & 1 deletion llvm/include/llvm/CodeGen/SelectionDAG.h
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ class SelectionDAG {
void viewGraph(const std::string &Title);
void viewGraph();

#ifndef NDEBUG
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
std::map<const SDNode *, std::string> NodeGraphAttrs;
#endif

Expand Down
8 changes: 4 additions & 4 deletions llvm/lib/CodeGen/MachineScheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,7 @@ void ScheduleDAGMI::moveInstruction(
}

bool ScheduleDAGMI::checkSchedLimit() {
#ifndef NDEBUG
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
if (NumInstrsScheduled == MISchedCutoff && MISchedCutoff != ~0U) {
CurrentTop = CurrentBottom;
return false;
Expand Down Expand Up @@ -2005,7 +2005,7 @@ void SchedBoundary::reset() {
ReservedCycles.clear();
ReservedCyclesIndex.clear();
ResourceGroupSubUnitMasks.clear();
#ifndef NDEBUG
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
// Track the maximum number of stall cycles that could arise either from the
// latency of a DAG edge or the number of cycles that a processor resource is
// reserved (SchedBoundary::ReservedCycles).
Expand Down Expand Up @@ -2193,7 +2193,7 @@ bool SchedBoundary::checkHazard(SUnit *SU) {
unsigned NRCycle, InstanceIdx;
std::tie(NRCycle, InstanceIdx) = getNextResourceCycle(SC, ResIdx, Cycles);
if (NRCycle > CurrCycle) {
#ifndef NDEBUG
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
MaxObservedStall = std::max(Cycles, MaxObservedStall);
#endif
LLVM_DEBUG(dbgs() << " SU(" << SU->NodeNum << ") "
Expand Down Expand Up @@ -2260,7 +2260,7 @@ void SchedBoundary::releaseNode(SUnit *SU, unsigned ReadyCycle, bool InPQueue,
unsigned Idx) {
assert(SU->getInstr() && "Scheduled SUnit must have instr");

#ifndef NDEBUG
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
// ReadyCycle was been bumped up to the CurrCycle when this node was
// scheduled, but CurrCycle may have been eagerly advanced immediately after
// scheduling, so may now be greater than ReadyCycle.
Expand Down
24 changes: 12 additions & 12 deletions llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -177,31 +177,31 @@ LLVM_DUMP_METHOD void SelectionDAG::dumpDotGraph(const Twine &FileName,
/// clearGraphAttrs - Clear all previously defined node graph attributes.
/// Intended to be used from a debugging tool (eg. gdb).
void SelectionDAG::clearGraphAttrs() {
#ifndef NDEBUG
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
NodeGraphAttrs.clear();
#else
errs() << "SelectionDAG::clearGraphAttrs is only available in debug builds"
<< " on systems with Graphviz or gv!\n";
errs() << "SelectionDAG::clearGraphAttrs is only available in builds with "
<< "ABI breaking checks enabled on systems with Graphviz or gv!\n";
#endif
}


/// setGraphAttrs - Set graph attributes for a node. (eg. "color=red".)
///
void SelectionDAG::setGraphAttrs(const SDNode *N, const char *Attrs) {
#ifndef NDEBUG
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
NodeGraphAttrs[N] = Attrs;
#else
errs() << "SelectionDAG::setGraphAttrs is only available in debug builds"
<< " on systems with Graphviz or gv!\n";
errs() << "SelectionDAG::setGraphAttrs is only available in builds with "
<< "ABI breaking checks enabled on systems with Graphviz or gv!\n";
#endif
}


/// getGraphAttrs - Get graph attributes for a node. (eg. "color=red".)
/// Used from getNodeAttributes.
std::string SelectionDAG::getGraphAttrs(const SDNode *N) const {
#ifndef NDEBUG
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
std::map<const SDNode *, std::string>::const_iterator I =
NodeGraphAttrs.find(N);

Expand All @@ -210,20 +210,20 @@ std::string SelectionDAG::getGraphAttrs(const SDNode *N) const {
else
return "";
#else
errs() << "SelectionDAG::getGraphAttrs is only available in debug builds"
<< " on systems with Graphviz or gv!\n";
errs() << "SelectionDAG::getGraphAttrs is only available in builds with "
<< "ABI breaking checks enabled on systems with Graphviz or gv!\n";
return std::string();
#endif
}

/// setGraphColor - Convenience for setting node color attribute.
///
void SelectionDAG::setGraphColor(const SDNode *N, const char *Color) {
#ifndef NDEBUG
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
NodeGraphAttrs[N] = std::string("color=") + Color;
#else
errs() << "SelectionDAG::setGraphColor is only available in debug builds"
<< " on systems with Graphviz or gv!\n";
errs() << "SelectionDAG::setGraphColor is only available in builds with "
<< "ABI breaking checks enabled on systems with Graphviz or gv!\n";
#endif
}

Expand Down

0 comments on commit c53cbce

Please sign in to comment.