[MLIR][Presburger][NFC] Use getters for IntegerPolyhedron members

[Groverkss]

Summary

This patch makes IntegerPolyhedron and derived classes use of getters to access
IntegerPolyhedron space information (numIds, numDims, numSymbols) instead of
directly accessing them.

This patch makes it easier to change the underlying implementation of the way
identifiers are stored, making it easier to extend/modify existing implementation.

[Groverkss]

• Remove private label

[arjunp]

Please write the rationale for this change in the patch description.

mlir/lib/Analysis/Presburger/IntegerPolyhedron.cpp

181–185

I think it would be great to replace this logic with uniform handling of each type of variable that does not depend on their internal ordering. Just write a free function getNumOverlap that returns the number of common elements between two intervals and you can call it once for each kind.

236–242

[Groverkss]

• Address Arjun's comments

[arjunp]

mlir/include/mlir/Analysis/Presburger/IntegerPolyhedron.h

507

[Groverkss]

• Fix getIdKindOverlap docs

[Groverkss]

Rebase (Flang seems to give error on windows build)

mlir/include/mlir/Analysis/Presburger/IntegerPolyhedron.h

protected:
 
   /// Prints the number of constraints, dimensions, symbols and locals in the
   /// IntegerPolyhedron.
   virtual void printSpace(raw_ostream &os) const;
 
   /// Return the index at which the specified kind of id starts.
   unsigned getIdKindOffset(IdKind kind) const;
 
+  /// Return the index at which the specified kind of id ends.
+  unsigned getIdKindEnd(IdKind kind) const;
+
   /// Get the number of ids of the specified kind.
   unsigned getNumIdKind(IdKind kind) const;
 
+  /// Get the number of elements of the specified kind in the range
+  /// [idStart, idLimit).
+  unsigned getIdKindOverlap(IdKind kind, unsigned idStart,
+                            unsigned idLimit) const;
+
   /// Removes identifiers in the column range [idStart, idLimit), and copies any
   /// remaining valid data into place, updates member variables, and resizes
   /// arrays as needed.
   virtual void removeIdRange(unsigned idStart, unsigned idLimit);
 
   /// A parameter that controls detection of an unrealistic number of
   /// constraints. If the number of constraints is this many times the number of
   /// variables, we consider such a system out of line with the intended use

