[llc/opt] Add an option to run all passes twice

[loladiro]

Summary

Lately, I have submitted a number of patches to fix bugs that only occurred when using the same pass manager to compile multiple modules (generally these bugs are failure to reset some persistent state). Unfortunately I don't think there is currently a way to test that from the command line. This adds a very simple flag to both llc and opt, under which the tools will simply re-run their respective pass pipelines using the same pass manager on the same module. I believe that should be sufficient to be able to test for that kind of bug, but any other ideas would be appreciated as well.

[yaron.keren]

That's a great idea. Maybe add a LIT test that uses this flag so breakages will be caught at the buildbots?

[loladiro]

The motivation for this were D14962 and D14963, so assuming this is an ok path forward, I was planning to just commit this first and add the appropriate tests there.

[yaron.keren]

This makes sense.

[yaron.keren]

Won't the second run output will overwrite the first run output?
If so, what do you think about renaming the second output so it may be binary compared to the first one?

[loladiro]

I think what happens currently is that it will just append the second one
at the end of the first. I originally thought binary comparing the output
was not sensible as we're actually running over the same module twice and
the passes are of course not idempotent, but now I'm thinking maybe it's
better to clone the module and allow this. I'll take a look.

[yaron.keren]

You are right, running passes over again for all but trivial code will not result in the same module.
The CloneModule approach should work.

[loladiro]

Ok, how about doing this for the run-twice option:

• Clone the module to make sure the operation is idempotent
• Emit both results into buffers and always compare them to make sure they're equal
• Always emit the result of the second run

That way, to investigate a failure here, run it once without the run-twice option,
once with the run twice option and compare the outputs.

I also added some trivial tests, just to make sure these are exercised now that
they actually have non-trivial behavior.

[yaron.keren]

Do you see any reasonable way to share the duplicated code between llc.cpp and opt.cpp?

[loladiro]

I don't see any existing abstraction that this naturally fits into, so I'm not sure. Did you have something in mind?

[yaron.keren]

Only abstracting the whole llc/opt drivers which is more trouble than this is worth.
Maybe a commment in llc that opt does something similar and vice versa.
LGTM.

[dschuff]

llvm/trunk/tools/opt/opt.cpp

623 ↗

