[llvm] [polly] [AST] Don't merge memory locations in AliasSetTracker (PR #65731)

Bruno De Fraine via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 12 02:53:46 PST 2024


================
@@ -317,62 +262,67 @@ AliasSet *AliasSetTracker::findAliasSetForUnknownInst(Instruction *Inst) {
 }
 
 AliasSet &AliasSetTracker::getAliasSetFor(const MemoryLocation &MemLoc) {
+  // The alias sets are indexed with a map from the memory locations' pointer
+  // values. If the memory location is already registered, we can find it in the
+  // alias set associated with its pointer.
+  AliasSet *&MapEntry = PointerMap[MemLoc.Ptr];
+  if (MapEntry) {
+    AliasSet *AS = MapEntry->getForwardedTarget(*this);
+    if (llvm::is_contained(AS->MemoryLocs, MemLoc)) {
+      if (AS != MapEntry) {
+        AS->addRef();
+        MapEntry->dropRef(*this);
+        MapEntry = AS;
+      }
+      return *AS;
+    }
+  }
 
-  Value * const Pointer = const_cast<Value*>(MemLoc.Ptr);
-  const LocationSize Size = MemLoc.Size;
-  const AAMDNodes &AAInfo = MemLoc.AATags;
-
-  AliasSet::PointerRec &Entry = getEntryFor(Pointer);
-
+  AliasSet *AS;
+  bool MustAliasAll = false;
   if (AliasAnyAS) {
     // At this point, the AST is saturated, so we only have one active alias
     // set. That means we already know which alias set we want to return, and
     // just need to add the pointer to that set to keep the data structure
     // consistent.
     // This, of course, means that we will never need a merge here.
-    if (Entry.hasAliasSet()) {
-      Entry.updateSizeAndAAInfo(Size, AAInfo);
-      assert(Entry.getAliasSet(*this) == AliasAnyAS &&
-             "Entry in saturated AST must belong to only alias set");
-    } else {
-      AliasAnyAS->addPointer(*this, Entry, Size, AAInfo);
-    }
-    return *AliasAnyAS;
+    AS = AliasAnyAS;
+  } else if (AliasSet *AliasAS =
+                 mergeAliasSetsForPointer(MemLoc, MustAliasAll)) {
+    // Add it to the alias set it aliases.
+    AS = AliasAS;
+  } else if (MapEntry) {
+    // Although we have an independent memory location, forgo creating a new
+    // alias set to retain the implementation invariant that all memory
+    // locations with the same pointer value are in the same alias set.
+    // (This is only known to occur for undef pointer values, which AA treats as
+    // noalias.)
+    AS = MapEntry->getForwardedTarget(*this);
+    AS->Alias = AliasSet::SetMayAlias;
----------------
brunodf-snps wrote:

OK. But then I think it is better to fully build in that the we assume a must-alias result for the existing alias set with a memory location using the same pointer value, and use this in `mergeAliasSetsForPointer`. I've pushed an updated version, see new commits b7fbb33320a9 and ad4fa84fd279. (This is maybe what you were after with your earlier remark about not calling `mergeAliasSetsForPointer` when there is a MapEntry: while I think we must call the method and do merging, we can save some alias queries there.)

https://github.com/llvm/llvm-project/pull/65731


More information about the llvm-commits mailing list