HotColdSplitting: invalidate the AssumptionCache on split

[compnerd]

Summary

When a cold path is outlined, the value tracking in the assumption cache may be
invalidated due to the code motion. We would previously trip an assertion in
subsequent passes (but required the passes to happen in a single run as the
assumption cache is shared across the passes). Invalidating the cache ensures
that we get the correct information when needed with the legacy pass manager as
well.

[hiraditya]

LGTM, thanks for spotting+fixing this bug.

[hiraditya]

lib/Transforms/IPO/HotColdSplitting.cpp

665

LookupAC(F) may return a nullptr, so we might have to take care of that.

[lebedev.ri]

It's weird though. Nothing else calls AssumptionCache::clear().
*Why* it is being preserved? It seems there might be a bigger bug.

[hiraditya]

Assumptions shouldn't change by optimization, code movement etc AFAICT. As Assumption cache doesn't currently have APIs to accomodate the changes caused by function splitting, invalidating the cache seems reasonable to me. If another pass requests for assumptions, they will be re-scanned, include/llvm/Analysis/AssumptionCache.h:132

[compnerd]

r372667

lib/Transforms/IPO/HotColdSplitting.cpp

do {
                                              OutlinedFunctionID);
       if (Outlined) {
         ++OutlinedFunctionID;
         Changed = true;
       }
     } while (!Region.empty());
   }
 
+  // We need to explicitly clear the assumption cache since the value tracking
+  // may now be invalid as part of the function has changed.
+  if (Changed)
+    LookupAC(F)->clear();
   return Changed;
 }
 
 bool HotColdSplitting::run(Module &M) {
   bool Changed = false;
   bool HasProfileSummary = (M.getProfileSummary(/* IsCS */ false) != nullptr);
   for (auto It = M.begin(), End = M.end(); It != End; ++It) {
     Function &F = *It;

test/Transforms/HotColdSplit/assumption-cache-invalidation.ll

+; RUN: opt -instcombine -hotcoldsplit -instsimplify %s -o /dev/null
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64"
+
+%a = type { i64, i64 }
+%b = type { i64 }
+
+define void @f() {
+entry:
+  %0 = getelementptr inbounds %a, %a* null, i64 0, i32 1
+  br label %label
+
+label:
+  %1 = bitcast i64* %0 to %b**
+  %2 = load %b*, %b** %1, align 8
+  %3 = getelementptr inbounds %b, %b* %2, i64 undef, i32 0
+  %4 = load i64, i64* %3, align 8
+  %5 = icmp ugt i64 %4, 1
+  br i1 %5, label %if.then, label %if.else
+
+if.then:
+  unreachable
+
+if.else:
+  call void @g(i8* undef)
+  %6 = load i64, i64* undef, align 8
+  %7 = and i64 %6, -16
+  %8 = inttoptr i64 %7 to i8*
+  %9 = icmp eq i64 %4, 0
+  call void @llvm.assume(i1 %9)
+  unreachable
+}
+
+declare void @g(i8*)
+
+declare void @llvm.assume(i1)
+