mlir/lib/Analysis/Presburger/IntegerPolyhedron.cpp

 }
 
 bool IntegerPolyhedron::isSubsetOf(const IntegerPolyhedron &other) const {
   return PresburgerSet(*this).isSubsetOf(PresburgerSet(other));
 }
 
 Optional<SmallVector<Fraction, 8>>
 IntegerPolyhedron::getRationalLexMin() const {
-  assert(numSymbols == 0 && "Symbols are not supported!");
+  assert(getNumSymbolIds() == 0 && "Symbols are not supported!");
   Optional<SmallVector<Fraction, 8>> maybeLexMin =
       LexSimplex(*this).getRationalLexMin();
 
   if (!maybeLexMin)
     return {};
 
   // The Simplex returns the lexmin over all the variables including locals. But
   // locals are not actually part of the space and should not be returned in the

 
 void IntegerPolyhedron::removeIdRange(unsigned idStart, unsigned idLimit) {
   assert(idLimit < getNumCols() && "invalid id limit");
 
   if (idStart >= idLimit)
     return;
 
   // We are going to be removing one or more identifiers from the range.
-  assert(idStart < numIds && "invalid idStart position");
+  assert(idStart < getNumIds() && "invalid idStart position");
 
-  // TODO: Make 'removeIdRange' a lambda called from here.
   // Remove eliminated identifiers from the constraints..
   equalities.removeColumns(idStart, idLimit - idStart);
   inequalities.removeColumns(idStart, idLimit - idStart);
 
   // Update members numDims, numSymbols and numIds.
-  unsigned numDimsEliminated = 0;
-  unsigned numLocalsEliminated = 0;
-  unsigned numColsEliminated = idLimit - idStart;
-  if (idStart < numDims) {
-    numDimsEliminated = std::min(numDims, idLimit) - idStart;
-  }
-  // Check how many local id's were removed. Note that our identifier order is
-  // [dims, symbols, locals]. Local id start at position numDims + numSymbols.
-  if (idLimit > numDims + numSymbols) {
-    numLocalsEliminated = std::min(
-        idLimit - std::max(idStart, numDims + numSymbols), getNumLocalIds());
-  }
+  unsigned numDimsEliminated =
+      getIdKindOverlap(IdKind::Dimension, idStart, idLimit);
   unsigned numSymbolsEliminated =
-      numColsEliminated - numDimsEliminated - numLocalsEliminated;
+      getIdKindOverlap(IdKind::Symbol, idStart, idLimit);
 
   numDims -= numDimsEliminated;
   numSymbols -= numSymbolsEliminated;
-  numIds = numIds - numColsEliminated;
+  numIds -= (idLimit - idStart);
 }
 
 void IntegerPolyhedron::removeEquality(unsigned pos) {
   equalities.removeRow(pos);
 }
 
 void IntegerPolyhedron::removeInequality(unsigned pos) {
   inequalities.removeRow(pos);
if (kind == IdKind::Dimension)
     return 0;
   if (kind == IdKind::Symbol)
     return getNumDimIds();
   if (kind == IdKind::Local)
     return getNumDimAndSymbolIds();
   llvm_unreachable("IdKind expected to be Dimension, Symbol or Local!");
 }
 
+unsigned IntegerPolyhedron::getIdKindEnd(IdKind kind) const {
+  return getIdKindOffset(kind) + getNumIdKind(kind);
+}
+
 unsigned IntegerPolyhedron::getNumIdKind(IdKind kind) const {
   if (kind == IdKind::Dimension)
     return getNumDimIds();
   if (kind == IdKind::Symbol)
     return getNumSymbolIds();
   if (kind == IdKind::Local)
     return getNumLocalIds();
   llvm_unreachable("IdKind expected to be Dimension, Symbol or Local!");
 }
 
+unsigned IntegerPolyhedron::getIdKindOverlap(IdKind kind, unsigned idStart,
+                                             unsigned idLimit) const {
+  unsigned idRangeStart = getIdKindOffset(kind);
+  unsigned idRangeEnd = getIdKindEnd(kind);
+
+  // Compute number of elements in intersection of the ranges [idStart, idLimit)
+  // and [idRangeStart, idRangeEnd).
+  unsigned overlapStart = std::max(idStart, idRangeStart);
+  unsigned overlapEnd = std::min(idLimit, idRangeEnd);
+
+  if (overlapStart > overlapEnd)
+    return 0;
+  return overlapEnd - overlapStart;
+}
+
 void IntegerPolyhedron::clearConstraints() {
   equalities.resizeVertically(0);
   inequalities.resizeVertically(0);
 }
 
 /// Gather all lower and upper bounds of the identifier at `pos`, and
 /// optionally any equalities on it. In addition, the bounds are to be
 /// independent of identifiers in position range [`offset`, `offset` + `num`).

 
 bool IntegerPolyhedron::hasConsistentState() const {
   if (!inequalities.hasConsistentState())
     return false;
   if (!equalities.hasConsistentState())
     return false;
 
   // Catches errors where numDims, numSymbols, numIds aren't consistent.
-  if (numDims > numIds || numSymbols > numIds || numDims + numSymbols > numIds)
+  if (getNumDimIds() > getNumIds() || getNumSymbolIds() > getNumIds() ||
+      getNumDimAndSymbolIds() > getNumIds())
     return false;
 
   return true;
 }
 
 void IntegerPolyhedron::setAndEliminate(unsigned pos,
                                         ArrayRef<int64_t> values) {
   if (values.empty())
void IntegerPolyhedron::gcdTightenInequalities() {
   }
 }
 
 // Eliminates all identifier variables in column range [posStart, posLimit).
 // Returns the number of variables eliminated.
 unsigned IntegerPolyhedron::gaussianEliminateIds(unsigned posStart,
                                                  unsigned posLimit) {
   // Return if identifier positions to eliminate are out of range.
-  assert(posLimit <= numIds);
+  assert(posLimit <= getNumIds());
   assert(hasConsistentState());
 
   if (posStart >= posLimit)
     return 0;
 
   gcdTightenInequalities();
 
   unsigned pivotCol = 0;
void IntegerPolyhedron::addLocalFloorDiv(ArrayRef<int64_t> dividend,
   // -dividend +qdivisor * q + divisor - 1 >= 0
   std::transform(bound.begin(), bound.end(), bound.begin(),
                  std::negate<int64_t>());
   bound[bound.size() - 1] += divisor - 1;
   addInequality(bound);
 }
 
 void IntegerPolyhedron::setDimSymbolSeparation(unsigned newSymbolCount) {
-  assert(newSymbolCount <= numDims + numSymbols &&
+  assert(newSymbolCount <= getNumDimAndSymbolIds() &&
          "invalid separation position");
   numDims = numDims + numSymbols - newSymbolCount;
   numSymbols = newSymbolCount;
 }
 
 /// Finds an equality that equates the specified identifier to a constant.
 /// Returns the position of the equality row. If 'symbolic' is set to true,
 /// symbols are also treated like a constant, i.e., an affine function of the
for (unsigned r = 0, e = a.getNumEqualities(); r < e; ++r) {
     }
   }
 }
 
 // Computes the bounding box with respect to 'other' by finding the min of the
 // lower bounds and the max of the upper bounds along each of the dimensions.
 LogicalResult
 IntegerPolyhedron::unionBoundingBox(const IntegerPolyhedron &otherCst) {
-  assert(otherCst.getNumDimIds() == numDims && "dims mismatch");
+  assert(otherCst.getNumDimIds() == getNumDimIds() && "dims mismatch");
   assert(otherCst.getNumLocalIds() == 0 && "local ids not supported here");
   assert(getNumLocalIds() == 0 && "local ids not supported yet here");
 
   // Get the constraints common to both systems; these will be added as is to
   // the union.
   IntegerPolyhedron commonCst;
   getCommonConstraints(*this, otherCst, commonCst);
 

mlir/lib/Dialect/Affine/Analysis/AffineStructures.cpp

FlatAffineConstraints::FlatAffineConstraints(IntegerSet set)
   }
   // Add the other constraints involving local id's from flattening.
   append(localVarCst);
 }
 
 // Construct from an IntegerSet.
 FlatAffineValueConstraints::FlatAffineValueConstraints(IntegerSet set)
     : FlatAffineConstraints(set) {
-  values.resize(numIds, None);
+  values.resize(getNumIds(), None);
 }
 
 // Construct a hyperrectangular constraint set from ValueRanges that represent
 // induction variables, lower and upper bounds. `ivs`, `lbs` and `ubs` are
 // expected to match one to one. The order of variables and constraints is:
 //
 // ivs | lbs | ubs | eq/ineq
 // ----+-----+-----+---------
