[clang][dataflow] Identify post-visit state changes in the HTML logge…

…r. (#66746)

Previously, post-visit state changes were indistinguishable from
ordinary
iterations, which could give a confusing picture of how many iterations
a block
needs to converge.

Now, post-visit state changes are marked with "post-visit" instead of an
iteration number:


![screenshot](https://github.com/llvm/llvm-project/assets/29098113/5e9553d6-dfaa-45d3-8ea4-e623a14ee4c5))

clang/include/clang/Analysis/FlowSensitive/Logger.h

@@ -50,7 +50,9 @@ class Logger {
   /// Called when we start (re-)processing a block in the CFG.
   /// The target program point is the entry to the specified block.
   /// Calls to log() describe transferBranch(), join() etc.
-  virtual void enterBlock(const CFGBlock &) {}
+  /// `PostVisit` specifies whether we're processing the block for the
+  /// post-visit callback.
+  virtual void enterBlock(const CFGBlock &, bool PostVisit) {}
   /// Called when we start processing an element in the current CFG block.
   /// The target program point is after the specified element.
   /// Calls to log() describe the transfer() function.

clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp

@@ -146,15 +146,21 @@ class ModelDumper {
 };
 
 class HTMLLogger : public Logger {
+  struct Iteration {
+    const CFGBlock *Block;
+    unsigned Iter;
+    bool PostVisit;
+  };
+
   StreamFactory Streams;
   std::unique_ptr<llvm::raw_ostream> OS;
   std::optional<llvm::json::OStream> JOS;
 
   const ControlFlowContext *CFG;
   // Timeline of iterations of CFG block visitation.
-  std::vector<std::pair<const CFGBlock *, unsigned>> Iters;
+  std::vector<Iteration> Iters;
   // Number of times each CFG block has been seen.
-  llvm::DenseMap<const CFGBlock *, unsigned> BlockIters;
+  llvm::DenseMap<const CFGBlock *, llvm::SmallVector<Iteration>> BlockIters;
   // The messages logged in the current context but not yet written.
   std::string ContextLogs;
   // The number of elements we have visited within the current CFG block.
@@ -198,8 +204,9 @@ class HTMLLogger : public Logger {
     JOS->attributeArray("timeline", [&] {
       for (const auto &E : Iters) {
         JOS->object([&] {
-          JOS->attribute("block", blockID(E.first->getBlockID()));
-          JOS->attribute("iter", E.second);
+          JOS->attribute("block", blockID(E.Block->getBlockID()));
+          JOS->attribute("iter", E.Iter);
+          JOS->attribute("post_visit", E.PostVisit);
         });
       }
     });
@@ -214,8 +221,11 @@ class HTMLLogger : public Logger {
     *OS << llvm::StringRef(HTMLLogger_html).split("<?INJECT?>").second;
   }
 
-  void enterBlock(const CFGBlock &B) override {
-    Iters.emplace_back(&B, ++BlockIters[&B]);
+  void enterBlock(const CFGBlock &B, bool PostVisit) override {
+    llvm::SmallVector<Iteration> &BIter = BlockIters[&B];
+    unsigned IterNum = BIter.size() + 1;
+    BIter.push_back({&B, IterNum, PostVisit});
+    Iters.push_back({&B, IterNum, PostVisit});
     ElementIndex = 0;
   }
   void enterElement(const CFGElement &E) override {
@@ -243,17 +253,19 @@ class HTMLLogger : public Logger {
   //  - meaningful names for values
   //  - which boolean values are implied true/false by the flow condition
   void recordState(TypeErasedDataflowAnalysisState &State) override {
-    unsigned Block = Iters.back().first->getBlockID();
-    unsigned Iter = Iters.back().second;
+    unsigned Block = Iters.back().Block->getBlockID();
+    unsigned Iter = Iters.back().Iter;
+    bool PostVisit = Iters.back().PostVisit;
     JOS->attributeObject(elementIterID(Block, Iter, ElementIndex), [&] {
       JOS->attribute("block", blockID(Block));
       JOS->attribute("iter", Iter);
+      JOS->attribute("post_visit", PostVisit);
       JOS->attribute("element", ElementIndex);
 
       // If this state immediately follows an Expr, show its built-in model.
       if (ElementIndex > 0) {
         auto S =
-            Iters.back().first->Elements[ElementIndex - 1].getAs<CFGStmt>();
+            Iters.back().Block->Elements[ElementIndex - 1].getAs<CFGStmt>();
         if (const Expr *E = S ? llvm::dyn_cast<Expr>(S->getStmt()) : nullptr) {
           if (E->isPRValue()) {
             if (auto *V = State.Env.getValue(*E))
@@ -289,9 +301,16 @@ class HTMLLogger : public Logger {
   // Write the CFG block details.
   // Currently this is just the list of elements in execution order.
   // FIXME: an AST dump would be a useful view, too.
-  void writeBlock(const CFGBlock &B, unsigned Iters) {
+  void writeBlock(const CFGBlock &B, llvm::ArrayRef<Iteration> ItersForB) {
     JOS->attributeObject(blockID(B.getBlockID()), [&] {
-      JOS->attribute("iters", Iters);
+      JOS->attributeArray("iters", [&] {
+        for (const auto &Iter : ItersForB) {
+          JOS->object([&] {
+            JOS->attribute("iter", Iter.Iter);
+            JOS->attribute("post_visit", Iter.PostVisit);
+          });
+        }
+      });
       JOS->attributeArray("elements", [&] {
         for (const auto &Elt : B.Elements) {
           std::string Dump;

clang/lib/Analysis/FlowSensitive/HTMLLogger.html

@@ -41,7 +41,11 @@
 <section id="timeline" data-selection="">
 <header>Timeline</header>
 <template data-for="entry in timeline">
-  <div id="{{entry.block}}:{{entry.iter}}" data-bb="{{entry.block}}" class="entry">{{entry.block}} ({{entry.iter}})</div>
+  <div id="{{entry.block}}:{{entry.iter}}" data-bb="{{entry.block}}" class="entry">
+    {{entry.block}}
+    <template data-if="entry.post_visit">(post-visit)</template>
+    <template data-if="!entry.post_visit">({{entry.iter}})</template>
+  </div>
 </template>
 </section>
 
@@ -54,8 +58,11 @@
 <section id="block" data-selection="bb">
 <header><template>Block {{selection.bb}}</template></header>
 <div id="iterations">
-  <template data-for="i in Array(cfg[selection.bb].iters).keys()">
-    <a class="chooser {{selection.bb}}:{{i+1}}" data-iter="{{selection.bb}}:{{i+1}}">Iteration {{i+1}}</a>
+  <template data-for="iter in cfg[selection.bb].iters">
+    <a class="chooser {{selection.bb}}:{{iter.iter}}" data-iter="{{selection.bb}}:{{iter.iter}}">
+      <template data-if="iter.post_visit">Post-visit</template>
+      <template data-if="!iter.post_visit">Iteration {{iter.iter}}</template>
+    </a>
   </template>
 </div>
 <table id="bb-elements">
@@ -77,8 +84,10 @@
 <section id="element" data-selection="iter,elt">
 <template data-let="state = states[selection.iter + '_' + selection.elt]">
 <header>
-  <template data-if="state.element == 0">{{state.block}} (iteration {{state.iter}}) initial state</template>
-  <template data-if="state.element != 0">Element {{selection.elt}} (iteration {{state.iter}})</template>
+  <template data-if="state.element == 0">{{state.block}} initial state</template>
+  <template data-if="state.element != 0">Element {{selection.elt}}</template>
+  <template data-if="state.post_visit"> (post-visit)</template>
+  <template data-if="!state.post_visit"> (iteration {{state.iter}})</template>
 </header>
 <template data-if="state.value" data-let="v = state.value">
   <h2>Value</h2>

clang/lib/Analysis/FlowSensitive/Logger.cpp

@@ -57,12 +57,16 @@ struct TextualLogger final : Logger {
     llvm::errs() << "=== Finished analysis: " << Blocks << " blocks in "
                  << Steps << " total steps ===\n";
   }
-  virtual void enterBlock(const CFGBlock &Block) override {
+  virtual void enterBlock(const CFGBlock &Block, bool PostVisit) override {
     unsigned Count = ++VisitCount[&Block];
     {
       llvm::WithColor Header(OS, llvm::raw_ostream::Colors::RED, /*Bold=*/true);
-      OS << "=== Entering block B" << Block.getBlockID() << " (iteration "
-         << Count << ") ===\n";
+      OS << "=== Entering block B" << Block.getBlockID();
+      if (PostVisit)
+        OS << " (post-visit)";
+      else
+        OS << " (iteration " << Count << ")";
+      OS << " ===\n";
     }
     Block.print(OS, CurrentCFG, CurrentAnalysis->getASTContext().getLangOpts(),
                 ShowColors);

clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

@@ -469,7 +469,7 @@ transferCFGBlock(const CFGBlock &Block, AnalysisContext &AC,
                  std::function<void(const CFGElement &,
                                     const TypeErasedDataflowAnalysisState &)>
                      PostVisitCFG = nullptr) {
-  AC.Log.enterBlock(Block);
+  AC.Log.enterBlock(Block, PostVisitCFG != nullptr);
   auto State = computeBlockInputState(Block, AC);
   AC.Log.recordState(State);
   int ElementIdx = 1;

clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp

@@ -64,8 +64,9 @@ class TestLogger : public Logger {
   }
   void endAnalysis() override { logText("\nendAnalysis()"); }
 
-  void enterBlock(const CFGBlock &B) override {
-    OS << "\nenterBlock(" << B.BlockID << ")\n";
+  void enterBlock(const CFGBlock &B, bool PostVisit) override {
+    OS << "\nenterBlock(" << B.BlockID << ", " << (PostVisit ? "true" : "false")
+       << ")\n";
   }
   void enterElement(const CFGElement &E) override {
     // we don't want the trailing \n
@@ -114,7 +115,7 @@ TEST(LoggerTest, Sequence) {
 
   EXPECT_EQ(Log, R"(beginAnalysis()
 
-enterBlock(4)
+enterBlock(4, false)
 recordState(Elements=0, Branches=0, Joins=0)
 enterElement(b)
 transfer()
@@ -123,21 +124,21 @@ enterElement(b (ImplicitCastExpr, LValueToRValue, _Bool))
 transfer()
 recordState(Elements=2, Branches=0, Joins=0)
 
-enterBlock(3)
+enterBlock(3, false)
 transferBranch(0)
 recordState(Elements=2, Branches=1, Joins=0)
 enterElement(q)
 transfer()
 recordState(Elements=3, Branches=1, Joins=0)
 
-enterBlock(2)
+enterBlock(2, false)
 transferBranch(1)
 recordState(Elements=2, Branches=1, Joins=0)
 enterElement(p)
 transfer()
 recordState(Elements=3, Branches=1, Joins=0)
 
-enterBlock(1)
+enterBlock(1, false)
 recordState(Elements=6, Branches=2, Joins=1)
 enterElement(b ? p : q)
 transfer()
@@ -149,7 +150,7 @@ enterElement(return b ? p : q;)
 transfer()
 recordState(Elements=9, Branches=2, Joins=1)
 
-enterBlock(0)
+enterBlock(0, false)
 recordState(Elements=9, Branches=2, Joins=1)
 
 endAnalysis()