From f78bc6008c7dea8acce3a4171dd3ece15103419f Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Mon, 15 Jul 2024 12:34:01 +0100 Subject: [PATCH] ConstraintSystem: replace data structure with Matrix ConstraintSystem currently stores constraints in a vector-of-vectors, performing inefficient memory operations on it, as part of its operation. Replace this data structure with the newly-minted Matrix, using MatrixView to eliminate row-swaps and truncation of the underlying storage. Since Matrix requires knowing an upper bounds on the number of columns ahead-of-time, add a cl::opt to ConstraintElimination upper-bounding this, and change addVariableRowFill to not add constraints whose length exceeds this upper bound. --- llvm/include/llvm/Analysis/ConstraintSystem.h | 66 +++++++++++++------ llvm/lib/Analysis/ConstraintSystem.cpp | 42 ++++++------ .../Scalar/ConstraintElimination.cpp | 10 ++- 3 files changed, 76 insertions(+), 42 deletions(-) diff --git a/llvm/include/llvm/Analysis/ConstraintSystem.h b/llvm/include/llvm/Analysis/ConstraintSystem.h index 7b02b618f7cb44..6014b2cb642471 100644 --- a/llvm/include/llvm/Analysis/ConstraintSystem.h +++ b/llvm/include/llvm/Analysis/ConstraintSystem.h @@ -9,22 +9,19 @@ #ifndef LLVM_ANALYSIS_CONSTRAINTSYSTEM_H #define LLVM_ANALYSIS_CONSTRAINTSYSTEM_H -#include "llvm/ADT/APInt.h" -#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseMap.h" -#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/Matrix.h" #include "llvm/Support/MathExtras.h" -#include - namespace llvm { class Value; class ConstraintSystem { struct Entry { - int64_t Coefficient; - uint16_t Id; + int64_t Coefficient = 0; + uint16_t Id = 0; + Entry() = default; Entry(int64_t Coefficient, uint16_t Id) : Coefficient(Coefficient), Id(Id) {} }; @@ -48,7 +45,10 @@ class ConstraintSystem { /// Current linear constraints in the system. /// An entry of the form c0, c1, ... cn represents the following constraint: /// c0 >= v0 * c1 + .... + v{n-1} * cn - SmallVector, 4> Constraints; + MatrixStorage Constraints; + + /// Constraints is only ever manipulated via this View. + JaggedArrayView View; /// A map of variables (IR values) to their corresponding index in the /// constraint system. @@ -64,18 +64,40 @@ class ConstraintSystem { SmallVector getVarNamesList() const; public: - ConstraintSystem() {} - ConstraintSystem(ArrayRef FunctionArgs) { + // The Matrix Constraints should always be initialized with an upper-bound + // number of columns. The default constructor hard-codes an upper-bound of 6, + // as it is only used in unit tests, and not in the actual + // ConstraintElimination Analysis. + ConstraintSystem() : Constraints(6), View(Constraints) {} + + // This constructor is used by ConstraintElimination, inside ConstraintInfo. + // Unfortunately, due to calls to addFact, that adds local variables, it is + // impossible to know how many local variables there are in advance. + // ConstraintElimination has a fixed upper-bound on the number of columns, + // configurable as a cl::opt, so use that number, and don't add the constraint + // if it exceeds that number. + ConstraintSystem(ArrayRef FunctionArgs, size_t NCols) + : Constraints(NCols), View(Constraints) { NumVariables += FunctionArgs.size(); for (auto *Arg : FunctionArgs) { Value2Index.insert({Arg, Value2Index.size() + 1}); } } - ConstraintSystem(const DenseMap &Value2Index) - : NumVariables(Value2Index.size()), Value2Index(Value2Index) {} + + // This constructor is only used by the dump function in + // ConstraintElimination. + ConstraintSystem(const DenseMap &Value2Index, + unsigned NVars) + : NumVariables(Value2Index.size()), + Constraints(std::max(Value2Index.size(), NVars)), View(Constraints), + Value2Index(Value2Index) {} + + ConstraintSystem(const ConstraintSystem &Other) + : NumVariables(Other.NumVariables), Constraints(Other.Constraints), + View(Other.View, Constraints), Value2Index(Other.Value2Index) {} bool addVariableRow(ArrayRef R) { - assert(Constraints.empty() || R.size() == NumVariables); + assert(View.empty() || R.size() == NumVariables); // If all variable coefficients are 0, the constraint does not provide any // usable information. if (all_of(ArrayRef(R).drop_front(1), [](int64_t C) { return C == 0; })) @@ -87,9 +109,10 @@ class ConstraintSystem { continue; NewRow.emplace_back(C, Idx); } - if (Constraints.empty()) + if (View.empty()) NumVariables = R.size(); - Constraints.push_back(std::move(NewRow)); + + View.addRow(std::move(NewRow)); return true; } @@ -104,6 +127,11 @@ class ConstraintSystem { if (all_of(ArrayRef(R).drop_front(1), [](int64_t C) { return C == 0; })) return false; + // There is no correctness issue if we don't add a constraint, for whatever + // reason. + if (R.size() > Constraints.getNumCols()) + return false; + NumVariables = std::max(R.size(), NumVariables); return addVariableRow(R); } @@ -145,21 +173,21 @@ class ConstraintSystem { bool isConditionImplied(SmallVector R) const; SmallVector getLastConstraint() const { - assert(!Constraints.empty() && "Constraint system is empty"); + assert(!View.empty() && "Constraint system is empty"); SmallVector Result(NumVariables, 0); - for (auto &Entry : Constraints.back()) + for (auto &Entry : View.lastRow()) Result[Entry.Id] = Entry.Coefficient; return Result; } - void popLastConstraint() { Constraints.pop_back(); } + void popLastConstraint() { View.dropLastRow(); } void popLastNVariables(unsigned N) { assert(NumVariables > N); NumVariables -= N; } /// Returns the number of rows in the constraint system. - unsigned size() const { return Constraints.size(); } + unsigned size() const { return View.getRowSpan(); } /// Print the constraints in the system. void dump() const; diff --git a/llvm/lib/Analysis/ConstraintSystem.cpp b/llvm/lib/Analysis/ConstraintSystem.cpp index e4c9dcc7544e99..0b08a8ffeca2ea 100644 --- a/llvm/lib/Analysis/ConstraintSystem.cpp +++ b/llvm/lib/Analysis/ConstraintSystem.cpp @@ -7,11 +7,12 @@ //===----------------------------------------------------------------------===// #include "llvm/Analysis/ConstraintSystem.h" +#include "llvm/ADT/Matrix.h" #include "llvm/ADT/SmallVector.h" -#include "llvm/Support/MathExtras.h" #include "llvm/ADT/StringExtras.h" #include "llvm/IR/Value.h" #include "llvm/Support/Debug.h" +#include "llvm/Support/MathExtras.h" #include @@ -26,37 +27,38 @@ bool ConstraintSystem::eliminateUsingFM() { // analysis." // Supercomputing'91: Proceedings of the 1991 ACM/ // IEEE conference on Supercomputing. IEEE, 1991. - assert(!Constraints.empty() && + assert(!View.empty() && "should only be called for non-empty constraint systems"); unsigned LastIdx = NumVariables - 1; // First, either remove the variable in place if it is 0 or add the row to // RemainingRows and remove it from the system. - SmallVector, 4> RemainingRows; - for (unsigned R1 = 0; R1 < Constraints.size();) { - SmallVector &Row1 = Constraints[R1]; + MatrixStorage RemainingRows(View.getMaxColSpan()); + JaggedArrayView RemainingRowsView{RemainingRows}; + for (unsigned R1 = 0; R1 < View.getRowSpan();) { + auto &Row1 = View[R1]; if (getLastCoefficient(Row1, LastIdx) == 0) { if (Row1.size() > 0 && Row1.back().Id == LastIdx) Row1.pop_back(); R1++; } else { - std::swap(Constraints[R1], Constraints.back()); - RemainingRows.push_back(std::move(Constraints.back())); - Constraints.pop_back(); + std::swap(View[R1], View.lastRow()); + RemainingRowsView.addRow(View.lastRow()); + View.dropLastRow(); } } // Process rows where the variable is != 0. - unsigned NumRemainingConstraints = RemainingRows.size(); + unsigned NumRemainingConstraints = RemainingRowsView.getRowSpan(); for (unsigned R1 = 0; R1 < NumRemainingConstraints; R1++) { // FIXME do not use copy for (unsigned R2 = R1 + 1; R2 < NumRemainingConstraints; R2++) { if (R1 == R2) continue; - int64_t UpperLast = getLastCoefficient(RemainingRows[R2], LastIdx); - int64_t LowerLast = getLastCoefficient(RemainingRows[R1], LastIdx); + int64_t UpperLast = getLastCoefficient(RemainingRowsView[R2], LastIdx); + int64_t LowerLast = getLastCoefficient(RemainingRowsView[R1], LastIdx); assert( UpperLast != 0 && LowerLast != 0 && "RemainingRows should only contain rows where the variable is != 0"); @@ -74,8 +76,8 @@ bool ConstraintSystem::eliminateUsingFM() { SmallVector NR; unsigned IdxUpper = 0; unsigned IdxLower = 0; - auto &LowerRow = RemainingRows[LowerR]; - auto &UpperRow = RemainingRows[UpperR]; + auto &LowerRow = RemainingRowsView[LowerR]; + auto &UpperRow = RemainingRowsView[UpperR]; while (true) { if (IdxUpper >= UpperRow.size() || IdxLower >= LowerRow.size()) break; @@ -112,9 +114,9 @@ bool ConstraintSystem::eliminateUsingFM() { } if (NR.empty()) continue; - Constraints.push_back(std::move(NR)); + View.addRow(std::move(NR)); // Give up if the new system gets too big. - if (Constraints.size() > 500) + if (size() > 500) return false; } } @@ -124,15 +126,15 @@ bool ConstraintSystem::eliminateUsingFM() { } bool ConstraintSystem::mayHaveSolutionImpl() { - while (!Constraints.empty() && NumVariables > 1) { + while (!View.empty() && NumVariables > 1) { if (!eliminateUsingFM()) return true; } - if (Constraints.empty() || NumVariables > 1) + if (View.empty() || NumVariables > 1) return true; - return all_of(Constraints, [](auto &R) { + return all_of(View, [](auto &R) { if (R.empty()) return true; if (R[0].Id == 0) @@ -158,10 +160,10 @@ SmallVector ConstraintSystem::getVarNamesList() const { void ConstraintSystem::dump() const { #ifndef NDEBUG - if (Constraints.empty()) + if (View.empty()) return; SmallVector Names = getVarNamesList(); - for (const auto &Row : Constraints) { + for (const auto &Row : View) { SmallVector Parts; for (const Entry &E : Row) { if (E.Id >= NumVariables) diff --git a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp index c31173879af1e6..ff13302abc5530 100644 --- a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp +++ b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp @@ -40,7 +40,6 @@ #include "llvm/Transforms/Utils/Cloning.h" #include "llvm/Transforms/Utils/ValueMapper.h" -#include #include #include @@ -57,6 +56,10 @@ static cl::opt MaxRows("constraint-elimination-max-rows", cl::init(500), cl::Hidden, cl::desc("Maximum number of rows to keep in constraint system")); +static cl::opt MaxColumns( + "constraint-elimination-max-cols", cl::init(16), cl::Hidden, + cl::desc("Maximum number of columns to keep in constraint system")); + static cl::opt DumpReproducers( "constraint-elimination-dump-reproducers", cl::init(false), cl::Hidden, cl::desc("Dump IR to reproduce successful transformations.")); @@ -274,7 +277,8 @@ class ConstraintInfo { public: ConstraintInfo(const DataLayout &DL, ArrayRef FunctionArgs) - : UnsignedCS(FunctionArgs), SignedCS(FunctionArgs), DL(DL) { + : UnsignedCS(FunctionArgs, MaxColumns), + SignedCS(FunctionArgs, MaxColumns), DL(DL) { auto &Value2Index = getValue2Index(false); // Add Arg > -1 constraints to unsigned system for all function arguments. for (Value *Arg : FunctionArgs) { @@ -894,7 +898,7 @@ void ConstraintInfo::transferToOtherSystem( static void dumpConstraint(ArrayRef C, const DenseMap &Value2Index) { - ConstraintSystem CS(Value2Index); + ConstraintSystem CS(Value2Index, C.size()); CS.addVariableRowFill(C); CS.dump(); }