FlatAffineValueConstraints::computeAlignedMap(AffineMap map,
   SmallVector<Value> dims, syms;
 #ifndef NDEBUG
   SmallVector<Value> newSyms;
   SmallVector<Value> *newSymsPtr = &newSyms;
 #else
   SmallVector<Value> *newSymsPtr = nullptr;
 #endif // NDEBUG
 
-  dims.reserve(numDims);
-  syms.reserve(numSymbols);
-  for (unsigned i = 0; i < numDims; ++i)
+  dims.reserve(getNumDimIds());
+  syms.reserve(getNumSymbolIds());
+  for (unsigned i = getIdKindOffset(IdKind::Dimension),
+                e = getIdKindEnd(IdKind::Dimension);
+       i < e; ++i)
     dims.push_back(values[i] ? *values[i] : Value());
-  for (unsigned i = numDims, e = numDims + numSymbols; i < e; ++i)
+  for (unsigned i = getIdKindOffset(IdKind::Symbol),
+                e = getIdKindEnd(IdKind::Symbol);
+       i < e; ++i)
     syms.push_back(values[i] ? *values[i] : Value());
 
   AffineMap alignedMap =
       alignAffineMapWithValues(map, operands, dims, syms, newSymsPtr);
   // All symbols are already part of this FlatAffineConstraints.
   assert(syms.size() == newSymsPtr->size() && "unexpected new/missing symbols");
   assert(std::equal(syms.begin(), syms.end(), newSymsPtr->begin()) &&
          "unexpected new/missing symbols");
void FlatAffineValueConstraints::clearAndCopyFrom(
   }
 
   if (auto *otherValueSet = dyn_cast<const FlatAffineValueConstraints>(&other))
     *static_cast<FlatAffineConstraints *>(this) = *otherValueSet;
   else
     *static_cast<IntegerPolyhedron *>(this) = other;
 
   values.clear();
-  values.resize(numIds, None);
+  values.resize(getNumIds(), None);
 }
 
 void FlatAffineValueConstraints::fourierMotzkinEliminate(
     unsigned pos, bool darkShadow, bool *isResultIntegerExact) {
   SmallVector<Optional<Value>, 8> newVals;
-  newVals.reserve(numIds - 1);
+  newVals.reserve(getNumIds() - 1);
   newVals.append(values.begin(), values.begin() + pos);
   newVals.append(values.begin() + pos + 1, values.end());
   // Note: Base implementation discards all associated Values.
   FlatAffineConstraints::fourierMotzkinEliminate(pos, darkShadow,
                                                  isResultIntegerExact);
   values = newVals;
   assert(values.size() == getNumIds());
 }
 
 void FlatAffineValueConstraints::projectOut(Value val) {
   unsigned pos;
   bool ret = findId(val, &pos);
   assert(ret);
   (void)ret;
   fourierMotzkinEliminate(pos);
 }
 
 LogicalResult FlatAffineValueConstraints::unionBoundingBox(
     const FlatAffineValueConstraints &otherCst) {
-  assert(otherCst.getNumDimIds() == numDims && "dims mismatch");
+  assert(otherCst.getNumDimIds() == getNumDimIds() && "dims mismatch");
   assert(otherCst.getMaybeValues()
              .slice(0, getNumDimIds())
              .equals(getMaybeValues().slice(0, getNumDimIds())) &&
          "dim values mismatch");
   assert(otherCst.getNumLocalIds() == 0 && "local ids not supported here");
   assert(getNumLocalIds() == 0 && "local ids not supported yet here");
 
   // Align `other` to this.
   if (!areIdsAligned(*this, otherCst)) {
     FlatAffineValueConstraints otherCopy(otherCst);
-    mergeAndAlignIds(/*offset=*/numDims, this, &otherCopy);
+    mergeAndAlignIds(/*offset=*/getNumDimIds(), this, &otherCopy);
     return FlatAffineConstraints::unionBoundingBox(otherCopy);
   }
 
   return FlatAffineConstraints::unionBoundingBox(otherCst);
 }
 
 /// Compute an explicit representation for local vars. For all systems coming
 /// from MLIR integer sets, maps, or expressions where local vars were