[llvm] bd5befb - Revert "Reland A new option -print-on-crash that prints the IR as it was upon entering the last pass when there is a crash."

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 7 12:58:06 PDT 2021


JFYI, when reverting, we encourage explaining why in the commit 
message.   Links to bugs, failing builders, etc... are all encouraged.

Philip

On 10/7/21 12:24 PM, Jamie Schmeiser via llvm-commits wrote:
> Author: Jamie Schmeiser
> Date: 2021-10-07T15:23:48-04:00
> New Revision: bd5befb55087199ee1c0c3e344847cd06a2ca839
>
> URL: https://github.com/llvm/llvm-project/commit/bd5befb55087199ee1c0c3e344847cd06a2ca839
> DIFF: https://github.com/llvm/llvm-project/commit/bd5befb55087199ee1c0c3e344847cd06a2ca839.diff
>
> LOG: Revert "Reland A new option -print-on-crash that prints the IR as it was upon entering the last pass when there is a crash."
>
> This reverts commit 13d1592716a65444314f501109ec9ca344ef1f87.
>
> Added:
>      
>
> Modified:
>      llvm/include/llvm/Passes/StandardInstrumentations.h
>      llvm/lib/Passes/PassBuilder.cpp
>      llvm/lib/Passes/PassRegistry.def
>      llvm/lib/Passes/StandardInstrumentations.cpp
>
> Removed:
>      llvm/test/Other/print-on-crash.ll
>
>
> ################################################################################
> diff  --git a/llvm/include/llvm/Passes/StandardInstrumentations.h b/llvm/include/llvm/Passes/StandardInstrumentations.h
> index 6729ae5d484f1..2f573585e766d 100644
> --- a/llvm/include/llvm/Passes/StandardInstrumentations.h
> +++ b/llvm/include/llvm/Passes/StandardInstrumentations.h
> @@ -187,6 +187,17 @@ template <typename IRUnitT> class ChangeReporter {
>     // Register required callbacks.
>     void registerRequiredCallbacks(PassInstrumentationCallbacks &PIC);
>   
> +  // Return true when this is a defined function for which printing
> +  // of changes is desired.
> +  bool isInterestingFunction(const Function &F);
> +
> +  // Return true when this is a pass for which printing of changes is desired.
> +  bool isInterestingPass(StringRef PassID);
> +
> +  // Return true when this is a pass on IR for which printing
> +  // of changes is desired.
> +  bool isInteresting(Any IR, StringRef PassID);
> +
>     // Called on the first IR processed.
>     virtual void handleInitialIR(Any IR) = 0;
>     // Called before and after a pass to get the representation of the IR.
> @@ -398,25 +409,6 @@ class VerifyInstrumentation {
>     void registerCallbacks(PassInstrumentationCallbacks &PIC);
>   };
>   
> -// Print IR on crash.
> -class PrintCrashIRInstrumentation {
> -public:
> -  PrintCrashIRInstrumentation()
> -      : SavedIR("*** Dump of IR Before Last Pass Unknown ***") {}
> -  ~PrintCrashIRInstrumentation();
> -  void registerCallbacks(PassInstrumentationCallbacks &PIC);
> -  void reportCrashIR();
> -
> -protected:
> -  std::string SavedIR;
> -
> -private:
> -  // The crash reporter that will report on a crash.
> -  static PrintCrashIRInstrumentation *CrashReporter;
> -  // Crash handler registered when print-on-crash is specified.
> -  static void SignalHandler(void *);
> -};
> -
>   /// This class provides an interface to register all the standard pass
>   /// instrumentations and manages their state (if any).
>   class StandardInstrumentations {
> @@ -429,7 +421,6 @@ class StandardInstrumentations {
>     IRChangedPrinter PrintChangedIR;
>     PseudoProbeVerifier PseudoProbeVerification;
>     InLineChangePrinter PrintChangedDiff;
> -  PrintCrashIRInstrumentation PrintCrashIR;
>     VerifyInstrumentation Verify;
>   
>     bool VerifyEach;
>
> diff  --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
> index 642453c143409..c3e7bc23ad23c 100644
> --- a/llvm/lib/Passes/PassBuilder.cpp
> +++ b/llvm/lib/Passes/PassBuilder.cpp
> @@ -357,13 +357,6 @@ bool shouldPopulateClassToPassNames() {
>            !printAfterPasses().empty();
>   }
>   
> -// A pass for testing -print-on-crash.
> -// DO NOT USE THIS EXCEPT FOR TESTING!
> -class TriggerCrashPass : public PassInfoMixin<TriggerCrashPass> {
> -public:
> -  PreservedAnalyses run(Module &, ModuleAnalysisManager &) { assert(false); }
> -};
> -
>   } // namespace
>   
>   PassBuilder::PassBuilder(TargetMachine *TM, PipelineTuningOptions PTO,
>
> diff  --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
> index 9f8c2cb11e04b..655c472878a00 100644
> --- a/llvm/lib/Passes/PassRegistry.def
> +++ b/llvm/lib/Passes/PassRegistry.def
> @@ -106,7 +106,6 @@ MODULE_PASS("strip-debug-declare", StripDebugDeclarePass())
>   MODULE_PASS("strip-nondebug", StripNonDebugSymbolsPass())
>   MODULE_PASS("strip-nonlinetable-debuginfo", StripNonLineTableDebugInfoPass())
>   MODULE_PASS("synthetic-counts-propagation", SyntheticCountsPropagation())
> -MODULE_PASS("trigger-crash", TriggerCrashPass())
>   MODULE_PASS("verify", VerifierPass())
>   MODULE_PASS("wholeprogramdevirt", WholeProgramDevirtPass())
>   MODULE_PASS("dfsan", DataFlowSanitizerPass())
>
> diff  --git a/llvm/lib/Passes/StandardInstrumentations.cpp b/llvm/lib/Passes/StandardInstrumentations.cpp
> index 9c374b8c5092a..fbe4f25242c2d 100644
> --- a/llvm/lib/Passes/StandardInstrumentations.cpp
> +++ b/llvm/lib/Passes/StandardInstrumentations.cpp
> @@ -27,12 +27,10 @@
>   #include "llvm/IR/PrintPasses.h"
>   #include "llvm/IR/Verifier.h"
>   #include "llvm/Support/CommandLine.h"
> -#include "llvm/Support/CrashRecoveryContext.h"
>   #include "llvm/Support/Debug.h"
>   #include "llvm/Support/FormatVariadic.h"
>   #include "llvm/Support/MemoryBuffer.h"
>   #include "llvm/Support/Program.h"
> -#include "llvm/Support/Signals.h"
>   #include "llvm/Support/raw_ostream.h"
>   #include <unordered_set>
>   #include <vector>
> @@ -121,12 +119,6 @@ static cl::opt<std::string>
>       DiffBinary("print-changed-
> diff -path", cl::Hidden, cl::init("
> diff "),
>                  cl::desc("system
> diff  used by change reporters"));
>   
> -// An option to print the IR that was being processed when a pass crashes.
> -static cl::opt<bool>
> -    PrintCrashIR("print-on-crash",
> -                 cl::desc("Print the last form of the IR before crash"),
> -                 cl::init(false), cl::Hidden);
> -
>   namespace {
>   
>   // Perform a system based
> diff  between \p Before and \p After, using
> @@ -376,11 +368,20 @@ bool isIgnored(StringRef PassID) {
>                           "DevirtSCCRepeatedPass", "ModuleInlinerWrapperPass"});
>   }
>   
> -bool isInterestingFunction(const Function &F) {
> +} // namespace
> +
> +template <typename IRUnitT>
> +ChangeReporter<IRUnitT>::~ChangeReporter<IRUnitT>() {
> +  assert(BeforeStack.empty() && "Problem with Change Printer stack.");
> +}
> +
> +template <typename IRUnitT>
> +bool ChangeReporter<IRUnitT>::isInterestingFunction(const Function &F) {
>     return isFunctionInPrintList(F.getName());
>   }
>   
> -bool isInterestingPass(StringRef PassID) {
> +template <typename IRUnitT>
> +bool ChangeReporter<IRUnitT>::isInterestingPass(StringRef PassID) {
>     if (isIgnored(PassID))
>       return false;
>   
> @@ -391,7 +392,8 @@ bool isInterestingPass(StringRef PassID) {
>   
>   // Return true when this is a pass on IR for which printing
>   // of changes is desired.
> -bool isInteresting(Any IR, StringRef PassID) {
> +template <typename IRUnitT>
> +bool ChangeReporter<IRUnitT>::isInteresting(Any IR, StringRef PassID) {
>     if (!isInterestingPass(PassID))
>       return false;
>     if (any_isa<const Function *>(IR))
> @@ -399,13 +401,6 @@ bool isInteresting(Any IR, StringRef PassID) {
>     return true;
>   }
>   
> -} // namespace
> -
> -template <typename IRUnitT>
> -ChangeReporter<IRUnitT>::~ChangeReporter<IRUnitT>() {
> -  assert(BeforeStack.empty() && "Problem with Change Printer stack.");
> -}
> -
>   template <typename IRUnitT>
>   void ChangeReporter<IRUnitT>::saveIRBeforePass(Any IR, StringRef PassID) {
>     // Always need to place something on the stack because invalidated passes
> @@ -1228,51 +1223,6 @@ StandardInstrumentations::StandardInstrumentations(
>                 PrintChanged == ChangePrinter::PrintChangedColourDiffQuiet),
>         Verify(DebugLogging), VerifyEach(VerifyEach) {}
>   
> -PrintCrashIRInstrumentation *PrintCrashIRInstrumentation::CrashReporter =
> -    nullptr;
> -
> -void PrintCrashIRInstrumentation::reportCrashIR() { dbgs() << SavedIR; }
> -
> -void PrintCrashIRInstrumentation::SignalHandler(void *) {
> -  // Called by signal handlers so do not lock here
> -  // Is the PrintCrashIRInstrumentation still alive?
> -  if (!CrashReporter)
> -    return;
> -
> -  assert(PrintCrashIR && "Did not expect to get here without option set.");
> -  CrashReporter->reportCrashIR();
> -}
> -
> -PrintCrashIRInstrumentation::~PrintCrashIRInstrumentation() {
> -  if (!CrashReporter)
> -    return;
> -
> -  assert(PrintCrashIR && "Did not expect to get here without option set.");
> -  CrashReporter = nullptr;
> -}
> -
> -void PrintCrashIRInstrumentation::registerCallbacks(
> -    PassInstrumentationCallbacks &PIC) {
> -  if (!PrintCrashIR || CrashReporter)
> -    return;
> -
> -  sys::AddSignalHandler(SignalHandler, nullptr);
> -  CrashReporter = this;
> -
> -  PIC.registerBeforeNonSkippedPassCallback([this](StringRef PassID, Any IR) {
> -    SavedIR.clear();
> -    raw_string_ostream OS(SavedIR);
> -    OS << formatv("*** Dump of {0}IR Before Last Pass {1}",
> -                  llvm::forcePrintModuleIR() ? "Module " : "", PassID);
> -    if (!isInteresting(IR, PassID)) {
> -      OS << " Filtered Out ***\n";
> -      return;
> -    }
> -    OS << " Started ***\n";
> -    unwrapAndPrint(OS, IR);
> -  });
> -}
> -
>   void StandardInstrumentations::registerCallbacks(
>       PassInstrumentationCallbacks &PIC, FunctionAnalysisManager *FAM) {
>     PrintIR.registerCallbacks(PIC);
> @@ -1287,7 +1237,6 @@ void StandardInstrumentations::registerCallbacks(
>     if (VerifyEach)
>       Verify.registerCallbacks(PIC);
>     PrintChangedDiff.registerCallbacks(PIC);
> -  PrintCrashIR.registerCallbacks(PIC);
>   }
>   
>   namespace llvm {
>
> diff  --git a/llvm/test/Other/print-on-crash.ll b/llvm/test/Other/print-on-crash.ll
> deleted file mode 100644
> index 7e8ec2dc09ce6..0000000000000
> --- a/llvm/test/Other/print-on-crash.ll
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -; A test that the hidden option -print-on-crash properly sets a signal handler
> -; which gets called when a pass crashes.  The trigger-crash pass asserts.
> -
> -; REQUIRES: asserts
> -
> -; RUN: not --crash opt -print-on-crash -passes=trigger-crash < %s 2>&1 | FileCheck %s --check-prefix=CHECK_SIMPLE
> -
> -; A test that the signal handler set by the  hidden option -print-on-crash
> -; is not called when no pass crashes.
> -
> -; RUN: opt -print-on-crash -passes="default<O2>" < %s 2>&1 | FileCheck %s --check-prefix=CHECK_NO_CRASH
> -
> -; RUN: not --crash opt -print-on-crash -print-module-scope -passes=trigger-crash < %s 2>&1 | FileCheck %s --check-prefix=CHECK_MODULE
> -
> -; RUN: not --crash opt -print-on-crash -print-module-scope -passes=trigger-crash -filter-passes="(anonymous namespace)::TriggerCrashPass" < %s 2>&1 | FileCheck %s --check-prefix=CHECK_MODULE
> -
> -; RUN: not --crash opt -print-on-crash -print-module-scope -passes=trigger-crash -filter-passes=blah < %s 2>&1 | FileCheck %s --check-prefix=CHECK_FILTERED
> -
> -; CHECK_SIMPLE: *** Dump of IR Before Last Pass {{.*}} Started ***
> -; CHECK_SIMPLE: @main
> -; CHECK_SIMPLE: entry:
> -; CHECK_NO_CRASH-NOT: *** Dump of IR
> -; CHECK_MODULE: *** Dump of Module IR Before Last Pass {{.*}} Started ***
> -; CHECK_MODULE: ; ModuleID = {{.*}}
> -; CHECK_FILTERED: *** Dump of Module IR Before Last Pass {{.*}} Filtered Out ***
> -
> -define i32 @main() {
> -entry:
> -  %retval = alloca i32, align 4
> -  store i32 0, i32* %retval, align 4
> -  ret i32 0
> -}
>
>
>          
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list