[MLIR][Presburger] Rename attachments to identifiers in PresburgerSpace

[Groverkss]

Summary

"attachment" was a temporary name chosen for the information attached to a
variable in a PresburgerSpace. After the disambiguation of "variables" and
"identifiers" in PresburgerSpace, we use the word "identifiers" for this
information, since this information is used to "identify" these variables.

[arjunp]

mlir/include/mlir/Analysis/Presburger/PresburgerSpace.h

75–76

[arjunp]

SGTM

[Groverkss]

• Update PresburgerSpace doc

mlir/include/mlir/Analysis/Presburger/PresburgerSpace.h

 /// A space with no distinction between types of dimension variables can
 /// be implemented as a space with zero domain. VarKind::SetDim should be used
 /// to refer to dimensions in such spaces.
 ///
 /// Compatibility of two spaces implies that number of variables of each kind
 /// other than Locals are equal. Equality of two spaces implies that number of
 /// variables of each kind are equal.
 ///
-/// PresburgerSpace optionally also supports attaching attachments to each
-/// variable in space. `resetAttachments<AttachmentType>` enables attaching
-/// attachments to space. All attachments must be of the same type,
-/// `AttachmentType`. `AttachmentType` must have a
-/// `llvm::PointerLikeTypeTraits` specialization available and should be
-/// supported via mlir::TypeID.
+/// PresburgerSpace optionally also supports attaching some information to each
+/// variable in space, called "identifier" of that variable. `resetIds<IdType>`
+/// is used to enable/reset these identifiers. All identifiers must be of the
+/// same type, `IdType`. `IdType` must have a `llvm::PointerLikeTypeTraits`
+/// specialization available and should be supported via `mlir::TypeID`.
 ///