(On Diff #41936)

This use of CloneModule means that llc now depends on TransformUtils (so it needed to be added to required_libraries in LLVMBuild.txt and CMakeLists.txt)

Fixed in r254786

[loladiro]

Thanks! Sorry about that.

test/MC/ELF/empty-twice.ll

+; Check that there is no persistent state in the ELF emitter that crashes us
+; when we try to reuse the pass manager
+; RUN: llc -compile-twice -filetype=obj %s -o -
+
+target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:32:32-n8:16:32"
+target triple = "i386-pc-linux-gnu"

test/Other/opt-twice.ll

+; The pass here doesn't matter (we use deadargelim), but test
+; that the -run-twice options exists, generates output, and
+; doesn't crash
+; RUN: opt -run-twice -deadargelim -S < %s | FileCheck %s
+
+; CHECK: define internal void @test
+define internal {} @test() {
+  ret {} undef
+}
+
+define void @caller() {
+  call {} @test()
+  ret void
+}

tools/llc/llc.cpp

 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetSubtargetInfo.h"
+#include "llvm/Transforms/Utils/Cloning.h"
 #include <memory>
 using namespace llvm;
 
 // General options for llc.  Other pass-specific options are specified
 // within the corresponding llc passes, and target-specific options
 // and back-end code generation options are specified with the target machine.
 //
 static cl::opt<std::string>

 static cl::opt<bool> EnableDwarfDirectory(
     "enable-dwarf-directory", cl::Hidden,
     cl::desc("Use .file directives with an explicit directory."));
 
 static cl::opt<bool> AsmVerbose("asm-verbose",
                                 cl::desc("Add comments to directives."),
                                 cl::init(true));
 
+static cl::opt<bool>
+    CompileTwice("compile-twice", cl::Hidden,
+                 cl::desc("Run everything twice, re-using the same pass "
+                          "manager and verify the the result is the same."),
+                 cl::init(false));
+
 static int compileModule(char **, LLVMContext &);
 
 static std::unique_ptr<tool_output_file>
 GetOutputStream(const char *TargetName, Triple::OSType OS,
                 const char *ProgName) {
   // If we don't yet have an output filename, make one.
   if (OutputFilename.empty()) {
     if (InputFilename == "-")
static int compileModule(char **argv, LLVMContext &Context) {
 
   if (RelaxAll.getNumOccurrences() > 0 &&
       FileType != TargetMachine::CGFT_ObjectFile)
     errs() << argv[0]
              << ": warning: ignoring -mc-relax-all because filetype != obj";
 
   {
     raw_pwrite_stream *OS = &Out->os();
-    std::unique_ptr<buffer_ostream> BOS;
-    if (FileType != TargetMachine::CGFT_AssemblyFile &&
-        !Out->os().supportsSeeking()) {
-      BOS = make_unique<buffer_ostream>(*OS);
+
+    // Manually do the buffering rather than using buffer_ostream,
+    // so we can memcmp the contents in CompileTwice mode
+    SmallVector<char, 0> Buffer;
+    std::unique_ptr<raw_svector_ostream> BOS;
+    if ((FileType != TargetMachine::CGFT_AssemblyFile &&
+         !Out->os().supportsSeeking()) ||
+        CompileTwice) {
+      BOS = make_unique<raw_svector_ostream>(Buffer);
       OS = BOS.get();
     }
 
     AnalysisID StartBeforeID = nullptr;
     AnalysisID StartAfterID = nullptr;
     AnalysisID StopAfterID = nullptr;
     const PassRegistry *PR = PassRegistry::getPassRegistry();
     if (!RunPass.empty()) {
if (Target->addPassesToEmitFile(PM, *OS, FileType, NoVerify, StartBeforeID,
       errs() << argv[0] << ": target does not support generation of this"
              << " file type!\n";
       return 1;
     }
 
     // Before executing passes, print the final values of the LLVM options.
     cl::PrintOptionValues();
 
+    // If requested, run the pass manager over the same module again,
+    // to catch any bugs due to persistent state in the passes
+    SmallVector<char, 0> CompileTwiceBuffer;
+    if (CompileTwice) {
+      std::unique_ptr<Module> M2(llvm::CloneModule(M.get()));
+      PM.run(*M2);
+      CompileTwiceBuffer = Buffer;
+      Buffer.clear();
+    }
+
     PM.run(*M);
+
+    // Compare the two outputs and make sure they're the same
+    if (CompileTwice) {
+      if (Buffer.size() != CompileTwiceBuffer.size() ||
+          (memcmp(Buffer.data(), CompileTwiceBuffer.data(), Buffer.size()) !=
+           0)) {
+        errs()
+            << "Running the pass manager twice changed the output.\n"
+               "Writing the result of the second run to the specified output\n"
+               "To generate the one-run comparison binary, just run without\n"
+               "the compile-twice option\n";
+        Out->os() << Buffer;
+        Out->keep();
+        return 1;
+      }
+    }
+
+    if (BOS) {
+      Out->os() << Buffer;
+    }
   }
 
   // Declare success.
   Out->keep();
 
   return 0;
 }

tools/opt/opt.cpp

 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Bitcode/BitcodeWriterPass.h"
 #include "llvm/CodeGen/CommandFlags.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/IRPrintingPasses.h"
 #include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/LegacyPassNameParser.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/LinkAllIR.h"
 #include "llvm/LinkAllPasses.h"
 #include "llvm/MC/SubtargetFeature.h"
-#include "llvm/IR/LegacyPassManager.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/PluginLoader.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/SystemUtils.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Transforms/IPO/PassManagerBuilder.h"
+#include "llvm/Transforms/Utils/Cloning.h"
 #include <algorithm>
 #include <memory>
 using namespace llvm;
 using namespace opt_tool;
 
 // The OptimizationList is automatically populated with registered Passes by the
 // PassNameParser.
 //
static cl::opt<bool> PreserveBitcodeUseListOrder(
     cl::desc("Preserve use-list order when writing LLVM bitcode."),
     cl::init(true), cl::Hidden);
 
 static cl::opt<bool> PreserveAssemblyUseListOrder(
     "preserve-ll-uselistorder",
     cl::desc("Preserve use-list order when writing LLVM assembly."),
     cl::init(false), cl::Hidden);
 
+static cl::opt<bool>
+    RunTwice("run-twice",
+             cl::desc("Run all passes twice, re-using the same pass manager."),
+             cl::init(false), cl::Hidden);
+
 static inline void addPass(legacy::PassManagerBase &PM, Pass *P) {
   // Add the pass to the pass manager...
   PM.add(P);
 
   // If we are verifying all of the intermediate steps, add the verifier...
   if (VerifyEach)
     PM.add(createVerifierPass());
 }
for (Function &F : *M)
       FPasses->run(F);
     FPasses->doFinalization();
   }
 
   // Check that the module is well formed on completion of optimization
   if (!NoVerify && !VerifyEach)
     Passes.add(createVerifierPass());
 
+  // In run twice mode, we want to make sure the output is bit-by-bit
+  // equivalent if we run the pass manager again, so setup two buffers and
+  // a stream to write to them.
+  SmallVector<char, 0> Buffer;
+  SmallVector<char, 0> CompileTwiceBuffer;
+  std::unique_ptr<raw_svector_ostream> BOS;
+  raw_ostream *OS = &Out->os();
+  if (RunTwice) {
+    BOS = make_unique<raw_svector_ostream>(Buffer);
+    OS = BOS.get();
+  }
+
   // Write bitcode or assembly to the output as the last step...
   if (!NoOutput && !AnalyzeOnly) {
     if (OutputAssembly)
-      Passes.add(
-          createPrintModulePass(Out->os(), "", PreserveAssemblyUseListOrder));
+      Passes.add(createPrintModulePass(*OS, "", PreserveAssemblyUseListOrder));
     else
-      Passes.add(
-          createBitcodeWriterPass(Out->os(), PreserveBitcodeUseListOrder));
+      Passes.add(createBitcodeWriterPass(*OS, PreserveBitcodeUseListOrder));
   }
 
   // Before executing passes, print the final values of the LLVM options.
   cl::PrintOptionValues();
 
   // Now that we have all of the passes ready, run them.
   Passes.run(*M);
 
+  // If requested, run all passes again with the same pass manager to catch
+  // bugs caused by persistent state in the passes
+  if (RunTwice) {
+    CompileTwiceBuffer = Buffer;
+    Buffer.clear();
+    std::unique_ptr<Module> M2(CloneModule(M.get()));
+    Passes.run(*M2);
+    if (Buffer.size() != CompileTwiceBuffer.size() ||
+        (memcmp(Buffer.data(), CompileTwiceBuffer.data(), Buffer.size()) !=
+         0)) {
+      errs() << "Running the pass manager twice changed the output.\n"
+                "Writing the result of the second run to the specified output."
+                "To generate the one-run comparison binary, just run without\n"
+                "the compile-twice option\n";
+      Out->os() << BOS->str();
+      Out->keep();
+      return 1;
+    }
+    Out->os() << BOS->str();
+  }
+
   // Declare success.
   if (!NoOutput || PrintBreakpoints)
     Out->keep();
 
   return 0;
 }