Skip analysis re-computation when no changes are reported

[serge-sans-paille]

Summary

This is a follow-up to https://reviews.llvm.org/D80707

[serge-sans-paille]

@nikic I'm curious about any binary/compilation time change?

[foad]

llvm/lib/Analysis/CallGraphSCCPass.cpp

492–493

This (and line 473) should probably test a "LocalChanged" flag which is ORed into Changed?

llvm/lib/Analysis/RegionPass.cpp

138–139

Use a LocalChanged flag here and line 101?

[serge-sans-paille]

Take review into account.

[foad]

Looks good, but do we also need an equivalent of D80916 to check that passes are reporting "changed" correctly?

[nikic]

Compile-time changes: https://llvm-compile-time-tracker.com/compare.php?from=b9496efbb9ebbf7ec76411fac4e12ff0dc7a8e06&to=4ef4b3b3cc00344aa6e3ff8691e484f1ef7bafba&stat=instructions There is a positive impact, but it's very small. No text size changes.

[serge-sans-paille]

In D86442#2235449, @foad wrote:

Looks good, but do we also need an equivalent of D80916 to check that passes are reporting "changed" correctly?

I've got this patch locally, it raises three errors. I'll submit the patch, the passes update once fixed and when everything lands we can finally consider landing this patch :-)

In D86442#2235498, @nikic wrote:

Compile-time changes: https://llvm-compile-time-tracker.com/compare.php?from=b9496efbb9ebbf7ec76411fac4e12ff0dc7a8e06&to=4ef4b3b3cc00344aa6e3ff8691e484f1ef7bafba&stat=instructions There is a positive impact, but it's very small. No text size changes.

Thanks for the positive feedback!

[serge-sans-paille]

Rebase

[nikic]

LGTM

[nikic]

Hrm, the committed patches showed text size changes at O3, so probably something isn't right here: https://llvm-compile-time-tracker.com/compare.php?from=5f1cad4d296a20025f0b3ea30d82b278ddb8779b&to=2296182181521c0b1803a0cd10b098fcfdab1c92&stat=size-text

Relevant files are CMakeFiles/SPASS.dir/renaming.c.o, CMakeFiles/7zip-benchmark.dir/CPP/7zip/Archive/Common/CoderMixer2MT.cpp.o

[nikic]

Okay, this is not related to this patch after all. I see a couple of text size flips in consecutive commits, so some (other) fairly recent change must have introduced compiler non-determinism :(

llvm/lib/Analysis/CallGraphSCCPass.cpp

if (!LocalChanged && (RefHash != StructuralHash(CG.getModule()))) {
       llvm_unreachable("Pass modifies its input and doesn't report it");
     }
 #endif
     if (LocalChanged)
       dumpPassInfo(P, MODIFICATION_MSG, ON_CG_MSG, "");
     dumpPreservedSet(P);
 
     verifyPreservedAnalysis(P);
+    if (LocalChanged)
       removeNotPreservedAnalysis(P);
     recordAvailableAnalysis(P);
     removeDeadPasses(P, "", ON_CG_MSG);
   }
 
   // If the callgraph was left out of date (because the last pass run was a
   // functionpass), refresh it before we move on to the next SCC.
   if (!CallGraphUpToDate)
     DevirtualizedCall |= RefreshCallGraph(CurSCC, CG, false);

llvm/lib/Analysis/LoopPass.cpp

 #endif
 
         // Then call the regular verifyAnalysis functions.
         verifyPreservedAnalysis(P);
 
         F.getContext().yield();
       }
 
+      if (LocalChanged)
         removeNotPreservedAnalysis(P);
       recordAvailableAnalysis(P);
       removeDeadPasses(P,
                        CurrentLoopDeleted ? "<deleted>"
                                           : CurrentLoop->getHeader()->getName(),
                        ON_LOOP_MSG);
 
       if (CurrentLoopDeleted)
         // Do not run other passes on this loop.

llvm/lib/Analysis/RegionPass.cpp

#endif
           TimeRegion PassTimer(getPassTimer(P));
           CurrentRegion->verifyRegion();
         }
 
         // Then call the regular verifyAnalysis functions.
         verifyPreservedAnalysis(P);
       }
 
+      if (LocalChanged)
         removeNotPreservedAnalysis(P);
       recordAvailableAnalysis(P);
       removeDeadPasses(P,
                        (!isPassDebuggingExecutionsOrMore() || skipThisRegion) ?
                        "<deleted>" :  CurrentRegion->getNameStr(),
                        ON_REGION_MSG);
 
       if (skipThisRegion)
         // Do not run other passes on this region.