-/// These attachments can be used to check if two variables in two different
-/// spaces correspond to the same variable.
+/// These identifiers can be used to check if two variables in two different
+/// spaces are actually same variable.
 class PresburgerSpace {
 public:
   static PresburgerSpace getRelationSpace(unsigned numDomain = 0,
                                           unsigned numRange = 0,
                                           unsigned numSymbols = 0,
                                           unsigned numLocals = 0) {
     return PresburgerSpace(numDomain, numRange, numSymbols, numLocals);
   }
public:
   /// Return the VarKind of the var at the specified position.
   VarKind getVarKindAt(unsigned pos) const;
 
   /// Insert `num` variables of the specified kind at position `pos`.
   /// Positions are relative to the kind of variable. Return the absolute
   /// column position (i.e., not relative to the kind of variable) of the
   /// first added variable.
   ///
-  /// If attachments are being used, the newly added variables have no
-  /// attachments.
+  /// If identifiers are being used, the newly added variables have no
+  /// identifiers.
   unsigned insertVar(VarKind kind, unsigned pos, unsigned num = 1);
 
   /// Removes variables of the specified kind in the column range [varStart,
   /// varLimit). The range is relative to the kind of variable.
   void removeVarRange(VarKind kind, unsigned varStart, unsigned varLimit);
 
   /// Swaps the posA^th variable of kindA and posB^th variable of kindB.
   void swapVar(VarKind kindA, VarKind kindB, unsigned posA, unsigned posB);
public:
   /// the split become symbols, or some of the symbols immediately after the
   /// split become dimensions.
   void setVarSymbolSeperation(unsigned newSymbolCount);
 
   void print(llvm::raw_ostream &os) const;
   void dump() const;
 
   //===--------------------------------------------------------------------===//
-  //     Attachment Interactions
+  //     Identifier Interactions
   //===--------------------------------------------------------------------===//
 
-  /// Set the attachment for `i^th` variable to `attachment`. `T` here should
-  /// match the type used to enable attachments.
+  /// Set the identifier for `i^th` variable to `id`. `T` here should match the
+  /// type used to enable identifiers.
   template <typename T>
-  void setAttachment(VarKind kind, unsigned i, T attachment) {
+  void setId(VarKind kind, unsigned i, T id) {
 #ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
-    assert(TypeID::get<T>() == attachmentType && "Type mismatch");
+    assert(TypeID::get<T>() == idType && "Type mismatch");
 #endif
-    atAttachment(kind, i) =
-        llvm::PointerLikeTypeTraits<T>::getAsVoidPointer(attachment);
+    atId(kind, i) = llvm::PointerLikeTypeTraits<T>::getAsVoidPointer(id);
   }
 
-  /// Get the attachment for `i^th` variable casted to type `T`. `T` here
-  /// should match the type used to enable attachments.
+  /// Get the identifier for `i^th` variable casted to type `T`. `T` here
+  /// should match the type used to enable identifiers.
   template <typename T>
-  T getAttachment(VarKind kind, unsigned i) const {
+  T getId(VarKind kind, unsigned i) const {
 #ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
-    assert(TypeID::get<T>() == attachmentType && "Type mismatch");
+    assert(TypeID::get<T>() == idType && "Type mismatch");
 #endif
-    return llvm::PointerLikeTypeTraits<T>::getFromVoidPointer(
-        atAttachment(kind, i));
+    return llvm::PointerLikeTypeTraits<T>::getFromVoidPointer(atId(kind, i));
   }
 
   /// Check if the i^th variable of the specified kind has a non-null
-  /// attachment.
-  bool hasAttachment(VarKind kind, unsigned i) const {
-    return atAttachment(kind, i) != nullptr;
+  /// identifier.
+  bool hasId(VarKind kind, unsigned i) const {
+    return atId(kind, i) != nullptr;
   }
 
-  /// Check if the spaces are compatible, as well as have the same attachments
+  /// Check if the spaces are compatible, as well as have the same identifiers
   /// for each variable.
   bool isAligned(const PresburgerSpace &other) const;
   /// Check if the number of variables of the specified kind match, and have
-  /// same attachments with the other space.
+  /// same identifiers with the other space.
   bool isAligned(const PresburgerSpace &other, VarKind kind) const;
 
-  /// Find the variable of the specified kind with attachment `val`.
-  /// PresburgerSpace::kIdNotFound if attachment is not found.
+  /// Find the variable of the specified kind with identifier `id`.
+  /// Returns PresburgerSpace::kIdNotFound if identifier is not found.
   template <typename T>
-  unsigned findId(VarKind kind, T val) const {
+  unsigned findId(VarKind kind, T id) const {
     unsigned i = 0;
     for (unsigned e = getNumVarKind(kind); i < e; ++i)
-      if (hasAttachment(kind, i) && getAttachment<T>(kind, i) == val)
+      if (hasId(kind, i) && getId<T>(kind, i) == id)
         return i;
     return kIdNotFound;
   }
   static const unsigned kIdNotFound = UINT_MAX;
 
-  /// Returns if attachments are being used.
-  bool isUsingAttachments() const { return usingAttachments; }
+  /// Returns if identifiers are being used.
+  bool isUsingIds() const { return usingIds; }
 
-  /// Reset the stored attachments in the space. Enables `usingAttachments` if
-  /// it was `false` before.
+  /// Reset the stored identifiers in the space. Enables `usingIds` if it was
+  /// `false` before.
   template <typename T>
-  void resetAttachments() {
-    attachments.clear();
-    attachments.resize(getNumDimAndSymbolVars());
+  void resetIds() {
+    identifiers.clear();
+    identifiers.resize(getNumDimAndSymbolVars());
 #ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
-    attachmentType = TypeID::get<T>();
+    idType = TypeID::get<T>();
 #endif
 
-    usingAttachments = true;
+    usingIds = true;
   }
 
-  /// Disable attachments being stored in space.
-  void disableAttachments() {
-    attachments.clear();
-    usingAttachments = false;
+  /// Disable identifiers being stored in space.
+  void disableIds() {
+    identifiers.clear();
+    usingIds = false;
   }
 
 protected:
   PresburgerSpace(unsigned numDomain = 0, unsigned numRange = 0,
                   unsigned numSymbols = 0, unsigned numLocals = 0)
       : numDomain(numDomain), numRange(numRange), numSymbols(numSymbols),
         numLocals(numLocals) {}
 
-  void *&atAttachment(VarKind kind, unsigned i) {
-    assert(usingAttachments &&
-           "Cannot access attachments when `usingAttachments` is false.");
+  void *&atId(VarKind kind, unsigned i) {
+    assert(usingIds && "Cannot access identifiers when `usingIds` is false.");
     assert(kind != VarKind::Local &&
-           "Local variables cannot have attachments.");
-    return attachments[getVarKindOffset(kind) + i];
+           "Local variables cannot have identifiers.");
+    return identifiers[getVarKindOffset(kind) + i];
   }
 
-  void *atAttachment(VarKind kind, unsigned i) const {
-    assert(usingAttachments &&
-           "Cannot access attachments when `usingAttachments` is false.");
+  void *atId(VarKind kind, unsigned i) const {
+    assert(usingIds && "Cannot access identifiers when `usingIds` is false.");
     assert(kind != VarKind::Local &&
-           "Local variables cannot have attachments.");
-    return attachments[getVarKindOffset(kind) + i];
+           "Local variables cannot have identifiers.");
+    return identifiers[getVarKindOffset(kind) + i];
   }
 
 private:
   // Number of variables corresponding to domain variables.
   unsigned numDomain;
 
   // Number of variables corresponding to range variables.
   unsigned numRange;
 
   /// Number of variables corresponding to symbols (unknown but constant for
   /// analysis).
   unsigned numSymbols;
 
   /// Number of variables corresponding to locals (variables corresponding
   /// to existentially quantified variables).
   unsigned numLocals;
 
-  /// Stores whether or not attachments are being used in this space.
-  bool usingAttachments = false;
+  /// Stores whether or not identifiers are being used in this space.
+  bool usingIds = false;
 
 #ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
-  /// TypeID of the attachments in space. This should be used in asserts only.
-  TypeID attachmentType;
+  /// TypeID of the identifiers in space. This should be used in asserts only.
+  TypeID idType;
 #endif
 
-  /// Stores a attachment for each non-local variable as a `void` pointer.
-  SmallVector<void *, 0> attachments;
+  /// Stores an identifier for each non-local variable as a `void` pointer.
+  SmallVector<void *, 0> identifiers;
 };
 
 } // namespace presburger
 } // namespace mlir
 
 #endif // MLIR_ANALYSIS_PRESBURGER_PRESBURGERSPACE_H

mlir/lib/Analysis/Presburger/PresburgerSpace.cpp

if (kind == VarKind::Domain)
     numDomain += num;
   else if (kind == VarKind::Range)
     numRange += num;
   else if (kind == VarKind::Symbol)
     numSymbols += num;
   else
     numLocals += num;
 
-  // Insert NULL attachments if `usingAttachments` and variables inserted are
+  // Insert NULL identifiers if `usingIds` and variables inserted are
   // not locals.
-  if (usingAttachments && kind != VarKind::Local)
-    attachments.insert(attachments.begin() + absolutePos, num, nullptr);
+  if (usingIds && kind != VarKind::Local)
+    identifiers.insert(identifiers.begin() + absolutePos, num, nullptr);
 
   return absolutePos;
 }
 
 void PresburgerSpace::removeVarRange(VarKind kind, unsigned varStart,
                                      unsigned varLimit) {
   assert(varLimit <= getNumVarKind(kind) && "invalid var limit");
 
   if (varStart >= varLimit)
     return;
 
   unsigned numVarsEliminated = varLimit - varStart;
   if (kind == VarKind::Domain)
     numDomain -= numVarsEliminated;
   else if (kind == VarKind::Range)
     numRange -= numVarsEliminated;
   else if (kind == VarKind::Symbol)
     numSymbols -= numVarsEliminated;
   else
     numLocals -= numVarsEliminated;
 
-  // Remove attachments if `usingAttachments` and variables removed are not
+  // Remove identifiers if `usingIds` and variables removed are not
   // locals.
-  if (usingAttachments && kind != VarKind::Local)
-    attachments.erase(attachments.begin() + getVarKindOffset(kind) + varStart,
-                      attachments.begin() + getVarKindOffset(kind) + varLimit);
+  if (usingIds && kind != VarKind::Local)
+    identifiers.erase(identifiers.begin() + getVarKindOffset(kind) + varStart,
+                      identifiers.begin() + getVarKindOffset(kind) + varLimit);
 }
 
 void PresburgerSpace::swapVar(VarKind kindA, VarKind kindB, unsigned posA,
                               unsigned posB) {
 
-  if (!usingAttachments)
+  if (!usingIds)
     return;
 
   if (kindA == VarKind::Local && kindB == VarKind::Local)
     return;
 
   if (kindA == VarKind::Local) {
-    atAttachment(kindB, posB) = nullptr;
+    atId(kindB, posB) = nullptr;
     return;
   }
 
   if (kindB == VarKind::Local) {
-    atAttachment(kindA, posA) = nullptr;
+    atId(kindA, posA) = nullptr;
     return;
   }
 
-  std::swap(atAttachment(kindA, posA), atAttachment(kindB, posB));
+  std::swap(atId(kindA, posA), atId(kindB, posB));
 }
 
 bool PresburgerSpace::isCompatible(const PresburgerSpace &other) const {
   return getNumDomainVars() == other.getNumDomainVars() &&
          getNumRangeVars() == other.getNumRangeVars() &&
          getNumSymbolVars() == other.getNumSymbolVars();
 }
 
 bool PresburgerSpace::isEqual(const PresburgerSpace &other) const {
   return isCompatible(other) && getNumLocalVars() == other.getNumLocalVars();
 }
 
 bool PresburgerSpace::isAligned(const PresburgerSpace &other) const {
-  assert(isUsingAttachments() && other.isUsingAttachments() &&
-         "Both spaces should be using attachments to check for "
+  assert(isUsingIds() && other.isUsingIds() &&
+         "Both spaces should be using identifiers to check for "
          "alignment.");
-  return isCompatible(other) && attachments == other.attachments;
+  return isCompatible(other) && identifiers == other.identifiers;
 }
 
 bool PresburgerSpace::isAligned(const PresburgerSpace &other,
                                 VarKind kind) const {
-  assert(isUsingAttachments() && other.isUsingAttachments() &&
-         "Both spaces should be using attachments to check for "
+  assert(isUsingIds() && other.isUsingIds() &&
+         "Both spaces should be using identifiers to check for "
          "alignment.");
 
   ArrayRef<void *> kindAttachments =
-      makeArrayRef(attachments)
+      makeArrayRef(identifiers)
           .slice(getVarKindOffset(kind), getNumVarKind(kind));
   ArrayRef<void *> otherKindAttachments =
-      makeArrayRef(other.attachments)
+      makeArrayRef(other.identifiers)
           .slice(other.getVarKindOffset(kind), other.getNumVarKind(kind));
   return kindAttachments == otherKindAttachments;
 }
 
 void PresburgerSpace::setVarSymbolSeperation(unsigned newSymbolCount) {
   assert(newSymbolCount <= getNumDimAndSymbolVars() &&
          "invalid separation position");
   numRange = numRange + numSymbols - newSymbolCount;
   numSymbols = newSymbolCount;
-  // We do not need to change `attachments` since the ordering of
-  // `attachments` remains same.
+  // We do not need to change `identifiers` since the ordering of
+  // `identifiers` remains same.
 }
 
 void PresburgerSpace::print(llvm::raw_ostream &os) const {
   os << "Domain: " << getNumDomainVars() << ", "
      << "Range: " << getNumRangeVars() << ", "
      << "Symbols: " << getNumSymbolVars() << ", "
      << "Locals: " << getNumLocalVars() << "\n";
 
-  if (usingAttachments) {
+  if (usingIds) {
 #ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
-    os << "TypeID of attachments: " << attachmentType.getAsOpaquePointer()
-       << "\n";
+    os << "TypeID of identifiers: " << idType.getAsOpaquePointer() << "\n";
 #endif
 
     os << "(";
-    for (void *attachment : attachments)
-      os << attachment << " ";
+    for (void *identifier : identifiers)
+      os << identifier << " ";
     os << ")\n";
   }
 }
 
 void PresburgerSpace::dump() const { print(llvm::errs()); }

mlir/unittests/Analysis/Presburger/PresburgerSpaceTest.cpp

TEST(PresburgerSpaceTest, removeIdRange) {
   // Remove 1 symbol and 1 range identifier.
   space.removeVarRange(VarKind::Symbol, 0, 1);
   space.removeVarRange(VarKind::Range, 0, 1);
   EXPECT_EQ(space.getNumDomainVars(), 1u);
   EXPECT_EQ(space.getNumRangeVars(), 0u);
   EXPECT_EQ(space.getNumSymbolVars(), 2u);
 }
 
-TEST(PresburgerSpaceTest, insertIdAttachment) {
+TEST(PresburgerSpaceTest, insertVarIdentifier) {
   PresburgerSpace space = PresburgerSpace::getRelationSpace(2, 2, 1, 0);
-  space.resetAttachments<int *>();
+  space.resetIds<int *>();
 
-  // Attach attachment to domain ids.
-  int attachments[2] = {0, 1};
-  space.setAttachment<int *>(VarKind::Domain, 0, &attachments[0]);
-  space.setAttachment<int *>(VarKind::Domain, 1, &attachments[1]);
+  // Attach identifiers to domain ids.
+  int identifiers[2] = {0, 1};
+  space.setId<int *>(VarKind::Domain, 0, &identifiers[0]);
+  space.setId<int *>(VarKind::Domain, 1, &identifiers[1]);
 
   // Try inserting 2 domain ids.
   space.insertVar(VarKind::Domain, 0, 2);
   EXPECT_EQ(space.getNumDomainVars(), 4u);
 
   // Try inserting 1 range ids.
   space.insertVar(VarKind::Range, 0, 1);
   EXPECT_EQ(space.getNumRangeVars(), 3u);
 
-  // Check if the attachments for the old ids are still attached properly.
-  EXPECT_EQ(*space.getAttachment<int *>(VarKind::Domain, 2), attachments[0]);
-  EXPECT_EQ(*space.getAttachment<int *>(VarKind::Domain, 3), attachments[1]);
+  // Check if the identifiers for the old ids are still attached properly.
+  EXPECT_EQ(*space.getId<int *>(VarKind::Domain, 2), identifiers[0]);
+  EXPECT_EQ(*space.getId<int *>(VarKind::Domain, 3), identifiers[1]);
 }
 
-TEST(PresburgerSpaceTest, removeIdRangeAttachment) {
+TEST(PresburgerSpaceTest, removeVarRangeIdentifier) {
   PresburgerSpace space = PresburgerSpace::getRelationSpace(2, 1, 3, 0);
-  space.resetAttachments<int *>();
+  space.resetIds<int *>();
 
-  int attachments[6] = {0, 1, 2, 3, 4, 5};
+  int identifiers[6] = {0, 1, 2, 3, 4, 5};
 
-  // Attach attachments to domain identifiers.
-  space.setAttachment<int *>(VarKind::Domain, 0, &attachments[0]);
-  space.setAttachment<int *>(VarKind::Domain, 1, &attachments[1]);
+  // Attach identifiers to domain identifiers.
+  space.setId<int *>(VarKind::Domain, 0, &identifiers[0]);
+  space.setId<int *>(VarKind::Domain, 1, &identifiers[1]);
 
-  // Attach attachments to range identifiers.
-  space.setAttachment<int *>(VarKind::Range, 0, &attachments[2]);
+  // Attach identifiers to range identifiers.
+  space.setId<int *>(VarKind::Range, 0, &identifiers[2]);
 
-  // Attach attachments to symbol identifiers.
-  space.setAttachment<int *>(VarKind::Symbol, 0, &attachments[3]);
-  space.setAttachment<int *>(VarKind::Symbol, 1, &attachments[4]);
-  space.setAttachment<int *>(VarKind::Symbol, 2, &attachments[5]);
+  // Attach identifiers to symbol identifiers.
+  space.setId<int *>(VarKind::Symbol, 0, &identifiers[3]);
+  space.setId<int *>(VarKind::Symbol, 1, &identifiers[4]);
+  space.setId<int *>(VarKind::Symbol, 2, &identifiers[5]);
 
   // Remove 1 domain identifier.
   space.removeVarRange(VarKind::Domain, 0, 1);
   EXPECT_EQ(space.getNumDomainVars(), 1u);
 
   // Remove 1 symbol and 1 range identifier.
   space.removeVarRange(VarKind::Symbol, 0, 1);
   space.removeVarRange(VarKind::Range, 0, 1);
   EXPECT_EQ(space.getNumDomainVars(), 1u);
   EXPECT_EQ(space.getNumRangeVars(), 0u);
   EXPECT_EQ(space.getNumSymbolVars(), 2u);
 
-  // Check if domain attachments are attached properly.
-  EXPECT_EQ(*space.getAttachment<int *>(VarKind::Domain, 0), attachments[1]);
+  // Check if domain identifiers are attached properly.
+  EXPECT_EQ(*space.getId<int *>(VarKind::Domain, 0), identifiers[1]);
 
-  // Check if symbol attachments are attached properly.
-  EXPECT_EQ(*space.getAttachment<int *>(VarKind::Range, 0), attachments[4]);
-  EXPECT_EQ(*space.getAttachment<int *>(VarKind::Range, 1), attachments[5]);
+  // Check if symbol identifiers are attached properly.
+  EXPECT_EQ(*space.getId<int *>(VarKind::Range, 0), identifiers[4]);
+  EXPECT_EQ(*space.getId<int *>(VarKind::Range, 1), identifiers[5]);
 }