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.

Author: artagnon

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 <string>
-
 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<SmallVector<Entry, 8>, 4> Constraints;
+  MatrixStorage<Entry, 64> Constraints;
+
+  /// Constraints is only ever manipulated via this View.
+  JaggedArrayView<Entry, 16, 64> View;
 
   /// A map of variables (IR values) to their corresponding index in the
   /// constraint system.
@@ -64,18 +64,40 @@ class ConstraintSystem {
   SmallVector<std::string> getVarNamesList() const;
 
 public:
-  ConstraintSystem() {}
-  ConstraintSystem(ArrayRef<Value *> 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<Value *> FunctionArgs, size_t NCols)
+      : Constraints(NCols), View(Constraints) {
     NumVariables += FunctionArgs.size();
     for (auto *Arg : FunctionArgs) {
       Value2Index.insert({Arg, Value2Index.size() + 1});
     }
   }
-  ConstraintSystem(const DenseMap<Value *, unsigned> &Value2Index)
-      : NumVariables(Value2Index.size()), Value2Index(Value2Index) {}
+
+  // This constructor is only used by the dump function in
+  // ConstraintElimination.
+  ConstraintSystem(const DenseMap<Value *, unsigned> &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<int64_t> 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<int64_t, 8> R) const;
 
   SmallVector<int64_t> getLastConstraint() const {
-    assert(!Constraints.empty() && "Constraint system is empty");
+    assert(!View.empty() && "Constraint system is empty");
     SmallVector<int64_t> 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;

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 <string>
 
@@ -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<SmallVector<Entry, 8>, 4> RemainingRows;
-  for (unsigned R1 = 0; R1 < Constraints.size();) {
-    SmallVector<Entry, 8> &Row1 = Constraints[R1];
+  MatrixStorage<Entry, 32> RemainingRows(View.getMaxColSpan());
+  JaggedArrayView<Entry, 8, 32> 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<Entry, 8> 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<std::string> ConstraintSystem::getVarNamesList() const {
 
 void ConstraintSystem::dump() const {
 #ifndef NDEBUG
-  if (Constraints.empty())
+  if (View.empty())
     return;
   SmallVector<std::string> Names = getVarNamesList();
-  for (const auto &Row : Constraints) {
+  for (const auto &Row : View) {
     SmallVector<std::string, 16> Parts;
     for (const Entry &E : Row) {
       if (E.Id >= NumVariables)

llvm/lib/Transforms/Scalar/ConstraintElimination.cpp

@@ -40,7 +40,6 @@
 #include "llvm/Transforms/Utils/Cloning.h"
 #include "llvm/Transforms/Utils/ValueMapper.h"
 
-#include <cmath>
 #include <optional>
 #include <string>
 
@@ -57,6 +56,10 @@ static cl::opt<unsigned>
     MaxRows("constraint-elimination-max-rows", cl::init(500), cl::Hidden,
             cl::desc("Maximum number of rows to keep in constraint system"));
 
+static cl::opt<unsigned> MaxColumns(
+    "constraint-elimination-max-cols", cl::init(16), cl::Hidden,
+    cl::desc("Maximum number of columns to keep in constraint system"));
+
 static cl::opt<bool> 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<Value *> 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<int64_t> C,
                            const DenseMap<Value *, unsigned> &Value2Index) {
-  ConstraintSystem CS(Value2Index);
+  ConstraintSystem CS(Value2Index, C.size());
   CS.addVariableRowFill(C);
   CS.dump();
 }