[PATCH] D86360: Add new hidden option -print-changed which only reports changes to IR

[Jamie Schmeiser via Phabricator via llvm-commits]

jamieschmeiser updated this revision to Diff 287456.
jamieschmeiser added a comment.

Always stack an entry on the stack before each pass in change reporters.

There was an inconsistency when handling invalidated passes in that they
always popped the stack even if the pass was filtered out or if it was for a
function that was filtered out.  However, for invalidated passes, no IR
is given in the callback from the pass manager so it is difficult to tell
if an invalidated pass was for a filtered function (which would not have
a stack entry previously).  Only the banner is printed for filtered,
invalidated or ignored passes so just print the banner message that the pass
is invalidated rather than being more specific and pop the stack (which will
always now have an entry).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86360/new/

https://reviews.llvm.org/D86360

Files:
  llvm/lib/Passes/StandardInstrumentations.cpp


Index: llvm/lib/Passes/StandardInstrumentations.cpp
===================================================================
--- llvm/lib/Passes/StandardInstrumentations.cpp
+++ llvm/lib/Passes/StandardInstrumentations.cpp
@@ -241,7 +241,13 @@
   }
 
   // Determine if this pass/IR is interesting and if so, save the IR
+  // otherwise it is left on the stack without data
   void saveIRBeforePass(Any IR, StringRef PassID) {
+    // Always need to place something on the stack because invalidated passes
+    // are not given the IR so it cannot be determined whether the pass was for
+    // something that was filtered out.
+    BeforeStack.emplace_back();
+
     if (!isInteresting(IR, PassID))
       return;
     // Is this the initial IR?
@@ -251,12 +257,12 @@
     }
 
     // Save the IR representation on the stack.
-    BeforeStack.emplace_back();
     auto &Data = BeforeStack.back();
     GenerateIRRepresentation(IR, PassID, Data);
   }
   // Compare the IR from before the pass after the pass.
   void handleIRAfterPass(Any IR, StringRef PassID) {
+    assert(!BeforeStack.empty() && "Unexpected empty stack encountered.");
     std::string Name;
 
     // unwrapModule has inconsistent handling of names for function IRs.
@@ -270,36 +276,33 @@
     if (Name == "")
       Name = " (module)";
 
-    if (isIgnored(PassID)) {
+    if (isIgnored(PassID))
       HandleIgnored(PassID, Name);
-      return;
-    }
-
-    if (!isInteresting(IR, PassID)) {
+    else if (!isInteresting(IR, PassID))
       HandleFiltered(PassID, Name);
-      return;
+    else {
+      // Get the before rep from the stack
+      T &Before = BeforeStack.back();
+      // Create the after rep
+      T After;
+      GenerateIRRepresentation(IR, PassID, After);
+
+      // was there a change in IR?
+      if (Same(Before, After))
+        OmitAfter(PassID, Name);
+      else
+        HandleAfter(Name, PassID, Before, After, IR);
     }
-
-    assert(!BeforeStack.empty() && "Unexpected empty stack for printing.");
-
-    // Get the before rep from the stack
-    T &Before = BeforeStack.back();
-    // Create the after rep
-    T After;
-    GenerateIRRepresentation(IR, PassID, After);
-
-    // was there a change in IR?
-    if (Same(Before, After))
-      OmitAfter(PassID, Name);
-    else
-      HandleAfter(Name, PassID, Before, After, IR);
-
-    // Now remove the before IR as it is no longer needed.
     BeforeStack.pop_back();
   }
   // Handle the situation where a pass is invalidated.
   void handleInvalidatedPass(StringRef PassID) {
-    assert(!BeforeStack.empty() && "Unexpected empty stack for printing.");
+    assert(!BeforeStack.empty() && "Unexpected empty stack encountered.");
+
+    // Always flag it as invalidated as we cannot determine when
+    // a pass for a filtered function is invalidated since we do not
+    // get the IR in the callback.  Also, the output is just alternate
+    // forms of the banner anyway.
     HandleInvalidated(PassID);
     BeforeStack.pop_back();
   }
@@ -458,7 +461,10 @@
 
 void registerChangePrinters(PassInstrumentationCallbacks &PIC) {
   if (PrintChanged) {
+    // Must be static as lifespan is across multiple callbacks.
+    // N.B. this may cause problems if the compiler becomes multi-threaded.
     static IRChangePrinter ICP(dbgs());
+
     PIC.registerBeforePassCallback([](StringRef P, Any IR) {
       ICP.saveIRBeforePass(IR, P);
       return true;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D86360.287456.patch
Type: text/x-patch
Size: 3425 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200824/3a10ee99/attachment.bin>