[Mlir-commits] [mlir] 862b5a5 - [MLIR][Presburger] Attach values only to non-local identifiers in FAVC

llvmlistbot at llvm.org llvmlistbot at llvm.org
Tue May 17 20:48:06 PDT 2022


Author: Groverkss
Date: 2022-05-18T09:17:23+05:30
New Revision: 862b5a52335fef9e29013b00506e49342ac473f1

URL: https://github.com/llvm/llvm-project/commit/862b5a52335fef9e29013b00506e49342ac473f1
DIFF: https://github.com/llvm/llvm-project/commit/862b5a52335fef9e29013b00506e49342ac473f1.diff

LOG: [MLIR][Presburger] Attach values only to non-local identifiers in FAVC

This patch changes `FlatAffineValueConstraints` to only allow attaching
values to non-local identifiers.

The reasoning for this change is:
1. Information attached to local identifiers can be lost since local identifiers
  can be removed for output size optimizations.
2. There are no current use cases for attaching values to Local identifiers.
3. Attaching a value to a local identifier does not make sense since a local
  identifier represents existential quantification.

This patch also adds some additional asserts to the affected functions.

Reviewed By: arjunp, bondhugula

Differential Revision: https://reviews.llvm.org/D125613

Added: 
    

Modified: 
    mlir/include/mlir/Dialect/Affine/Analysis/AffineStructures.h
    mlir/lib/Dialect/Affine/Analysis/AffineStructures.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Dialect/Affine/Analysis/AffineStructures.h b/mlir/include/mlir/Dialect/Affine/Analysis/AffineStructures.h
index 427d96fef23a6..0ddacad04792e 100644
--- a/mlir/include/mlir/Dialect/Affine/Analysis/AffineStructures.h
+++ b/mlir/include/mlir/Dialect/Affine/Analysis/AffineStructures.h
@@ -33,7 +33,7 @@ class MemRefType;
 struct MutableAffineMap;
 
 /// FlatAffineValueConstraints represents an extension of IntegerPolyhedron
