[llvm] ConstraintSystem: replace data structure with Matrix (PR #98895)

Ramkumar Ramachandra via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 15 05:58:37 PDT 2024


https://github.com/artagnon created https://github.com/llvm/llvm-project/pull/98895

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.

-- 8< --
Based on #98893.

>From 04f97f6f13f3fa1e239402f4628ab716c8bbecbc Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Mon, 15 Jul 2024 09:30:24 +0100
Subject: [PATCH 1/3] ArrayRef: add missing constexpr annotations (NFC)

---
 llvm/include/llvm/ADT/ArrayRef.h | 97 +++++++++++++++++---------------
 1 file changed, 53 insertions(+), 44 deletions(-)

diff --git a/llvm/include/llvm/ADT/ArrayRef.h b/llvm/include/llvm/ADT/ArrayRef.h
index 1c6799f1c56ed..8c487e5b41ebf 100644
--- a/llvm/include/llvm/ADT/ArrayRef.h
+++ b/llvm/include/llvm/ADT/ArrayRef.h
@@ -64,14 +64,14 @@ namespace llvm {
     /// @{
 
     /// Construct an empty ArrayRef.
-    /*implicit*/ ArrayRef() = default;
+    /*implicit*/ constexpr ArrayRef() = default;
 
     /// Construct an empty ArrayRef from std::nullopt.
-    /*implicit*/ ArrayRef(std::nullopt_t) {}
+    /*implicit*/ constexpr ArrayRef(std::nullopt_t) {}
 
     /// Construct an ArrayRef from a single element.
-    /*implicit*/ ArrayRef(const T &OneElt)
-      : Data(&OneElt), Length(1) {}
+    /*implicit*/ constexpr ArrayRef(const T &OneElt)
+        : Data(&OneElt), Length(1) {}
 
     /// Construct an ArrayRef from a pointer and length.
     constexpr /*implicit*/ ArrayRef(const T *data, size_t length)
@@ -123,9 +123,10 @@ namespace llvm {
     /// Construct an ArrayRef<const T*> from ArrayRef<T*>. This uses SFINAE to
     /// ensure that only ArrayRefs of pointers can be converted.
     template <typename U>
-    ArrayRef(const ArrayRef<U *> &A,
-             std::enable_if_t<std::is_convertible<U *const *, T const *>::value>
-                 * = nullptr)
+    constexpr ArrayRef(
+        const ArrayRef<U *> &A,
+        std::enable_if_t<std::is_convertible<U *const *, T const *>::value> * =
+            nullptr)
         : Data(A.data()), Length(A.size()) {}
 
     /// Construct an ArrayRef<const T*> from a SmallVector<T*>. This is
@@ -150,28 +151,32 @@ namespace llvm {
     /// @name Simple Operations
     /// @{
 
-    iterator begin() const { return Data; }
-    iterator end() const { return Data + Length; }
+    constexpr iterator begin() const { return Data; }
+    constexpr iterator end() const { return Data + Length; }
 
-    reverse_iterator rbegin() const { return reverse_iterator(end()); }
-    reverse_iterator rend() const { return reverse_iterator(begin()); }
+    constexpr reverse_iterator rbegin() const {
+      return reverse_iterator(end());
+    }
+    constexpr reverse_iterator rend() const {
+      return reverse_iterator(begin());
+    }
 
     /// empty - Check if the array is empty.
-    bool empty() const { return Length == 0; }
+    constexpr bool empty() const { return Length == 0; }
 
-    const T *data() const { return Data; }
+    constexpr const T *data() const { return Data; }
 
     /// size - Get the array size.
-    size_t size() const { return Length; }
+    constexpr size_t size() const { return Length; }
 
     /// front - Get the first element.
-    const T &front() const {
+    constexpr const T &front() const {
       assert(!empty());
       return Data[0];
     }
 
     /// back - Get the last element.
-    const T &back() const {
+    constexpr const T &back() const {
       assert(!empty());
       return Data[Length-1];
     }
@@ -184,7 +189,7 @@ namespace llvm {
     }
 
     /// equals - Check for element-wise equality.
-    bool equals(ArrayRef RHS) const {
+    constexpr bool equals(ArrayRef RHS) const {
       if (Length != RHS.Length)
         return false;
       return std::equal(begin(), end(), RHS.begin());
@@ -192,22 +197,22 @@ namespace llvm {
 
     /// slice(n, m) - Chop off the first N elements of the array, and keep M
     /// elements in the array.
-    ArrayRef<T> slice(size_t N, size_t M) const {
+    constexpr ArrayRef<T> slice(size_t N, size_t M) const {
       assert(N+M <= size() && "Invalid specifier");
       return ArrayRef<T>(data()+N, M);
     }
 
     /// slice(n) - Chop off the first N elements of the array.
-    ArrayRef<T> slice(size_t N) const { return slice(N, size() - N); }
+    constexpr ArrayRef<T> slice(size_t N) const { return slice(N, size() - N); }
 
     /// Drop the first \p N elements of the array.
-    ArrayRef<T> drop_front(size_t N = 1) const {
+    constexpr ArrayRef<T> drop_front(size_t N = 1) const {
       assert(size() >= N && "Dropping more elements than exist");
       return slice(N, size() - N);
     }
 
     /// Drop the last \p N elements of the array.
-    ArrayRef<T> drop_back(size_t N = 1) const {
+    constexpr ArrayRef<T> drop_back(size_t N = 1) const {
       assert(size() >= N && "Dropping more elements than exist");
       return slice(0, size() - N);
     }
@@ -225,14 +230,14 @@ namespace llvm {
     }
 
     /// Return a copy of *this with only the first \p N elements.
-    ArrayRef<T> take_front(size_t N = 1) const {
+    constexpr ArrayRef<T> take_front(size_t N = 1) const {
       if (N >= size())
         return *this;
       return drop_back(size() - N);
     }
 
     /// Return a copy of *this with only the last \p N elements.
-    ArrayRef<T> take_back(size_t N = 1) const {
+    constexpr ArrayRef<T> take_back(size_t N = 1) const {
       if (N >= size())
         return *this;
       return drop_front(size() - N);
@@ -253,7 +258,7 @@ namespace llvm {
     /// @}
     /// @name Operator Overloads
     /// @{
-    const T &operator[](size_t Index) const {
+    constexpr const T &operator[](size_t Index) const {
       assert(Index < Length && "Invalid index!");
       return Data[Index];
     }
@@ -319,20 +324,20 @@ namespace llvm {
     using difference_type = ptrdiff_t;
 
     /// Construct an empty MutableArrayRef.
-    /*implicit*/ MutableArrayRef() = default;
+    /*implicit*/ constexpr MutableArrayRef() = default;
 
     /// Construct an empty MutableArrayRef from std::nullopt.
-    /*implicit*/ MutableArrayRef(std::nullopt_t) : ArrayRef<T>() {}
+    /*implicit*/ constexpr MutableArrayRef(std::nullopt_t) : ArrayRef<T>() {}
 
     /// Construct a MutableArrayRef from a single element.
-    /*implicit*/ MutableArrayRef(T &OneElt) : ArrayRef<T>(OneElt) {}
+    /*implicit*/ constexpr MutableArrayRef(T &OneElt) : ArrayRef<T>(OneElt) {}
 
     /// Construct a MutableArrayRef from a pointer and length.
-    /*implicit*/ MutableArrayRef(T *data, size_t length)
-      : ArrayRef<T>(data, length) {}
+    /*implicit*/ constexpr MutableArrayRef(T *data, size_t length)
+        : ArrayRef<T>(data, length) {}
 
     /// Construct a MutableArrayRef from a range.
-    MutableArrayRef(T *begin, T *end) : ArrayRef<T>(begin, end) {}
+    constexpr MutableArrayRef(T *begin, T *end) : ArrayRef<T>(begin, end) {}
 
     /// Construct a MutableArrayRef from a SmallVector.
     /*implicit*/ MutableArrayRef(SmallVectorImpl<T> &Vec)
@@ -351,45 +356,49 @@ namespace llvm {
     template <size_t N>
     /*implicit*/ constexpr MutableArrayRef(T (&Arr)[N]) : ArrayRef<T>(Arr) {}
 
-    T *data() const { return const_cast<T*>(ArrayRef<T>::data()); }
+    constexpr T *data() const { return const_cast<T *>(ArrayRef<T>::data()); }
 
-    iterator begin() const { return data(); }
-    iterator end() const { return data() + this->size(); }
+    constexpr iterator begin() const { return data(); }
+    constexpr iterator end() const { return data() + this->size(); }
 
-    reverse_iterator rbegin() const { return reverse_iterator(end()); }
-    reverse_iterator rend() const { return reverse_iterator(begin()); }
+    constexpr reverse_iterator rbegin() const {
+      return reverse_iterator(end());
+    }
+    constexpr reverse_iterator rend() const {
+      return reverse_iterator(begin());
+    }
 
     /// front - Get the first element.
-    T &front() const {
+    constexpr T &front() const {
       assert(!this->empty());
       return data()[0];
     }
 
     /// back - Get the last element.
-    T &back() const {
+    constexpr T &back() const {
       assert(!this->empty());
       return data()[this->size()-1];
     }
 
     /// slice(n, m) - Chop off the first N elements of the array, and keep M
     /// elements in the array.
-    MutableArrayRef<T> slice(size_t N, size_t M) const {
+    constexpr MutableArrayRef<T> slice(size_t N, size_t M) const {
       assert(N + M <= this->size() && "Invalid specifier");
       return MutableArrayRef<T>(this->data() + N, M);
     }
 
     /// slice(n) - Chop off the first N elements of the array.
-    MutableArrayRef<T> slice(size_t N) const {
+    constexpr MutableArrayRef<T> slice(size_t N) const {
       return slice(N, this->size() - N);
     }
 
     /// Drop the first \p N elements of the array.
-    MutableArrayRef<T> drop_front(size_t N = 1) const {
+    constexpr MutableArrayRef<T> drop_front(size_t N = 1) const {
       assert(this->size() >= N && "Dropping more elements than exist");
       return slice(N, this->size() - N);
     }
 
-    MutableArrayRef<T> drop_back(size_t N = 1) const {
+    constexpr MutableArrayRef<T> drop_back(size_t N = 1) const {
       assert(this->size() >= N && "Dropping more elements than exist");
       return slice(0, this->size() - N);
     }
@@ -409,14 +418,14 @@ namespace llvm {
     }
 
     /// Return a copy of *this with only the first \p N elements.
-    MutableArrayRef<T> take_front(size_t N = 1) const {
+    constexpr MutableArrayRef<T> take_front(size_t N = 1) const {
       if (N >= this->size())
         return *this;
       return drop_back(this->size() - N);
     }
 
     /// Return a copy of *this with only the last \p N elements.
-    MutableArrayRef<T> take_back(size_t N = 1) const {
+    constexpr MutableArrayRef<T> take_back(size_t N = 1) const {
       if (N >= this->size())
         return *this;
       return drop_front(this->size() - N);
@@ -439,7 +448,7 @@ namespace llvm {
     /// @}
     /// @name Operator Overloads
     /// @{
-    T &operator[](size_t Index) const {
+    constexpr T &operator[](size_t Index) const {
       assert(Index < this->size() && "Invalid index!");
       return data()[Index];
     }

>From 17844494b4ea0bc465fc31b745a537b9367d6608 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Wed, 10 Jul 2024 18:23:08 +0100
Subject: [PATCH 2/3] ADT/Matrix: two-dimensional Container with View

Leverage the excellent SmallVector infrastructure to write a
two-dimensional container, along with a View that abstracts out
indexing-arithmetic, eliminating memory operations on the underlying
storage. The immediate applicability of Matrix is to replace uses of the
vector-of-vectors idiom, with one caveat: an upper bound on the number
of columns should be known ahead of time.
---
 llvm/include/llvm/ADT/ArrayRef.h  |   3 +
 llvm/include/llvm/ADT/Matrix.h    | 339 ++++++++++++++++++++++++++++++
 llvm/unittests/ADT/CMakeLists.txt |   1 +
 llvm/unittests/ADT/MatrixTest.cpp | 325 ++++++++++++++++++++++++++++
 4 files changed, 668 insertions(+)
 create mode 100644 llvm/include/llvm/ADT/Matrix.h
 create mode 100644 llvm/unittests/ADT/MatrixTest.cpp

diff --git a/llvm/include/llvm/ADT/ArrayRef.h b/llvm/include/llvm/ADT/ArrayRef.h
index 8c487e5b41ebf..6fc7acff7ba8c 100644
--- a/llvm/include/llvm/ADT/ArrayRef.h
+++ b/llvm/include/llvm/ADT/ArrayRef.h
@@ -25,6 +25,7 @@
 
 namespace llvm {
   template<typename T> class [[nodiscard]] MutableArrayRef;
+  template <typename T> struct [[nodiscard]] MatrixRowView;
 
   /// ArrayRef - Represent a constant reference to an array (0 or more elements
   /// consecutively in memory), i.e. a start pointer and a length.  It allows
@@ -59,6 +60,8 @@ namespace llvm {
     /// The number of elements.
     size_type Length = 0;
 
+    friend MatrixRowView<T>;
+
   public:
     /// @name Constructors
     /// @{
diff --git a/llvm/include/llvm/ADT/Matrix.h b/llvm/include/llvm/ADT/Matrix.h
new file mode 100644
index 0000000000000..eeb9702772738
--- /dev/null
+++ b/llvm/include/llvm/ADT/Matrix.h
@@ -0,0 +1,339 @@
+//===- Matrix.h - Two-dimensional Container with View -----------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_ADT_MATRIX_H
+#define LLVM_ADT_MATRIX_H
+
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallVector.h"
+
+namespace llvm {
+template <typename T, size_t M, size_t NStorageInline> class MatrixView;
+
+/// Due to the SmallVector infrastructure using SmallVectorAlignmentAndOffset
+/// that depends on the exact data layout, no derived classes can have extra
+/// members.
+template <typename T, size_t N>
+struct MatrixStorageBase : public SmallVectorImpl<T>, SmallVectorStorage<T, N> {
+  MatrixStorageBase() : SmallVectorImpl<T>(N) {}
+  MatrixStorageBase(size_t Size) : SmallVectorImpl<T>(N) { resize(Size); }
+  ~MatrixStorageBase() { destroy_range(this->begin(), this->end()); }
+  MatrixStorageBase(const MatrixStorageBase &RHS) : SmallVectorImpl<T>(N) {
+    if (!RHS.empty())
+      SmallVectorImpl<T>::operator=(RHS);
+  }
+  MatrixStorageBase(MatrixStorageBase &&RHS) : SmallVectorImpl<T>(N) {
+    if (!RHS.empty())
+      SmallVectorImpl<T>::operator=(::std::move(RHS));
+  }
+  using SmallVectorImpl<T>::size;
+  using SmallVectorImpl<T>::resize;
+  using SmallVectorImpl<T>::append;
+  using SmallVectorImpl<T>::erase;
+  using SmallVectorImpl<T>::destroy_range;
+
+  T *begin() const { return const_cast<T *>(SmallVectorImpl<T>::begin()); }
+  T *end() const { return const_cast<T *>(SmallVectorImpl<T>::end()); }
+};
+
+/// A two-dimensional container storage, whose upper bound on the number of
+/// columns should be known ahead of time. Not menat to be used directly: the
+/// primary usage API is MatrixView.
+template <typename T,
+          size_t N = CalculateSmallVectorDefaultInlinedElements<T>::value>
+class MatrixStorage {
+public:
+  MatrixStorage() = delete;
+  MatrixStorage(size_t NRows, size_t NCols)
+      : Base(NRows * NCols), NCols(NCols) {}
+  MatrixStorage(size_t NCols) : Base(), NCols(NCols) {}
+
+  size_t size() const { return Base.size(); }
+  bool empty() const { return !size(); }
+  size_t getNumRows() const { return size() / NCols; }
+  size_t getNumCols() const { return NCols; }
+  void setNumCols(size_t NCols) {
+    assert(empty() && "Column-resizing a non-empty MatrixStorage");
+    this->NCols = NCols;
+  }
+  void resize(size_t NRows) { Base.resize(NCols * NRows); }
+
+protected:
+  template <typename U, size_t M, size_t NStorageInline>
+  friend class MatrixView;
+
+  T *begin() const { return Base.begin(); }
+  T *rowFromIdx(size_t RowIdx, size_t Offset = 0) const {
+    return begin() + RowIdx * NCols + Offset;
+  }
+  std::pair<size_t, size_t> idxFromRow(T *Ptr) const {
+    assert(Ptr >= begin() && "Internal error");
+    size_t Offset = (Ptr - begin()) % NCols;
+    return {(Ptr - begin()) / NCols, Offset};
+  }
+
+  // If Arg.size() < NCols, the number of columns won't be changed, and the
+  // difference is default-constructed.
+  void addRow(const SmallVectorImpl<T> &Arg) {
+    assert(Arg.size() <= NCols &&
+           "MatrixStorage has insufficient number of columns");
+    size_t Diff = NCols - Arg.size();
+    Base.append(Arg.begin(), Arg.end());
+    Base.append(Diff, T());
+  }
+
+private:
+  MatrixStorageBase<T, N> Base;
+  size_t NCols;
+};
+
+/// MutableArrayRef with a copy-assign, and extra APIs.
+template <typename T>
+struct [[nodiscard]] MatrixRowView : public MutableArrayRef<T> {
+  using pointer = typename MutableArrayRef<T>::pointer;
+  using iterator = typename MutableArrayRef<T>::iterator;
+  using const_iterator = typename MutableArrayRef<T>::const_iterator;
+
+  constexpr MatrixRowView() = delete;
+  constexpr MatrixRowView(pointer Data, size_t Length)
+      : MutableArrayRef<T>(Data, Length) {}
+  constexpr MatrixRowView(iterator Begin, iterator End)
+      : MutableArrayRef<T>(Begin, End) {}
+  constexpr MatrixRowView(const_iterator Begin, const_iterator End)
+      : MutableArrayRef<T>(Begin, End) {}
+  constexpr MatrixRowView(MutableArrayRef<T> Other)
+      : MutableArrayRef<T>(Other.data(), Other.size()) {}
+  MatrixRowView(const SmallVectorImpl<T> &Vec) : MutableArrayRef<T>(Vec) {}
+
+  using MutableArrayRef<T>::size;
+  using MutableArrayRef<T>::data;
+
+  constexpr T &back() const { return MutableArrayRef<T>::back(); }
+  constexpr T &front() const { return MutableArrayRef<T>::front(); }
+  constexpr MatrixRowView<T> drop_back(size_t N = 1) const { // NOLINT
+    return MutableArrayRef<T>::drop_back(N);
+  }
+  constexpr MatrixRowView<T> drop_front(size_t N = 1) const { // NOLINT
+    return MutableArrayRef<T>::drop_front(N);
+  }
+  // This slice is different from the MutableArrayRef slice, and specifies a
+  // Begin and End index, instead of a Begin and Length.
+  constexpr MatrixRowView<T> slice(size_t Begin, size_t End) {
+    return MutableArrayRef<T>::slice(Begin, End - Begin);
+  }
+  constexpr void pop_back(size_t N = 1) { // NOLINT
+    this->Length -= N;
+  }
+  constexpr void pop_front(size_t N = 1) { // NOLINT
+    this->Data += N;
+    this->Length -= N;
+  }
+
+  MatrixRowView &operator=(const SmallVectorImpl<T> &Vec) {
+    copy_assign(Vec.begin(), Vec.end());
+    return *this;
+  }
+  MatrixRowView &operator=(std::initializer_list<T> IL) {
+    copy_assign(IL.begin(), IL.end());
+    return *this;
+  }
+
+  void swap(MatrixRowView<T> &Other) {
+    std::swap(this->Data, Other.Data);
+    std::swap(this->Length, Other.Length);
+  }
+
+protected:
+  void copy_assign(iterator Begin, iterator End) { // NOLINT
+    std::uninitialized_copy(Begin, End, data());
+    this->Length = End - Begin;
+  }
+  void copy_assign(const_iterator Begin, const_iterator End) { // NOLINT
+    std::uninitialized_copy(Begin, End, data());
+    this->Length = End - Begin;
+  }
+};
+
+/// The primary usage API of MatrixStorage. Abstracts out indexing-arithmetic,
+/// eliminating memory operations on the underlying data. Supports
+/// variable-length columns.
+template <typename T,
+          size_t N = CalculateSmallVectorDefaultInlinedElements<T>::value,
+          size_t NStorageInline =
+              CalculateSmallVectorDefaultInlinedElements<T>::value>
+class [[nodiscard]] MatrixView {
+public:
+  using row_type = MatrixRowView<T>;
+  using container_type = SmallVector<row_type, N>;
+  using iterator = typename container_type::iterator;
+  using const_iterator = typename container_type::const_iterator;
+
+  constexpr MatrixView(MatrixStorage<T, NStorageInline> &Mat, size_t RowSpan,
+                       size_t ColSpan)
+      : Mat(Mat) {
+    RowView.reserve(RowSpan);
+    for (size_t RowIdx = 0; RowIdx < RowSpan; ++RowIdx) {
+      auto RangeBegin = Mat.begin() + RowIdx * ColSpan;
+      RowView.emplace_back(RangeBegin, RangeBegin + ColSpan);
+    }
+  }
+
+  // Constructor with a full View of the underlying MatrixStorage, if
+  // MatrixStorage has a non-zero number of Columns. Otherwise, creates an empty
+  // view.
+  constexpr MatrixView(MatrixStorage<T, NStorageInline> &Mat)
+      : MatrixView(Mat, Mat.getNumRows(), Mat.getNumCols()) {}
+
+  // Obvious copy-construator is deleted, since the underlying storage could
+  // have changed.
+  constexpr MatrixView(const MatrixView &) = delete;
+
+  // Copy-assignment operator should not be used when the underlying storage
+  // changes.
+  constexpr MatrixView &operator=(const MatrixView &Other) {
+    assert(Mat.begin() == Other.Mat.begin() &&
+           "Underlying storage has changed: use custom copy-constructor");
+    RowView = Other.RowView;
+    return *this;
+  }
+
+  // The actual copy-constructor: to be used when the underlying storage is
+  // copy-constructed.
+  MatrixView(const MatrixView &OldView,
+             MatrixStorage<T, NStorageInline> &NewMat)
+      : Mat(NewMat) {
+    assert(OldView.Mat.size() == Mat.size() &&
+           "Custom copy-constructor called on non-copied storage");
+
+    // The underlying storage will change. Construct a new RowView by performing
+    // pointer-arithmetic on the underlying storage of OldView, using pointers
+    // from OldVie.
+    for (const auto &R : OldView.RowView) {
+      auto [StorageIdx, StartOffset] = OldView.Mat.idxFromRow(R.data());
+      RowView.emplace_back(Mat.rowFromIdx(StorageIdx, StartOffset), R.size());
+    }
+  }
+
+  void addRow(const SmallVectorImpl<T> &Row) {
+    // The underlying storage may be resized, performing reallocations. The
+    // pointers in RowView will no longer be valid, so save and restore the
+    // data. Construct RestoreData by performing pointer-arithmetic on the
+    // underlying storgge.
+    SmallVector<std::tuple<size_t, size_t, size_t>> RestoreData;
+    RestoreData.reserve(RowView.size());
+    for (const auto &R : RowView) {
+      auto [StorageIdx, StartOffset] = Mat.idxFromRow(R.data());
+      RestoreData.emplace_back(StorageIdx, StartOffset, R.size());
+    }
+
+    Mat.addRow(Row);
+
+    // Restore the RowView by performing pointer-arithmetic on the
+    // possibly-reallocated storage, using information from RestoreData.
+    RowView.clear();
+    for (const auto &[StorageIdx, StartOffset, Len] : RestoreData)
+      RowView.emplace_back(Mat.rowFromIdx(StorageIdx, StartOffset), Len);
+
+    // Finally, add the new row to the VRowView.
+    RowView.emplace_back(Mat.rowFromIdx(Mat.getNumRows() - 1), Row.size());
+  }
+
+  // To support addRow(View[Idx]).
+  void addRow(const row_type &Row) { addRow(SmallVector<T>{Row}); }
+
+  void addRow(std::initializer_list<T> Row) { addRow(SmallVector<T>{Row}); }
+
+  constexpr row_type &operator[](size_t RowIdx) {
+    assert(RowIdx < RowView.size() && "Indexing out of bounds");
+    return RowView[RowIdx];
+  }
+
+  constexpr T *data() const {
+    assert(!empty() && "Non-empty view expected");
+    return RowView.front().data();
+  }
+  size_t size() const { return getRowSpan() * getMaxColSpan(); }
+  bool empty() const { return RowView.empty(); }
+  size_t getRowSpan() const { return RowView.size(); }
+  size_t getColSpan(size_t RowIdx) const {
+    assert(RowIdx < RowView.size() && "Indexing out of bounds");
+    return RowView[RowIdx].size();
+  }
+  constexpr size_t getMaxColSpan() const {
+    return std::max_element(RowView.begin(), RowView.end(),
+                            [](const row_type &RowA, const row_type &RowB) {
+                              return RowA.size() < RowB.size();
+                            })
+        ->size();
+  }
+
+  iterator begin() { return RowView.begin(); }
+  iterator end() { return RowView.end(); }
+  const_iterator begin() const { return RowView.begin(); }
+  const_iterator end() const { return RowView.end(); }
+
+  constexpr MatrixView<T, N, NStorageInline> rowSlice(size_t Begin,
+                                                      size_t End) {
+    assert(Begin < getRowSpan() && End <= getRowSpan() &&
+           "Indexing out of bounds");
+    assert(Begin < End && "Invalid slice");
+    container_type NewRowView;
+    for (size_t RowIdx = Begin; RowIdx < End; ++RowIdx)
+      NewRowView.emplace_back(RowView[RowIdx]);
+    return {Mat, std::move(NewRowView)};
+  }
+
+  constexpr MatrixView<T, N, NStorageInline> colSlice(size_t Begin,
+                                                      size_t End) {
+    assert(Begin < End && "Invalid slice");
+    size_t MinColSpan =
+        std::min_element(RowView.begin(), RowView.end(),
+                         [](const row_type &RowA, const row_type &RowB) {
+                           return RowA.size() < RowB.size();
+                         })
+            ->size();
+    assert(Begin < MinColSpan && End <= MinColSpan && "Indexing out of bounds");
+    container_type NewRowView;
+    for (row_type Row : RowView)
+      NewRowView.emplace_back(Row.slice(Begin, End));
+    return {Mat, std::move(NewRowView)};
+  }
+
+  row_type &lastRow() {
+    assert(!empty() && "Non-empty view expected");
+    return RowView.back();
+  }
+  const row_type &lastRow() const {
+    assert(!empty() && "Non-empty view expected");
+    return RowView.back();
+  }
+  void dropLastRow() {
+    assert(!empty() && "Non-empty view expected");
+    RowView.pop_back();
+  }
+
+protected:
+  // Helper constructor.
+  constexpr MatrixView(MatrixStorage<T, NStorageInline> &Mat,
+                       SmallVectorImpl<row_type> &&RowView)
+      : Mat(Mat), RowView(std::move(RowView)) {}
+
+private:
+  MatrixStorage<T, NStorageInline> &Mat;
+  container_type RowView;
+};
+} // namespace llvm
+
+namespace std {
+template <typename T>
+inline void swap(llvm::MatrixRowView<T> &LHS, llvm::MatrixRowView<T> &RHS) {
+  LHS.swap(RHS);
+}
+} // end namespace std
+
+#endif
diff --git a/llvm/unittests/ADT/CMakeLists.txt b/llvm/unittests/ADT/CMakeLists.txt
index f48d840a10595..cd04778e86e2d 100644
--- a/llvm/unittests/ADT/CMakeLists.txt
+++ b/llvm/unittests/ADT/CMakeLists.txt
@@ -54,6 +54,7 @@ add_llvm_unittest(ADTTests
   LazyAtomicPointerTest.cpp
   MappedIteratorTest.cpp
   MapVectorTest.cpp
+  MatrixTest.cpp
   PackedVectorTest.cpp
   PagedVectorTest.cpp
   PointerEmbeddedIntTest.cpp
diff --git a/llvm/unittests/ADT/MatrixTest.cpp b/llvm/unittests/ADT/MatrixTest.cpp
new file mode 100644
index 0000000000000..7fba1769788a1
--- /dev/null
+++ b/llvm/unittests/ADT/MatrixTest.cpp
@@ -0,0 +1,325 @@
+#include "llvm/ADT/Matrix.h"
+#include "llvm/ADT/DynamicAPInt.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+template <typename T> static SmallVector<T> getDummyValues(size_t NValues) {
+  SmallVector<T> Ret(NValues);
+  for (size_t Idx = 0; Idx < NValues; ++Idx)
+    Ret[Idx] = T(Idx + 1);
+  return Ret;
+}
+
+template <typename T> class MatrixTest : public testing::Test {
+protected:
+  MatrixTest()
+      : ColumnInitMatrix(8), SmallMatrix(2, 2), OtherSmall(3, 3),
+        LargeMatrix(16, 16) {}
+  MatrixStorage<T, 16> ColumnInitMatrix;
+  MatrixStorage<T> SmallMatrix;
+  MatrixStorage<T> OtherSmall;
+  MatrixStorage<T> LargeMatrix;
+  SmallVector<T> getDummyRow(size_t NValues) {
+    return getDummyValues<T>(NValues);
+  }
+};
+
+using MatrixTestTypes = ::testing::Types<int64_t, DynamicAPInt>;
+TYPED_TEST_SUITE(MatrixTest, MatrixTestTypes, );
+
+TYPED_TEST(MatrixTest, Construction) {
+  auto &E = this->ColumnInitMatrix;
+  ASSERT_TRUE(E.empty());
+  ASSERT_EQ(E.getNumCols(), 8u);
+  E.setNumCols(3);
+  ASSERT_EQ(E.getNumCols(), 3u);
+  ASSERT_TRUE(E.empty());
+  ASSERT_EQ(E.getNumRows(), 0u);
+  auto &M = this->SmallMatrix;
+  ASSERT_FALSE(M.empty());
+  ASSERT_EQ(M.size(), 4u);
+  ASSERT_EQ(M.getNumRows(), 2u);
+  ASSERT_EQ(M.getNumCols(), 2u);
+}
+
+TYPED_TEST(MatrixTest, CopyConstruction) {
+  auto &OldMat = this->SmallMatrix;
+  auto V = MatrixView<TypeParam>{OldMat};
+  V[0] = this->getDummyRow(2);
+  V[0].pop_back();
+  V[1] = this->getDummyRow(2);
+  V[1].pop_front();
+  ASSERT_EQ(V.getRowSpan(), 2u);
+  ASSERT_EQ(V.getColSpan(0), 1u);
+  ASSERT_EQ(V.getColSpan(1), 1u);
+  ASSERT_EQ(V[0][0], 1);
+  ASSERT_EQ(V[1][0], 2);
+  ASSERT_EQ(MatrixView<TypeParam>{OldMat}[0][0], 1);
+  ASSERT_EQ(MatrixView<TypeParam>{OldMat}[0][1], 2);
+  ASSERT_EQ(MatrixView<TypeParam>{OldMat}[1][0], 1);
+  ASSERT_EQ(MatrixView<TypeParam>{OldMat}[1][1], 2);
+  MatrixStorage<TypeParam> NewMat{OldMat};
+  MatrixView<TypeParam> C{V, NewMat};
+  ASSERT_EQ(C.getRowSpan(), 2u);
+  ASSERT_EQ(C.getColSpan(0), 1u);
+  ASSERT_EQ(C.getColSpan(1), 1u);
+  ASSERT_EQ(C[0][0], 1);
+  ASSERT_EQ(C[1][0], 2);
+  C.addRow(this->getDummyRow(2));
+  ASSERT_EQ(C[2][0], 1);
+  ASSERT_EQ(C[2][1], 2);
+}
+
+TYPED_TEST(MatrixTest, RowOps) {
+  auto &M = this->SmallMatrix;
+  auto &O = this->OtherSmall;
+  MatrixView<TypeParam> V{M};
+  ASSERT_EQ(M.getNumRows(), 2u);
+  ASSERT_EQ(V.getRowSpan(), 2u);
+  V[0] = this->getDummyRow(2);
+  V = MatrixView<TypeParam>{M};
+  ASSERT_EQ(V[0][0], 1);
+  ASSERT_EQ(V[0][1], 2);
+  ASSERT_EQ(M.getNumRows(), 2u);
+  V.addRow({TypeParam(4), TypeParam(5)});
+  ASSERT_EQ(M.getNumRows(), 3u);
+  ASSERT_EQ(V[2][0], 4);
+  ASSERT_EQ(V[2][1], 5);
+  ASSERT_EQ(O.getNumRows(), 3u);
+  MatrixView<TypeParam> W{O};
+  W.addRow(V[0]);
+  ASSERT_EQ(O.getNumCols(), 3u);
+  ASSERT_EQ(O.getNumRows(), 4u);
+  W[3] = {TypeParam(7), TypeParam(8)};
+  auto &WRow3 = W[3];
+  WRow3 = WRow3.drop_back();
+  ASSERT_EQ(WRow3[0], 7);
+  WRow3 = WRow3.drop_back();
+  ASSERT_TRUE(WRow3.empty());
+  ASSERT_EQ(W.getColSpan(3), 0u);
+  ASSERT_EQ(W[0][2], 0);
+  ASSERT_EQ(W[1][2], 0);
+  ASSERT_EQ(W[2][2], 0);
+  W = MatrixView<TypeParam>{O};
+  ASSERT_EQ(W[0][2], 0);
+  ASSERT_EQ(W[1][2], 0);
+  ASSERT_EQ(W[2][2], 0);
+  ASSERT_EQ(W[3][2], 0);
+}
+
+TYPED_TEST(MatrixTest, ResizeAssign) {
+  auto &M = this->SmallMatrix;
+  M.resize(3);
+  ASSERT_EQ(M.getNumRows(), 3u);
+  MatrixView<TypeParam> V{M};
+  V[0] = this->getDummyRow(2);
+  V[1] = this->getDummyRow(2);
+  V[2] = this->getDummyRow(2);
+  ASSERT_EQ(M.getNumRows(), 3u);
+  ASSERT_EQ(V[0][0], V[1][0]);
+  ASSERT_EQ(V[2][1], V[1][1]);
+}
+
+TYPED_TEST(MatrixTest, ColSlice) {
+  auto &M = this->OtherSmall;
+  MatrixView<TypeParam> V{M};
+  V[0] = {TypeParam(3), TypeParam(7)};
+  V[1] = {TypeParam(4), TypeParam(5)};
+  V[2] = {TypeParam(8), TypeParam(9)};
+  auto W = V.colSlice(1, 2);
+  ASSERT_EQ(W.getRowSpan(), 3u);
+  ASSERT_EQ(W.getColSpan(0), 1u);
+  ASSERT_EQ(V[0][0], 3);
+  ASSERT_EQ(V[0][1], 7);
+  ASSERT_EQ(V[1][0], 4);
+  ASSERT_EQ(V[1][1], 5);
+  ASSERT_EQ(V[2][0], 8);
+  ASSERT_EQ(V[2][1], 9);
+  ASSERT_EQ(W[0][0], 7);
+  ASSERT_EQ(W[1][0], 5);
+  ASSERT_EQ(W[2][0], 9);
+}
+
+TYPED_TEST(MatrixTest, RowColSlice) {
+  auto &M = this->OtherSmall;
+  MatrixView<TypeParam> V{M};
+  V[0] = {TypeParam(3), TypeParam(7)};
+  V[2] = {TypeParam(4), TypeParam(5)};
+  auto W = V.rowSlice(0, 2).colSlice(0, 2);
+  ASSERT_EQ(W.getRowSpan(), 2u);
+  ASSERT_EQ(W.getColSpan(0), 2u);
+  ASSERT_EQ(V[0][0], 3);
+  ASSERT_EQ(V[0][1], 7);
+  ASSERT_EQ(V[1][0], 0);
+  ASSERT_EQ(V[1][1], 0);
+  ASSERT_EQ(V[2][0], 4);
+  ASSERT_EQ(V[2][1], 5);
+  ASSERT_EQ(W[0][0], 3);
+  ASSERT_EQ(W[0][1], 7);
+  ASSERT_EQ(W[1][0], 0);
+  ASSERT_EQ(W[1][1], 0);
+}
+
+TYPED_TEST(MatrixTest, LastRowOps) {
+  auto &M = this->SmallMatrix;
+  MatrixView<TypeParam> V{M};
+  V[0] = {TypeParam(3), TypeParam(7)};
+  V[1] = {TypeParam(4), TypeParam(5)};
+  V.dropLastRow();
+  ASSERT_EQ(M.size(), 4u);
+  ASSERT_EQ(V.size(), 2u);
+  auto W = V.lastRow();
+  ASSERT_EQ(W.size(), 2u);
+  ASSERT_EQ(W[0], 3);
+  ASSERT_EQ(W[1], 7);
+  V.lastRow() = {TypeParam(1), TypeParam(2)};
+  ASSERT_EQ(V.lastRow()[0], 1);
+  ASSERT_EQ(V.lastRow()[1], 2);
+  V.dropLastRow();
+  ASSERT_TRUE(V.empty());
+}
+
+TYPED_TEST(MatrixTest, Swap) {
+  auto &M = this->SmallMatrix;
+  MatrixView<TypeParam> V{M};
+  V[0] = {TypeParam(3), TypeParam(7)};
+  V[1] = {TypeParam(4), TypeParam(5)};
+  std::swap(V[0], V[1]);
+  ASSERT_EQ(V.lastRow()[0], 3);
+  ASSERT_EQ(V.lastRow()[1], 7);
+  ASSERT_EQ(V[0][0], 4);
+  ASSERT_EQ(V[0][1], 5);
+  ASSERT_EQ(V[1][0], 3);
+  ASSERT_EQ(V[1][1], 7);
+  auto W = MatrixView<TypeParam>{M};
+  ASSERT_EQ(W[0][0], 3);
+  ASSERT_EQ(W[0][1], 7);
+  ASSERT_EQ(W[1][0], 4);
+  ASSERT_EQ(W[1][1], 5);
+}
+
+TYPED_TEST(MatrixTest, DropLastRow) {
+  auto &M = this->OtherSmall;
+  auto V = MatrixView<TypeParam>{M};
+  ASSERT_EQ(V.getRowSpan(), 3u);
+  V.dropLastRow();
+  ASSERT_EQ(V.getRowSpan(), 2u);
+  V[0] = {TypeParam(19), TypeParam(7), TypeParam(3)};
+  V[1] = {TypeParam(19), TypeParam(7), TypeParam(3)};
+  ASSERT_EQ(V.getColSpan(1), 3u);
+  V.addRow(V.lastRow());
+  ASSERT_EQ(V.getRowSpan(), 3u);
+  ASSERT_EQ(V.getColSpan(2), 3u);
+  ASSERT_EQ(V[1], V.lastRow());
+  V.dropLastRow();
+  ASSERT_EQ(V.getRowSpan(), 2u);
+  ASSERT_EQ(V[0], V.lastRow());
+  V.addRow(this->getDummyRow(3));
+  V.dropLastRow();
+  V.addRow(this->getDummyRow(3));
+  V.dropLastRow();
+  ASSERT_EQ(V.getRowSpan(), 2u);
+  ASSERT_EQ(V[0], V.lastRow());
+  V.dropLastRow();
+  V.dropLastRow();
+  ASSERT_TRUE(V.empty());
+  V.addRow({TypeParam(9), TypeParam(7), TypeParam(3)});
+  V.addRow(this->getDummyRow(3));
+  ASSERT_EQ(V.getRowSpan(), 2u);
+  ASSERT_EQ(V.lastRow()[0], 1);
+  ASSERT_EQ(V.lastRow()[1], 2);
+  ASSERT_EQ(V.lastRow()[2], 3);
+  ASSERT_EQ(V[0][0], 9);
+  ASSERT_EQ(V[0][1], 7);
+  ASSERT_EQ(V[0][2], 3);
+  V.dropLastRow();
+  V.addRow({TypeParam(21), TypeParam(22), TypeParam(23)});
+  ASSERT_EQ(V.getRowSpan(), 2u);
+  ASSERT_EQ(V.lastRow()[0], 21);
+  ASSERT_EQ(V.lastRow()[1], 22);
+  ASSERT_EQ(V.lastRow()[2], 23);
+}
+
+TYPED_TEST(MatrixTest, Iteration) {
+  auto &M = this->SmallMatrix;
+  M.resize(2);
+  MatrixView<TypeParam> V{M};
+  for (const auto &[RowIdx, Row] : enumerate(V)) {
+    V[RowIdx] = this->getDummyRow(2);
+    for (const auto &[ColIdx, Col] : enumerate(Row)) {
+      ASSERT_GT(V[RowIdx][ColIdx], 0);
+    }
+  }
+}
+
+TYPED_TEST(MatrixTest, VariableLengthColumns) {
+  auto &M = this->ColumnInitMatrix;
+  MatrixView<TypeParam, 8, 16> V{M};
+  ASSERT_EQ(V.empty(), true);
+  size_t NumCols = 6;
+  size_t NumRows = 3;
+  SmallVector<TypeParam> ColumnVec;
+  for (size_t Var = 0; Var < NumCols; ++Var)
+    ColumnVec.emplace_back(Var + 1);
+  ASSERT_EQ(ColumnVec.size(), NumCols);
+  for (size_t RowIdx = 0; RowIdx < NumRows; ++RowIdx)
+    V.addRow(ColumnVec);
+  ASSERT_EQ(V.getMaxColSpan(), NumCols);
+  V.addRow({TypeParam(19)});
+  ASSERT_EQ(V.getColSpan(NumRows), 1u);
+  V.dropLastRow();
+  V.addRow(V.lastRow());
+  ASSERT_EQ(V.getColSpan(NumRows), NumCols);
+  ASSERT_EQ(V[NumRows - 1], V[NumRows]);
+  V.dropLastRow();
+  ASSERT_EQ(V.getRowSpan(), NumRows);
+  ASSERT_EQ(M.getNumCols(), 8u);
+  ASSERT_EQ(M.getNumRows(), NumRows + 2);
+  V.dropLastRow();
+  ASSERT_EQ(V.getRowSpan(), NumRows - 1);
+  ASSERT_EQ(M.getNumRows(), NumRows + 2);
+  ASSERT_EQ(V.getColSpan(1), NumCols);
+  V[1][0] = 19;
+  std::swap(V[0], V[1]);
+  V[0].pop_back();
+  V[1] = V[1].drop_back().drop_back();
+  ASSERT_EQ(V.getColSpan(0), NumCols - 1);
+  ASSERT_EQ(V.getColSpan(1), NumCols - 2);
+  ASSERT_EQ(V[0][0], 19);
+  ASSERT_EQ(V[1][0], 1);
+  ASSERT_EQ(V.getMaxColSpan(), NumCols - 1);
+  V = V.colSlice(0, 1).rowSlice(0, 2);
+  ASSERT_EQ(V.getColSpan(0), 1u);
+  ASSERT_EQ(V.getColSpan(1), 1u);
+  ASSERT_EQ(V.getMaxColSpan(), 1u);
+  ASSERT_EQ(V.getRowSpan(), 2u);
+  ASSERT_EQ(V[0][0], 19);
+  ASSERT_EQ(V[1][0], 1);
+  V.dropLastRow();
+  V.dropLastRow();
+  ASSERT_TRUE(V.empty());
+  ASSERT_EQ(M.size(), 40u);
+}
+
+TYPED_TEST(MatrixTest, LargeMatrixOps) {
+  auto &M = this->LargeMatrix;
+  ASSERT_EQ(M.getNumRows(), 16u);
+  ASSERT_EQ(M.getNumCols(), 16u);
+  MatrixView<TypeParam, 16> V{M};
+  V[0] = {TypeParam(1),  TypeParam(2), TypeParam(1),  TypeParam(4),
+          TypeParam(5),  TypeParam(1), TypeParam(1),  TypeParam(8),
+          TypeParam(9),  TypeParam(1), TypeParam(1),  TypeParam(12),
+          TypeParam(13), TypeParam(1), TypeParam(15), TypeParam(1)};
+  V[1] = this->getDummyRow(16);
+  ASSERT_EQ(V[0][14], 15);
+  ASSERT_EQ(V[0][3], 4);
+  ASSERT_EQ(std::count(V[0].begin(), V[0].end(), 1), 8);
+  ASSERT_TRUE(
+      std::all_of(V[0].begin(), V[0].end(), [](auto &El) { return El > 0; }));
+  auto W = V.rowSlice(0, 1).colSlice(2, 4);
+  ASSERT_EQ(W.getRowSpan(), 1u);
+  ASSERT_EQ(W.getColSpan(0), 2u);
+  ASSERT_EQ(W[0][0], 1);
+  ASSERT_EQ(W[0][1], 4);
+}

>From b1c870b25bb7fa20a17362f3e14557830bb31ff2 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Mon, 15 Jul 2024 12:34:01 +0100
Subject: [PATCH 3/3] 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 7b02b618f7cb4..7d4b3eaad9e82 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 <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.
+  MatrixView<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;
diff --git a/llvm/lib/Analysis/ConstraintSystem.cpp b/llvm/lib/Analysis/ConstraintSystem.cpp
index e4c9dcc7544e9..4f25621f5c742 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 <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());
+  MatrixView<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)
diff --git a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
index c31173879af1e..ff13302abc553 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 <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();
 }



More information about the llvm-commits mailing list