<div dir="ltr">r254804<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Dec 4, 2015 at 6:58 PM, Keno Fischer <span dir="ltr"><<a href="mailto:kfischer@college.harvard.edu" target="_blank">kfischer@college.harvard.edu</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Thanks,<div><br></div><div>I think I have a handle on this. Fix coming shortly.</div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Dec 4, 2015 at 6:52 PM, Justin Bogner <span dir="ltr"><<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Keno Fischer via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> writes:<br>
> Author: kfischer<br>
> Date: Fri Dec  4 15:56:46 2015<br>
> New Revision: 254774<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=254774&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=254774&view=rev</a><br>
> Log:<br>
> [llc/opt] Add an option to run all passes twice<br>
><br>
> Summary: Lately, I have submitted a number of patches to fix bugs that<br>
> only occurred when using the same pass manager to compile multiple<br>
> modules (generally these bugs are failure to reset some persistent<br>
> state). Unfortunately I don't think there is currently a way to test<br>
> that from the command line. This adds a very simple flag to both llc<br>
> and opt, under which the tools will simply re-run their respective<br>
> pass pipelines using the same pass manager on (a clone of the same<br>
> module). Additionally, we verify that both outputs are bitwise the<br>
> same.<br>
<br>
I'm getting a large number of Asan+Ubsan errors in opt calls from `ninja<br>
check` after this change. See below.<br>
<br>
> Reviewers: yaron.keren<br>
><br>
> Subscribers: loladiro, yaron.keren, kcc, llvm-commits<br>
><br>
> Differential Revision: <a href="http://reviews.llvm.org/D14965" rel="noreferrer" target="_blank">http://reviews.llvm.org/D14965</a><br>
><br>
> Added:<br>
>     llvm/trunk/test/MC/ELF/empty-twice.ll<br>
>     llvm/trunk/test/Other/opt-twice.ll<br>
> Modified:<br>
>     llvm/trunk/tools/llc/llc.cpp<br>
>     llvm/trunk/tools/opt/opt.cpp<br>
><br>
> Added: llvm/trunk/test/MC/ELF/empty-twice.ll<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ELF/empty-twice.ll?rev=254774&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ELF/empty-twice.ll?rev=254774&view=auto</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/MC/ELF/empty-twice.ll (added)<br>
> +++ llvm/trunk/test/MC/ELF/empty-twice.ll Fri Dec  4 15:56:46 2015<br>
> @@ -0,0 +1,6 @@<br>
> +; Check that there is no persistent state in the ELF emitter that crashes us<br>
> +; when we try to reuse the pass manager<br>
> +; RUN: llc -compile-twice -filetype=obj %s -o -<br>
> +<br>
> +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"<br>
> +target triple = "i386-pc-linux-gnu"<br>
><br>
> Added: llvm/trunk/test/Other/opt-twice.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/opt-twice.ll?rev=254774&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/opt-twice.ll?rev=254774&view=auto</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/Other/opt-twice.ll (added)<br>
> +++ llvm/trunk/test/Other/opt-twice.ll Fri Dec  4 15:56:46 2015<br>
> @@ -0,0 +1,14 @@<br>
> +; The pass here doesn't matter (we use deadargelim), but test<br>
> +; that the -run-twice options exists, generates output, and<br>
> +; doesn't crash<br>
> +; RUN: opt -run-twice -deadargelim -S < %s | FileCheck %s<br>
> +<br>
> +; CHECK: define internal void @test<br>
> +define internal {} @test() {<br>
> +  ret {} undef<br>
> +}<br>
> +<br>
> +define void @caller() {<br>
> +  call {} @test()<br>
> +  ret void<br>
> +}<br>
><br>
> Modified: llvm/trunk/tools/llc/llc.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llc/llc.cpp?rev=254774&r1=254773&r2=254774&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llc/llc.cpp?rev=254774&r1=254773&r2=254774&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/tools/llc/llc.cpp (original)<br>
> +++ llvm/trunk/tools/llc/llc.cpp Fri Dec  4 15:56:46 2015<br>
> @@ -45,6 +45,7 @@<br>
>  #include "llvm/Support/ToolOutputFile.h"<br>
>  #include "llvm/Target/TargetMachine.h"<br>
>  #include "llvm/Target/TargetSubtargetInfo.h"<br>
> +#include "llvm/Transforms/Utils/Cloning.h"<br>
>  #include <memory><br>
>  using namespace llvm;<br>
><br>
> @@ -96,6 +97,12 @@ static cl::opt<bool> AsmVerbose("asm-ver<br>
>                                  cl::desc("Add comments to directives."),<br>
>                                  cl::init(true));<br>
><br>
> +static cl::opt<bool><br>
> +    CompileTwice("compile-twice", cl::Hidden,<br>
> +                 cl::desc("Run everything twice, re-using the same pass "<br>
> +                          "manager and verify the the result is the same."),<br>
> +                 cl::init(false));<br>
> +<br>
>  static int compileModule(char **, LLVMContext &);<br>
><br>
>  static std::unique_ptr<tool_output_file><br>
> @@ -325,10 +332,15 @@ static int compileModule(char **argv, LL<br>
><br>
>    {<br>
>      raw_pwrite_stream *OS = &Out->os();<br>
> -    std::unique_ptr<buffer_ostream> BOS;<br>
> -    if (FileType != TargetMachine::CGFT_AssemblyFile &&<br>
> -        !Out->os().supportsSeeking()) {<br>
> -      BOS = make_unique<buffer_ostream>(*OS);<br>
> +<br>
> +    // Manually do the buffering rather than using buffer_ostream,<br>
> +    // so we can memcmp the contents in CompileTwice mode<br>
> +    SmallVector<char, 0> Buffer;<br>
> +    std::unique_ptr<raw_svector_ostream> BOS;<br>
> +    if ((FileType != TargetMachine::CGFT_AssemblyFile &&<br>
> +         !Out->os().supportsSeeking()) ||<br>
> +        CompileTwice) {<br>
> +      BOS = make_unique<raw_svector_ostream>(Buffer);<br>
>        OS = BOS.get();<br>
>      }<br>
><br>
> @@ -378,7 +390,39 @@ static int compileModule(char **argv, LL<br>
>      // Before executing passes, print the final values of the LLVM options.<br>
>      cl::PrintOptionValues();<br>
><br>
> +    // If requested, run the pass manager over the same module again,<br>
> +    // to catch any bugs due to persistent state in the passes. Note that<br>
> +    // opt has the same functionality, so it may be worth abstracting this out<br>
> +    // in the future.<br>
> +    SmallVector<char, 0> CompileTwiceBuffer;<br>
> +    if (CompileTwice) {<br>
> +      std::unique_ptr<Module> M2(llvm::CloneModule(M.get()));<br>
> +      PM.run(*M2);<br>
> +      CompileTwiceBuffer = Buffer;<br>
> +      Buffer.clear();<br>
> +    }<br>
> +<br>
>      PM.run(*M);<br>
> +<br>
> +    // Compare the two outputs and make sure they're the same<br>
> +    if (CompileTwice) {<br>
> +      if (Buffer.size() != CompileTwiceBuffer.size() ||<br>
> +          (memcmp(Buffer.data(), CompileTwiceBuffer.data(), Buffer.size()) !=<br>
> +           0)) {<br>
> +        errs()<br>
> +            << "Running the pass manager twice changed the output.\n"<br>
> +               "Writing the result of the second run to the specified output\n"<br>
> +               "To generate the one-run comparison binary, just run without\n"<br>
> +               "the compile-twice option\n";<br>
> +        Out->os() << Buffer;<br>
> +        Out->keep();<br>
> +        return 1;<br>
> +      }<br>
> +    }<br>
> +<br>
> +    if (BOS) {<br>
> +      Out->os() << Buffer;<br>
> +    }<br>
>    }<br>
><br>
>    // Declare success.<br>
><br>
> Modified: llvm/trunk/tools/opt/opt.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/opt/opt.cpp?rev=254774&r1=254773&r2=254774&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/opt/opt.cpp?rev=254774&r1=254773&r2=254774&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/tools/opt/opt.cpp (original)<br>
> +++ llvm/trunk/tools/opt/opt.cpp Fri Dec  4 15:56:46 2015<br>
> @@ -28,6 +28,7 @@<br>
>  #include "llvm/IR/DebugInfo.h"<br>
>  #include "llvm/IR/IRPrintingPasses.h"<br>
>  #include "llvm/IR/LLVMContext.h"<br>
> +#include "llvm/IR/LegacyPassManager.h"<br>
>  #include "llvm/IR/LegacyPassNameParser.h"<br>
>  #include "llvm/IR/Module.h"<br>
>  #include "llvm/IR/Verifier.h"<br>
> @@ -36,7 +37,6 @@<br>
>  #include "llvm/LinkAllIR.h"<br>
>  #include "llvm/LinkAllPasses.h"<br>
>  #include "llvm/MC/SubtargetFeature.h"<br>
> -#include "llvm/IR/LegacyPassManager.h"<br>
>  #include "llvm/Support/Debug.h"<br>
>  #include "llvm/Support/FileSystem.h"<br>
>  #include "llvm/Support/Host.h"<br>
> @@ -51,6 +51,7 @@<br>
>  #include "llvm/Support/ToolOutputFile.h"<br>
>  #include "llvm/Target/TargetMachine.h"<br>
>  #include "llvm/Transforms/IPO/PassManagerBuilder.h"<br>
> +#include "llvm/Transforms/Utils/Cloning.h"<br>
>  #include <algorithm><br>
>  #include <memory><br>
>  using namespace llvm;<br>
> @@ -190,6 +191,11 @@ static cl::opt<bool> PreserveAssemblyUse<br>
>      cl::desc("Preserve use-list order when writing LLVM assembly."),<br>
>      cl::init(false), cl::Hidden);<br>
><br>
> +static cl::opt<bool><br>
> +    RunTwice("run-twice",<br>
> +             cl::desc("Run all passes twice, re-using the same pass manager."),<br>
> +             cl::init(false), cl::Hidden);<br>
> +<br>
>  static inline void addPass(legacy::PassManagerBase &PM, Pass *P) {<br>
>    // Add the pass to the pass manager...<br>
>    PM.add(P);<br>
> @@ -582,14 +588,25 @@ int main(int argc, char **argv) {<br>
>    if (!NoVerify && !VerifyEach)<br>
>      Passes.add(createVerifierPass());<br>
><br>
> +  // In run twice mode, we want to make sure the output is bit-by-bit<br>
> +  // equivalent if we run the pass manager again, so setup two buffers and<br>
> +  // a stream to write to them. Note that llc does something similar and it<br>
> +  // may be worth to abstract this out in the future.<br>
> +  SmallVector<char, 0> Buffer;<br>
> +  SmallVector<char, 0> CompileTwiceBuffer;<br>
> +  std::unique_ptr<raw_svector_ostream> BOS;<br>
> +  raw_ostream *OS = &Out->os();<br>
<br>
Apparently `Out` is sometimes (often?) null here. If you want/need to<br>
investigate without having to build with sanitizers, just throw an<br>
assert(Out) here.<br>
<br>
> +  if (RunTwice) {<br>
> +    BOS = make_unique<raw_svector_ostream>(Buffer);<br>
> +    OS = BOS.get();<br>
> +  }<br>
> +<br>
>    // Write bitcode or assembly to the output as the last step...<br>
>    if (!NoOutput && !AnalyzeOnly) {<br>
>      if (OutputAssembly)<br>
> -      Passes.add(<br>
> -          createPrintModulePass(Out->os(), "", PreserveAssemblyUseListOrder));<br>
> +      Passes.add(createPrintModulePass(*OS, "", PreserveAssemblyUseListOrder));<br>
>      else<br>
> -      Passes.add(<br>
> -          createBitcodeWriterPass(Out->os(), PreserveBitcodeUseListOrder));<br>
> +      Passes.add(createBitcodeWriterPass(*OS, PreserveBitcodeUseListOrder));<br>
>    }<br>
><br>
>    // Before executing passes, print the final values of the LLVM options.<br>
> @@ -598,6 +615,27 @@ int main(int argc, char **argv) {<br>
>    // Now that we have all of the passes ready, run them.<br>
>    Passes.run(*M);<br>
><br>
> +  // If requested, run all passes again with the same pass manager to catch<br>
> +  // bugs caused by persistent state in the passes<br>
> +  if (RunTwice) {<br>
> +    CompileTwiceBuffer = Buffer;<br>
> +    Buffer.clear();<br>
> +    std::unique_ptr<Module> M2(CloneModule(M.get()));<br>
> +    Passes.run(*M2);<br>
> +    if (Buffer.size() != CompileTwiceBuffer.size() ||<br>
> +        (memcmp(Buffer.data(), CompileTwiceBuffer.data(), Buffer.size()) !=<br>
> +         0)) {<br>
> +      errs() << "Running the pass manager twice changed the output.\n"<br>
> +                "Writing the result of the second run to the specified output."<br>
> +                "To generate the one-run comparison binary, just run without\n"<br>
> +                "the compile-twice option\n";<br>
> +      Out->os() << BOS->str();<br>
> +      Out->keep();<br>
> +      return 1;<br>
> +    }<br>
> +    Out->os() << BOS->str();<br>
> +  }<br>
> +<br>
>    // Declare success.<br>
>    if (!NoOutput || PrintBreakpoints)<br>
>      Out->keep();<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>