-/// where each identifier can have an SSA Value attached to it.
+/// where each non-local identifier can have an SSA Value attached to it.
 class FlatAffineValueConstraints : public presburger::IntegerPolyhedron {
 public:
   /// Constructs a constraint system reserving memory for the specified number
@@ -48,10 +48,10 @@ class FlatAffineValueConstraints : public presburger::IntegerPolyhedron {
                           presburger::PresburgerSpace::getSetSpace(
                               numDims, numSymbols, numLocals)) {
     assert(numReservedCols >= getNumIds() + 1);
-    assert(valArgs.empty() || valArgs.size() == getNumIds());
+    assert(valArgs.empty() || valArgs.size() == getNumDimAndSymbolIds());
     values.reserve(numReservedCols);
     if (valArgs.empty())
-      values.resize(getNumIds(), None);
+      values.resize(getNumDimAndSymbolIds(), None);
     else
       values.append(valArgs.begin(), valArgs.end());
   }
@@ -70,9 +70,9 @@ class FlatAffineValueConstraints : public presburger::IntegerPolyhedron {
   FlatAffineValueConstraints(const IntegerPolyhedron &fac,
                              ArrayRef<Optional<Value>> valArgs = {})
       : IntegerPolyhedron(fac) {
-    assert(valArgs.empty() || valArgs.size() == getNumIds());
+    assert(valArgs.empty() || valArgs.size() == getNumDimAndSymbolIds());
     if (valArgs.empty())
-      values.resize(getNumIds(), None);
+      values.resize(getNumDimAndSymbolIds(), None);
     else
       values.append(valArgs.begin(), valArgs.end());
   }
@@ -275,9 +275,10 @@ class FlatAffineValueConstraints : public presburger::IntegerPolyhedron {
   /// Insert identifiers of the specified kind at position `pos`. Positions are
   /// relative to the kind of identifier. The coefficient columns corresponding
   /// to the added identifiers are initialized to zero. `vals` are the Values
-  /// corresponding to the identifiers. Return the absolute column position
-  /// (i.e., not relative to the kind of identifier) of the first added
-  /// identifier.
+  /// corresponding to the identifiers. Values should not be used with
+  /// IdKind::Local since values can only be attached to non-local identifiers.
+  /// Return the absolute column position (i.e., not relative to the kind of
+  /// identifier) of the first added identifier.
   ///
   /// Note: Empty Values are allowed in `vals`.
   unsigned insertDimId(unsigned pos, unsigned num = 1) {
@@ -397,12 +398,16 @@ class FlatAffineValueConstraints : public presburger::IntegerPolyhedron {
   /// Returns the Value associated with the pos^th identifier. Asserts if
   /// no Value identifier was associated.
   inline Value getValue(unsigned pos) const {
+    assert(pos < getNumDimAndSymbolIds() && "Invalid position");
     assert(hasValue(pos) && "identifier's Value not set");
     return values[pos].getValue();
   }
 
   /// Returns true if the pos^th identifier has an associated Value.
-  inline bool hasValue(unsigned pos) const { return values[pos].hasValue(); }
+  inline bool hasValue(unsigned pos) const {
+    assert(pos < getNumDimAndSymbolIds() && "Invalid position");
+    return values[pos].hasValue();
+  }
 
   /// Returns true if at least one identifier has an associated Value.
   bool hasValues() const;
@@ -411,15 +416,15 @@ class FlatAffineValueConstraints : public presburger::IntegerPolyhedron {
   /// Asserts if no Value was associated with one of these identifiers.
   inline void getValues(unsigned start, unsigned end,
                         SmallVectorImpl<Value> *values) const {
-    assert((start < getNumIds() || start == end) && "invalid start position");
-    assert(end <= getNumIds() && "invalid end position");
+    assert(end <= getNumDimAndSymbolIds() && "invalid end position");
+    assert(start <= end && "invalid start position");
     values->clear();
     values->reserve(end - start);
     for (unsigned i = start; i < end; i++)
       values->push_back(getValue(i));
   }
   inline void getAllValues(SmallVectorImpl<Value> *values) const {
-    getValues(0, getNumIds(), values);
+    getValues(0, getNumDimAndSymbolIds(), values);
   }
 
   inline ArrayRef<Optional<Value>> getMaybeValues() const {
@@ -440,15 +445,17 @@ class FlatAffineValueConstraints : public presburger::IntegerPolyhedron {
 
   /// Sets the Value associated with the pos^th identifier.
   inline void setValue(unsigned pos, Value val) {
-    assert(pos < getNumIds() && "invalid id position");
+    assert(pos < getNumDimAndSymbolIds() && "invalid id position");
     values[pos] = val;
   }
 
   /// Sets the Values associated with the identifiers in the range [start, end).
+  /// The range must contain only dim and symbol identifiers.
   void setValues(unsigned start, unsigned end, ArrayRef<Value> values) {
-    assert((start < getNumIds() || end == start) && "invalid start position");
     assert(end <= getNumIds() && "invalid end position");
-    assert(values.size() == end - start);
+    assert(start <= end && "invalid start position");
+    assert(values.size() == end - start &&
+           "value should be provided for each identifier in the range.");
     for (unsigned i = start; i < end; ++i)
       setValue(i, values[i - start]);
   }
@@ -495,9 +502,9 @@ class FlatAffineValueConstraints : public presburger::IntegerPolyhedron {
   /// an SSA Value attached to it.
   void printSpace(raw_ostream &os) const override;
 
-  /// Values corresponding to the (column) identifiers of this constraint
-  /// system appearing in the order the identifiers correspond to columns.
-  /// Temporary ones or those that aren't associated with any Value are set to
+  /// Values corresponding to the (column) non-local identifiers of this
+  /// constraint system appearing in the order the identifiers correspond to
+  /// columns. Identifiers that aren't associated with any Value are set to
   /// None.
   SmallVector<Optional<Value>, 8> values;
 };

diff  --git a/mlir/lib/Dialect/Affine/Analysis/AffineStructures.cpp b/mlir/lib/Dialect/Affine/Analysis/AffineStructures.cpp
index 434a2bfdffc16..b3bf543f93dc5 100644
--- a/mlir/lib/Dialect/Affine/Analysis/AffineStructures.cpp
+++ b/mlir/lib/Dialect/Affine/Analysis/AffineStructures.cpp
@@ -157,7 +157,7 @@ FlatAffineValueConstraints::FlatAffineValueConstraints(IntegerSet set)
                                                      /*numLocals=*/0)) {
 
   // Resize values.
-  values.resize(getNumIds(), None);
+  values.resize(getNumDimAndSymbolIds(), None);
 
   // Flatten expressions and add them to the constraint system.
   std::vector<SmallVector<int64_t, 8>> flatExprs;
@@ -294,14 +294,20 @@ unsigned FlatAffineValueConstraints::insertSymbolId(unsigned pos,
 unsigned FlatAffineValueConstraints::insertId(IdKind kind, unsigned pos,
                                               unsigned num) {
   unsigned absolutePos = IntegerPolyhedron::insertId(kind, pos, num);
-  values.insert(values.begin() + absolutePos, num, None);
-  assert(values.size() == getNumIds());
+
+  if (kind != IdKind::Local) {
+    values.insert(values.begin() + absolutePos, num, None);
+    assert(values.size() == getNumDimAndSymbolIds());
+  }
+
   return absolutePos;
 }
 
 unsigned FlatAffineValueConstraints::insertId(IdKind kind, unsigned pos,
                                               ValueRange vals) {
-  assert(!vals.empty() && "expected ValueRange with Values");
+  assert(!vals.empty() && "expected ValueRange with Values.");
+  assert(kind != IdKind::Local &&
+         "values cannot be attached to local identifiers.");
   unsigned num = vals.size();
   unsigned absolutePos = IntegerPolyhedron::insertId(kind, pos, num);
 
@@ -310,7 +316,7 @@ unsigned FlatAffineValueConstraints::insertId(IdKind kind, unsigned pos,
     values.insert(values.begin() + absolutePos + i,
                   vals[i] ? Optional<Value>(vals[i]) : None);
 
-  assert(values.size() == getNumIds());
+  assert(values.size() == getNumDimAndSymbolIds());
   return absolutePos;
 }
 
@@ -342,8 +348,9 @@ bool FlatAffineValueConstraints::areIdsAlignedWithOther(
 static bool LLVM_ATTRIBUTE_UNUSED areIdsUnique(
     const FlatAffineValueConstraints &cst, unsigned start, unsigned end) {
 
-  assert(start <= cst.getNumIds() && "Start position out of bounds");
-  assert(end <= cst.getNumIds() && "End position out of bounds");
+  assert(start <= cst.getNumDimAndSymbolIds() &&
+         "Start position out of bounds");
+  assert(end <= cst.getNumDimAndSymbolIds() && "End position out of bounds");
 
   if (start >= end)
     return true;
@@ -361,7 +368,7 @@ static bool LLVM_ATTRIBUTE_UNUSED areIdsUnique(
 /// Checks if the SSA values associated with `cst`'s identifiers are unique.
 static bool LLVM_ATTRIBUTE_UNUSED
 areIdsUnique(const FlatAffineValueConstraints &cst) {
-  return areIdsUnique(cst, 0, cst.getNumIds());
+  return areIdsUnique(cst, 0, cst.getNumDimAndSymbolIds());
 }
 
 /// Checks if the SSA values associated with `cst`'s identifiers of kind `kind`
@@ -373,8 +380,6 @@ areIdsUnique(const FlatAffineValueConstraints &cst, IdKind kind) {
     return areIdsUnique(cst, 0, cst.getNumDimIds());
   if (kind == IdKind::Symbol)
     return areIdsUnique(cst, cst.getNumDimIds(), cst.getNumDimAndSymbolIds());
-  if (kind == IdKind::Local)
-    return areIdsUnique(cst, cst.getNumDimAndSymbolIds(), cst.getNumIds());
   llvm_unreachable("Unexpected IdKind");
 }
 
@@ -395,11 +400,11 @@ static void mergeAndAlignIds(unsigned offset, FlatAffineValueConstraints *a,
   assert(areIdsUnique(*b) && "B's values aren't unique");
 
   assert(std::all_of(a->getMaybeValues().begin() + offset,
-                     a->getMaybeValues().begin() + a->getNumDimAndSymbolIds(),
+                     a->getMaybeValues().end(),
                      [](Optional<Value> id) { return id.hasValue(); }));
 
   assert(std::all_of(b->getMaybeValues().begin() + offset,
-                     b->getMaybeValues().begin() + b->getNumDimAndSymbolIds(),
+                     b->getMaybeValues().end(),
                      [](Optional<Value> id) { return id.hasValue(); }));
 
   SmallVector<Value, 4> aDimValues;
@@ -703,15 +708,18 @@ void FlatAffineValueConstraints::addAffineIfOpDomain(AffineIfOp ifOp) {
 
 bool FlatAffineValueConstraints::hasConsistentState() const {
   return IntegerPolyhedron::hasConsistentState() &&
-         values.size() == getNumIds();
+         values.size() == getNumDimAndSymbolIds();
 }
 
 void FlatAffineValueConstraints::removeIdRange(IdKind kind, unsigned idStart,
                                                unsigned idLimit) {
   IntegerPolyhedron::removeIdRange(kind, idStart, idLimit);
   unsigned offset = getIdKindOffset(kind);
-  values.erase(values.begin() + idStart + offset,
-               values.begin() + idLimit + offset);
+
+  if (kind != IdKind::Local) {
+    values.erase(values.begin() + idStart + offset,
+                 values.begin() + idLimit + offset);
+  }
 }
 
 // Determine whether the identifier at 'pos' (say id_r) can be expressed as
@@ -1339,7 +1347,17 @@ bool FlatAffineValueConstraints::containsId(Value val) const {
 
 void FlatAffineValueConstraints::swapId(unsigned posA, unsigned posB) {
   IntegerPolyhedron::swapId(posA, posB);
-  std::swap(values[posA], values[posB]);
+
+  if (getIdKindAt(posA) == IdKind::Local && getIdKindAt(posB) == IdKind::Local)
+    return;
+
+  // Treat value of a local identifer as None.
+  if (getIdKindAt(posA) == IdKind::Local)
+    values[posB] = None;
+  else if (getIdKindAt(posB) == IdKind::Local)
+    values[posA] = None;
+  else
+    std::swap(values[posA], values[posB]);
 }
 
 void FlatAffineValueConstraints::addBound(BoundType type, Value val,
@@ -1354,12 +1372,16 @@ void FlatAffineValueConstraints::addBound(BoundType type, Value val,
 void FlatAffineValueConstraints::printSpace(raw_ostream &os) const {
   IntegerPolyhedron::printSpace(os);
   os << "(";
-  for (unsigned i = 0, e = getNumIds(); i < e; i++) {
+  for (unsigned i = 0, e = getNumDimAndSymbolIds(); i < e; i++) {
     if (hasValue(i))
       os << "Value ";
     else
       os << "None ";
   }
+  for (unsigned i = getIdKindOffset(IdKind::Local),
+                e = getIdKindEnd(IdKind::Local);
+       i < e; ++i)
+    os << "Local ";
   os << " const)\n";
 }
 
@@ -1372,21 +1394,20 @@ void FlatAffineValueConstraints::clearAndCopyFrom(
   } else {
     *static_cast<IntegerRelation *>(this) = other;
     values.clear();
-    values.resize(getNumIds(), None);
+    values.resize(getNumDimAndSymbolIds(), None);
   }
 }
 
 void FlatAffineValueConstraints::fourierMotzkinEliminate(
     unsigned pos, bool darkShadow, bool *isResultIntegerExact) {
-  SmallVector<Optional<Value>, 8> newVals;
-  newVals.reserve(getNumIds() - 1);
-  newVals.append(values.begin(), values.begin() + pos);
-  newVals.append(values.begin() + pos + 1, values.end());
+  SmallVector<Optional<Value>, 8> newVals = values;
+  if (getIdKindAt(pos) != IdKind::Local)
+    newVals.erase(newVals.begin() + pos);
   // Note: Base implementation discards all associated Values.
   IntegerPolyhedron::fourierMotzkinEliminate(pos, darkShadow,
                                              isResultIntegerExact);
   values = newVals;
-  assert(values.size() == getNumIds());
+  assert(values.size() == getNumDimAndSymbolIds());
 }
 
 void FlatAffineValueConstraints::projectOut(Value val) {


        


More information about the Mlir-commits mailing list