[llvm] f9235e4 - A new hidden option test-changed=exe that calls exe after each time IR changes

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 15 10:13:20 PST 2021


This looks like a really useful mechanism, but the naming here is 
somewhat confusing.

Could we rename this to something like "-exec-on-ir-change=executable"?

Philip

On 12/8/21 11:23 AM, Jamie Schmeiser via llvm-commits wrote:

> Author: Jamie Schmeiser
> Date: 2021-12-08T14:23:31-05:00
> New Revision: f9235e45fd1f5ca21f95105427184a6afd0f9d95
>
> URL: https://github.com/llvm/llvm-project/commit/f9235e45fd1f5ca21f95105427184a6afd0f9d95
> DIFF: https://github.com/llvm/llvm-project/commit/f9235e45fd1f5ca21f95105427184a6afd0f9d95.diff
>
> LOG: A new hidden option test-changed=exe that calls exe after each time IR changes
>
> Summary:
> A new option test-changed is defined that allows one to specify an
> exe that is called after each pass in the opt pipeline that changes the IR.
> The test-changed=exe option saves the IR in a temporary file and calls exe
> with the name of the file and the name of the pass that just changed it after
> each pass alters the IR. exe is also called with the initial IR. This
> can be used, for example, to determine which pass corrupts the IR by having
> exe as a script that calls llc and runs a test to see after which pass the
> results change. The print-changed filtering options are respected.
>
> Author: Jamie Schmeiser <schmeise at ca.ibm.com>
> Reviewed By: aeubanks (Arthur Eubanks)
> Differential Revision: https://reviews.llvm.org/D110776
>
> Added:
>      llvm/test/Other/test-changed-script.sh
>      llvm/test/Other/test-changed.ll
>
> Modified:
>      llvm/include/llvm/Passes/StandardInstrumentations.h
>      llvm/lib/Passes/StandardInstrumentations.cpp
>
> Removed:
>      
>
>
> ################################################################################
> diff  --git a/llvm/include/llvm/Passes/StandardInstrumentations.h b/llvm/include/llvm/Passes/StandardInstrumentations.h
> index 6cab4ce7d138..1446c5898ef9 100644
> --- a/llvm/include/llvm/Passes/StandardInstrumentations.h
> +++ b/llvm/include/llvm/Passes/StandardInstrumentations.h
> @@ -269,6 +269,32 @@ class IRChangedPrinter : public TextChangeReporter<std::string> {
>                      Any) override;
>   };
>   
> +class IRChangedTester : public IRChangedPrinter {
> +public:
> +  IRChangedTester() : IRChangedPrinter(true) {}
> +  ~IRChangedTester() override;
> +  void registerCallbacks(PassInstrumentationCallbacks &PIC);
> +
> +protected:
> +  void handleIR(const std::string &IR, StringRef PassID);
> +
> +  // Check initial IR
> +  void handleInitialIR(Any IR) override;
> +  // Do nothing.
> +  void omitAfter(StringRef PassID, std::string &Name) override;
> +  // Do nothing.
> +  void handleInvalidated(StringRef PassID) override;
> +  // Do nothing.
> +  void handleFiltered(StringRef PassID, std::string &Name) override;
> +  // Do nothing.
> +  void handleIgnored(StringRef PassID, std::string &Name) override;
> +
> +  // Call test as interesting IR has changed.
> +  void handleAfter(StringRef PassID, std::string &Name,
> +                   const std::string &Before, const std::string &After,
> +                   Any) override;
> +};
> +
>   // Information that needs to be saved for a basic block in order to compare
>   // before and after the pass to determine if it was changed by a pass.
>   template <typename T> class BlockDataT {
> @@ -504,6 +530,7 @@ class StandardInstrumentations {
>     PseudoProbeVerifier PseudoProbeVerification;
>     InLineChangePrinter PrintChangedDiff;
>     DotCfgChangeReporter WebsiteChangeReporter;
> +  IRChangedTester ChangeTester;
>     VerifyInstrumentation Verify;
>   
>     bool VerifyEach;
>
> diff  --git a/llvm/lib/Passes/StandardInstrumentations.cpp b/llvm/lib/Passes/StandardInstrumentations.cpp
> index 23c825c78713..b2283cc30658 100644
> --- a/llvm/lib/Passes/StandardInstrumentations.cpp
> +++ b/llvm/lib/Passes/StandardInstrumentations.cpp
> @@ -164,22 +164,30 @@ static cl::opt<std::string> DotCfgDir(
>       cl::desc("Generate dot files into specified directory for changed IRs"),
>       cl::Hidden, cl::init("./"));
>   
> +// An option for specifying an executable that will be called with the IR
> +// everytime it changes in the opt pipeline.  It will also be called on
> +// the initial IR as it enters the pipeline.  The executable will be passed
> +// the name of a temporary file containing the IR and the PassID.  This may
> +// be used, for example, to call llc on the IR and run a test to determine
> +// which pass makes a change that changes the functioning of the IR.
> +// The usual modifier options work as expected.
> +static cl::opt<std::string>
> +    TestChanged("test-changed", cl::Hidden, cl::init(""),
> +                cl::desc("exe called with module IR after each pass that "
> +                         "changes it"));
> +
>   namespace {
>   
> -// Perform a system based
> diff  between \p Before and \p After, using
> -// \p OldLineFormat, \p NewLineFormat, and \p UnchangedLineFormat
> -// to control the formatting of the output.  Return an error message
> -// for any failures instead of the
> diff .
> -std::string doSystemDiff(StringRef Before, StringRef After,
> -                         StringRef OldLineFormat, StringRef NewLineFormat,
> -                         StringRef UnchangedLineFormat) {
> -  StringRef SR[2]{Before, After};
> -  // Store the 2 bodies into temporary files and call
> diff  on them
> -  // to get the body of the node.
> -  const unsigned NumFiles = 3;
> -  static std::string FileName[NumFiles];
> -  static int FD[NumFiles]{-1, -1, -1};
> -  for (unsigned I = 0; I < NumFiles; ++I) {
> +// Ensure temporary files exist, creating or re-using them.  \p FD contains
> +// file descriptors (-1 indicates that the file should be created) and
> +// \p SR contains the corresponding initial content.  \p FileName will have
> +// the filenames filled in when creating files.  Return any error message
> +// or "" if none.
> +std::string prepareTempFiles(SmallVector<int> &FD, ArrayRef<StringRef> SR,
> +                             SmallVector<std::string> &FileName) {
> +  assert(FD.size() >= SR.size() && FileName.size() == FD.size() &&
> +         "Unexpected array sizes");
> +  for (unsigned I = 0; I < FD.size(); ++I) {
>       if (FD[I] == -1) {
>         SmallVector<char, 200> SV;
>         std::error_code EC =
> @@ -188,19 +196,44 @@ std::string doSystemDiff(StringRef Before, StringRef After,
>           return "Unable to create temporary file.";
>         FileName[I] = Twine(SV).str();
>       }
> -    // The third file is used as the result of the
> diff .
> -    if (I == NumFiles - 1)
> -      break;
> +    // Only the first M files have initial content.
> +    if (I < SR.size()) {
> +      std::error_code EC = sys::fs::openFileForWrite(FileName[I], FD[I]);
> +      if (EC)
> +        return "Unable to open temporary file for writing.";
> +      raw_fd_ostream OutStream(FD[I], /*shouldClose=*/true);
> +      if (FD[I] == -1)
> +        return "Error opening file for writing.";
> +      OutStream << SR[I];
> +    }
> +  }
> +  return "";
> +}
>   
> -    std::error_code EC = sys::fs::openFileForWrite(FileName[I], FD[I]);
> +std::string cleanUpTempFiles(ArrayRef<std::string> FileName) {
> +  for (unsigned I = 0; I < FileName.size(); ++I) {
> +    std::error_code EC = sys::fs::remove(FileName[I]);
>       if (EC)
> -      return "Unable to open temporary file for writing.";
> -
> -    raw_fd_ostream OutStream(FD[I], /*shouldClose=*/true);
> -    if (FD[I] == -1)
> -      return "Error opening file for writing.";
> -    OutStream << SR[I];
> +      return "Unable to remove temporary file.";
>     }
> +  return "";
> +}
> +
> +// Perform a system based
> diff  between \p Before and \p After, using
> +// \p OldLineFormat, \p NewLineFormat, and \p UnchangedLineFormat
> +// to control the formatting of the output.  Return an error message
> +// for any failures instead of the
> diff .
> +std::string doSystemDiff(StringRef Before, StringRef After,
> +                         StringRef OldLineFormat, StringRef NewLineFormat,
> +                         StringRef UnchangedLineFormat) {
> +  // Store the 2 bodies into temporary files and call
> diff  on them
> +  // to get the body of the node.
> +  static SmallVector<int> FD{-1, -1, -1};
> +  SmallVector<StringRef> SR{Before, After};
> +  static SmallVector<std::string> FileName{"", "", ""};
> +  std::string Err = prepareTempFiles(FD, SR, FileName);
> +  if (Err != "")
> +    return Err;
>   
>     static ErrorOr<std::string> DiffExe = sys::findProgramByName(DiffBinary);
>     if (!DiffExe)
> @@ -224,12 +257,10 @@ std::string doSystemDiff(StringRef Before, StringRef After,
>     else
>       return "Unable to read result.";
>   
> -  // Clean up.
> -  for (const std::string &I : FileName) {
> -    std::error_code EC = sys::fs::remove(I);
> -    if (EC)
> -      return "Unable to remove temporary file.";
> -  }
> +  Err = cleanUpTempFiles(FileName);
> +  if (Err != "")
> +    return Err;
> +
>     return Diff;
>   }
>   
> @@ -620,6 +651,59 @@ void IRChangedPrinter::handleAfter(StringRef PassID, std::string &Name,
>     Out << "*** IR Dump After " << PassID << " on " << Name << " ***\n" << After;
>   }
>   
> +IRChangedTester::~IRChangedTester() {}
> +
> +void IRChangedTester::registerCallbacks(PassInstrumentationCallbacks &PIC) {
> +  if (TestChanged != "")
> +    TextChangeReporter<std::string>::registerRequiredCallbacks(PIC);
> +}
> +
> +void IRChangedTester::handleIR(const std::string &S, StringRef PassID) {
> +  // Store the body into a temporary file
> +  static SmallVector<int> FD{-1};
> +  SmallVector<StringRef> SR{S};
> +  static SmallVector<std::string> FileName{""};
> +  std::string Err = prepareTempFiles(FD, SR, FileName);
> +  if (Err != "") {
> +    dbgs() << Err;
> +    return;
> +  }
> +  static ErrorOr<std::string> Exe = sys::findProgramByName(TestChanged);
> +  if (!Exe) {
> +    dbgs() << "Unable to find test-changed executable.";
> +    return;
> +  }
> +
> +  StringRef Args[] = {TestChanged, FileName[0], PassID};
> +  int Result = sys::ExecuteAndWait(*Exe, Args);
> +  if (Result < 0) {
> +    dbgs() << "Error executing test-changed executable.";
> +    return;
> +  }
> +
> +  Err = cleanUpTempFiles(FileName);
> +  if (Err != "")
> +    dbgs() << Err;
> +}
> +
> +void IRChangedTester::handleInitialIR(Any IR) {
> +  // Always test the initial module.
> +  // Unwrap and print directly to avoid filtering problems in general routines.
> +  std::string S;
> +  generateIRRepresentation(IR, "Initial IR", S);
> +  handleIR(S, "Initial IR");
> +}
> +
> +void IRChangedTester::omitAfter(StringRef PassID, std::string &Name) {}
> +void IRChangedTester::handleInvalidated(StringRef PassID) {}
> +void IRChangedTester::handleFiltered(StringRef PassID, std::string &Name) {}
> +void IRChangedTester::handleIgnored(StringRef PassID, std::string &Name) {}
> +void IRChangedTester::handleAfter(StringRef PassID, std::string &Name,
> +                                  const std::string &Before,
> +                                  const std::string &After, Any) {
> +  handleIR(After, PassID);
> +}
> +
>   template <typename T>
>   void OrderedChangedData<T>::report(
>       const OrderedChangedData &Before, const OrderedChangedData &After,
> @@ -2132,6 +2216,7 @@ void StandardInstrumentations::registerCallbacks(
>       Verify.registerCallbacks(PIC);
>     PrintChangedDiff.registerCallbacks(PIC);
>     WebsiteChangeReporter.registerCallbacks(PIC);
> +  ChangeTester.registerCallbacks(PIC);
>   }
>   
>   template class ChangeReporter<std::string>;
>
> diff  --git a/llvm/test/Other/test-changed-script.sh b/llvm/test/Other/test-changed-script.sh
> new file mode 100755
> index 000000000000..19c02bb06844
> --- /dev/null
> +++ b/llvm/test/Other/test-changed-script.sh
> @@ -0,0 +1,4 @@
> +#!/bin/sh
> +
> +echo "***" $2 "***"
> +cat $1
>
> diff  --git a/llvm/test/Other/test-changed.ll b/llvm/test/Other/test-changed.ll
> new file mode 100644
> index 000000000000..e34f3de366c5
> --- /dev/null
> +++ b/llvm/test/Other/test-changed.ll
> @@ -0,0 +1,103 @@
> +; Simple checks of -test-changed=%S/test-changed-script.sh functionality
> +;
> +; Simple functionality check.
> +; RUN: opt -S -test-changed=%S/test-changed-script.sh -passes=instsimplify 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-SIMPLE
> +;
> +; Check that only the passes that change the IR are printed and that the
> +; others (including g) are filtered out.
> +; RUN: opt -S -test-changed=%S/test-changed-script.sh -passes=instsimplify -filter-print-funcs=f  2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-FUNC-FILTER
> +;
> +; Check that the reporting of IRs respects -print-module-scope
> +; RUN: opt -S -test-changed=%S/test-changed-script.sh -passes=instsimplify -print-module-scope 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-PRINT-MOD-SCOPE
> +;
> +; Check that the reporting of IRs respects -print-module-scope
> +; RUN: opt -S -test-changed=%S/test-changed-script.sh -passes=instsimplify -filter-print-funcs=f -print-module-scope 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-FUNC-FILTER-MOD-SCOPE
> +;
> +; Check that reporting of multiple functions happens
> +; RUN: opt -S -test-changed=%S/test-changed-script.sh -passes=instsimplify -filter-print-funcs="f,g" 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-FILTER-MULT-FUNC
> +;
> +; Check that the reporting of IRs respects -filter-passes
> +; RUN: opt -S -test-changed=%S/test-changed-script.sh -passes="instsimplify,no-op-function" -filter-passes="NoOpFunctionPass" 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-FILTER-PASSES
> +;
> +; Check that the reporting of IRs respects -filter-passes with multiple passes
> +; RUN: opt -S -test-changed=%S/test-changed-script.sh -passes="instsimplify,no-op-function" -filter-passes="NoOpFunctionPass,InstSimplifyPass" 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-FILTER-MULT-PASSES
> +;
> +; Check that the reporting of IRs respects both -filter-passes and -filter-print-funcs
> +; RUN: opt -S -test-changed=%S/test-changed-script.sh -passes="instsimplify,no-op-function" -filter-passes="NoOpFunctionPass,InstSimplifyPass" -filter-print-funcs=f 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-FILTER-FUNC-PASSES
> +;
> +; Check that the reporting of IRs respects -filter-passes, -filter-print-funcs and -print-module-scope
> +; RUN: opt -S -test-changed=%S/test-changed-script.sh -passes="instsimplify,no-op-function" -filter-passes="NoOpFunctionPass,InstSimplifyPass" -filter-print-funcs=f -print-module-scope 2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-FILTER-FUNC-PASSES-MOD-SCOPE
> +;
> +; Check that repeated passes that change the IR are printed and that the
> +; others (including g) are filtered out.  Note that the second time
> +; instsimplify is run on f, it does not change the IR
> +; RUN: opt -S -test-changed=%S/test-changed-script.sh -passes="instsimplify,instsimplify" -filter-print-funcs=f  2>&1 -o /dev/null < %s | FileCheck %s --check-prefix=CHECK-MULT-PASSES-FILTER-FUNC
> +;
> +
> +define i32 @g() {
> +entry:
> +  %a = add i32 2, 3
> +  ret i32 %a
> +}
> +
> +define i32 @f() {
> +entry:
> +  %a = add i32 2, 3
> +  ret i32 %a
> +}
> +
> +; CHECK-SIMPLE: *** Initial IR ***
> +; CHECK-SIMPLE-NEXT: ; ModuleID = {{.+}}
> +; CHECK-SIMPLE: *** InstSimplifyPass ***
> +; CHECK-SIMPLE-NEXT: define i32 @g()
> +; CHECK-SIMPLE: *** InstSimplifyPass ***
> +; CHECK-SIMPLE-NEXT: define i32 @f()
> +
> +; CHECK-FUNC-FILTER: *** Initial IR ***
> +; CHECK-FUNC-FILTER-NEXT: define i32 @f()
> +; CHECK-FUNC-FILTER: *** InstSimplifyPass ***
> +; CHECK-FUNC-FILTER-NEXT: define i32 @f()
> +
> +; CHECK-PRINT-MOD-SCOPE: *** Initial IR ***
> +; CHECK-PRINT-MOD-SCOPE-NEXT: ModuleID = {{.+}}
> +; CHECK-PRINT-MOD-SCOPE: *** InstSimplifyPass ***
> +; CHECK-PRINT-MOD-SCOPE-NEXT: ModuleID = {{.+}}
> +; CHECK-PRINT-MOD-SCOPE: *** InstSimplifyPass ***
> +; CHECK-PRINT-MOD-SCOPE-NEXT: ModuleID = {{.+}}
> +
> +; CHECK-FUNC-FILTER-MOD-SCOPE: *** Initial IR ***
> +; CHECK-FUNC-FILTER-MOD-SCOPE-NEXT: ; ModuleID = {{.+}}
> +; CHECK-FUNC-FILTER-MOD-SCOPE: *** InstSimplifyPass ***
> +; CHECK-FUNC-FILTER-MOD-SCOPE-NEXT: ModuleID = {{.+}}
> +
> +; CHECK-FILTER-MULT-FUNC: *** Initial IR ***
> +; CHECK-FILTER-MULT-FUNC-NEXT: define i32 @g()
> +; CHECK-FILTER-MULT-FUNC: *** InstSimplifyPass ***
> +; CHECK-FILTER-MULT-FUNC-NEXT: define i32 @g()
> +; CHECK-FILTER-MULT-FUNC: *** InstSimplifyPass ***
> +; CHECK-FILTER-MULT-FUNC-NEXT: define i32 @f()
> +
> +; CHECK-FILTER-PASSES: *** Initial IR ***
> +; CHECK-FILTER-PASSES-NEXT: define i32 @g()
> +
> +; CHECK-FILTER-MULT-PASSES: *** Initial IR ***
> +; CHECK-FILTER-MULT-PASSES-NEXT: define i32 @g()
> +; CHECK-FILTER-MULT-PASSES: *** InstSimplifyPass ***
> +; CHECK-FILTER-MULT-PASSES-NEXT: define i32 @g()
> +; CHECK-FILTER-MULT-PASSES: *** InstSimplifyPass ***
> +; CHECK-FILTER-MULT-PASSES-NEXT: define i32 @f()
> +
> +; CHECK-FILTER-FUNC-PASSES: *** Initial IR ***
> +; CHECK-FILTER-FUNC-PASSES-NEXT: define i32 @f()
> +; CHECK-FILTER-FUNC-PASSES: *** InstSimplifyPass ***
> +; CHECK-FILTER-FUNC-PASSES-NEXT: define i32 @f()
> +
> +; CHECK-FILTER-FUNC-PASSES-MOD-SCOPE: *** Initial IR ***
> +; CHECK-FILTER-FUNC-PASSES-MOD-SCOPE-NEXT: ; ModuleID = {{.+}}
> +; CHECK-FILTER-FUNC-PASSES-MOD-SCOPE: *** InstSimplifyPass ***
> +; CHECK-FILTER-FUNC-PASSES-MOD-SCOPE-NEXT: ModuleID = {{.+}}
> +
> +; CHECK-MULT-PASSES-FILTER-FUNC: *** Initial IR ***
> +; CHECK-MULT-PASSES-FILTER-FUNC-NEXT: define i32 @f()
> +; CHECK-MULT-PASSES-FILTER-FUNC: *** InstSimplifyPass ***
> +; CHECK-MULT-PASSES-FILTER-FUNC-NEXT: define i32 @f()
>
>
>          
> _______________________________________________
> 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