[clang][dataflow] Identify post-visit state changes in the HTML logger.

[martinboehme]

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:

)

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-analysis

Changes

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 (see attached screenshot for an example).

---

Full diff: https://github.com/llvm/llvm-project/pull/66746.diff

5 Files Affected:

• (modified) clang/include/clang/Analysis/FlowSensitive/Logger.h (+3-1)
• (modified) clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp (+30-11)
• (modified) clang/lib/Analysis/FlowSensitive/HTMLLogger.html (+14-5)
• (modified) clang/lib/Analysis/FlowSensitive/Logger.cpp (+7-3)
• (modified) clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp (+1-1)

diff --git a/clang/include/clang/Analysis/FlowSensitive/Logger.h b/clang/include/clang/Analysis/FlowSensitive/Logger.h
index 6836488003a97de..f4bd39f6ed49ef7 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Logger.h
+++ b/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.
diff --git a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
index a5f64021eb6ba4b..47915958750d1f4 100644
--- a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
+++ b/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;
diff --git a/clang/lib/Analysis/FlowSensitive/HTMLLogger.html b/clang/lib/Analysis/FlowSensitive/HTMLLogger.html
index a60259a99cce066..87695623cb318b2 100644
--- a/clang/lib/Analysis/FlowSensitive/HTMLLogger.html
+++ b/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>
diff --git a/clang/lib/Analysis/FlowSensitive/Logger.cpp b/clang/lib/Analysis/FlowSensitive/Logger.cpp
index 28557638522e8f3..8c401df62e4459c 100644
--- a/clang/lib/Analysis/FlowSensitive/Logger.cpp
+++ b/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);
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 900aa97e9f6de64..01b397787430ac6 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/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;

[martinboehme]

Sorry, completely missed the fact that I had broken a test. Should be fixed now.

[sam-mccall]

Thanks, this looks nice!