[LICM/AST] Check if the AliasAny set is removed from the tracker.

[asbirlea]

Summary

Resolves PR38513.
Credit to @bjope for debugging this.

[uabelho]

I've no idea if the fix itself makes sense but at least it does seem to fix my reproducer for PR38513 and I didn't see any problems with the patch during the (limited) testing I've done.

Thanks!

test/Transforms/LICM/pr38513.ll

4

Add a few CHECKs here perhaps?
Right now FileCheck complains there are none:

error: no check strings found with prefix 'CHECK:'

[bjope]

lib/Analysis/AliasSetTracker.cpp

705

Nit: Wouldn't mind if we added a printout here to indicate when we have the AliasAny set (as that was helpful when debugging). I used something like this:

if (AliasAnyAS)
  OS << "Saturated!\n";

test/Transforms/LICM/pr38513.ll

2

It is recommended to use redirect for the input to opt (https://llvm.org/docs/TestingGuide.html#fragile-tests):

; RUN: opt < %s -enable-mssa-loop-dependency=false -disable-basicaa -alias-set-saturation-threshold=2 -licm -S | FileCheck %s

4

Yes, CHECK:s are needed. And then "REQUIRES: asserts" can be removed.

[asbirlea]

Address comments.

test/Transforms/LICM/pr38513.ll

4

The test hits an assertion. It won't trigger without asserts.

[sanjoy.google]

lgtm

lib/Analysis/AliasSetTracker.cpp

void AliasSetTracker::removeAliasSet(AliasSet *AS) {
   if (AliasSet *Fwd = AS->Forward) {
     Fwd->dropRef(*this);
     AS->Forward = nullptr;
   } else // Update TotalMayAliasSetSize only if not forwarding.
       if (AS->Alias == AliasSet::SetMayAlias)
         TotalMayAliasSetSize -= AS->size();
 
   AliasSets.erase(AS);
+  // If we've removed the saturated alias set, set saturated marker back to
+  // nullptr and ensure this tracker is empty.
+  if (AS == AliasAnyAS) {
+    AliasAnyAS = nullptr;
+    assert(AliasSets.empty() && "Tracker not empty");
+  }
 }
 
 void AliasSet::removeFromTracker(AliasSetTracker &AST) {
   assert(RefCount == 0 && "Cannot remove non-dead alias set from tracker!");
   AST.removeAliasSet(this);
 }
 
 void AliasSet::addPointer(AliasSetTracker &AST, PointerRec &Entry,
for (unsigned i = 0, e = UnknownInsts.size(); i != e; ++i) {
           I->print(OS);
       }
     }
   }
   OS << "\n";
 }
 
 void AliasSetTracker::print(raw_ostream &OS) const {
-  OS << "Alias Set Tracker: " << AliasSets.size() << " alias sets for "
-     << PointerMap.size() << " pointer values.\n";
+  OS << "Alias Set Tracker: " << AliasSets.size();
+  if (AliasAnyAS)
+    OS << " (Saturated)";
+  OS << " alias sets for " << PointerMap.size() << " pointer values.\n";
   for (const AliasSet &AS : *this)
     AS.print(OS);
   OS << "\n";
 }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 LLVM_DUMP_METHOD void AliasSet::dump() const { print(dbgs()); }
 LLVM_DUMP_METHOD void AliasSetTracker::dump() const { print(dbgs()); }
 #endif
 
 //===----------------------------------------------------------------------===//

test/Transforms/LICM/pr38513.ll

+; RUN: opt -enable-mssa-loop-dependency=false -disable-basicaa -alias-set-saturation-threshold=2 -licm -S < %s | FileCheck %s
+; REQUIRES: asserts
+
+; CHECK-LABEL: @f1()
+define void @f1() {
+  %lc1.10 = alloca [3 x i16]
+  br label %bb1
+
+bb1:                                              ; preds = %bb6, %0
+  store i16 undef, i16* undef
+  br label %bb2
+
+bb2:                                              ; preds = %bb8, %bb1
+  %_tmp18.fca.0.gep = getelementptr inbounds [3 x i16], [3 x i16]* %lc1.10, i32 0, i32 0
+  %_tmp18.fca.0.load = load i16, i16* %_tmp18.fca.0.gep
+  %_tmp18.fca.1.gep = getelementptr inbounds [3 x i16], [3 x i16]* %lc1.10, i32 0, i32 1
+  %_tmp18.fca.1.load = load i16, i16* %_tmp18.fca.1.gep
+  %_tmp18.fca.2.gep = getelementptr inbounds [3 x i16], [3 x i16]* %lc1.10, i32 0, i32 2
+  %_tmp18.fca.2.load = load i16, i16* %_tmp18.fca.2.gep
+  br label %bb8
+
+bb8:                                              ; preds = %bb2
+  br i1 undef, label %bb2, label %bb6
+
+bb6:                                              ; preds = %bb8
+  br label %bb1
+}