[llvm-reduce] Allow writing temporary files as bitcode.

[fhahn]

Summary

Textual LLVM IR files are much bigger and take longer to write to disk.
To avoid the extra cost incurred by serializing to text, this patch adds
an option to save temporary files as bitcode instead.

IMO writing bitcode would ideally be the default, as it is more
efficient and for most uses cases the temporary files will likely be
consumed by other LLVM tools that support bitcode.

But using textual IR allows for convenient use of FileCheck in the
tests, so all test would need updating.

I am also not sure how to best add a unit test using bitcode temporary
files. Python script that somehow also takes the patch to llvm-as as
argument somehow?

[dblaikie]

Would be good to have some testing - I guess it might look like the testing that was done before it was switched over to FileCheck - but I forget /exactly/ how awkward that was.

[fhahn]

In D113858#3131845, @dblaikie wrote:

Would be good to have some testing - I guess it might look like the testing that was done before it was switched over to FileCheck - but I forget /exactly/ how awkward that was.

There are a couple of tests that still use a python script from the Inputs directory. But I am not sure how to pass the path to llvm-as to the script? :(

[dblaikie]

In D113858#3131850, @fhahn wrote:

In D113858#3131845, @dblaikie wrote:

Would be good to have some testing - I guess it might look like the testing that was done before it was switched over to FileCheck - but I forget /exactly/ how awkward that was.

There are a couple of tests that still use a python script from the Inputs directory. But I am not sure how to pass the path to llvm-as to the script? :(

If the script took the path as an argument, then the test would pass the argument via --test-arg llvm-as - I think lit will do the substitution there...

[fhahn]

In D113858#3131872, @dblaikie wrote:

In D113858#3131850, @fhahn wrote:

In D113858#3131845, @dblaikie wrote:

Would be good to have some testing - I guess it might look like the testing that was done before it was switched over to FileCheck - but I forget /exactly/ how awkward that was.

There are a couple of tests that still use a python script from the Inputs directory. But I am not sure how to pass the path to llvm-as to the script? :(

If the script took the path as an argument, then the test would pass the argument via --test-arg llvm-as - I think lit will do the substitution there...

Ah great, I missed that there's --test-arg to pass multipel options through to the test script. I updated the patch with a python script to run llvm-dis and then FileCheck on the output. I'm planning on landing this once the pre-commit tests come back ok.

[dblaikie]

In D113858#3134353, @fhahn wrote:

In D113858#3131872, @dblaikie wrote:

In D113858#3131850, @fhahn wrote:

In D113858#3131845, @dblaikie wrote:

Would be good to have some testing - I guess it might look like the testing that was done before it was switched over to FileCheck - but I forget /exactly/ how awkward that was.

There are a couple of tests that still use a python script from the Inputs directory. But I am not sure how to pass the path to llvm-as to the script? :(

If the script took the path as an argument, then the test would pass the argument via --test-arg llvm-as - I think lit will do the substitution there...

Ah great, I missed that there's --test-arg to pass multipel options through to the test script. I updated the patch with a python script to run llvm-dis and then FileCheck on the output. I'm planning on landing this once the pre-commit tests come back ok.

Nice, looks great!

[fhahn]

Any opinions on adjusting the default?

[dblaikie]

In D113858#3138549, @fhahn wrote:

Any opinions on adjusting the default?

Yeah, I'd be open to/probably in favor of that - as you say, most often this'll be used to reduce an llvm crash, miscompile, etc - which would be fine consuming a bitcode file.

[aeubanks]

In D113858#3138629, @dblaikie wrote:

In D113858#3138549, @fhahn wrote:

Any opinions on adjusting the default?

Yeah, I'd be open to/probably in favor of that - as you say, most often this'll be used to reduce an llvm crash, miscompile, etc - which would be fine consuming a bitcode file.

+1, should be fine

llvm/test/tools/llvm-reduce/Inputs/llvm-dis-and-filecheck.py

+"""
+Script to disassembles a bitcode file and run FileCheck on the output with the
+provided arguments. The first 2 arguments are the paths to the llvm-dis and
+FileCheck binaries, followed by arguments to be passed to FileCheck. The last
+argument is the bitcode file to disassemble.
+
+Usage:
+    python llvm-dis-and-filecheck.py
+      <path to llvm-dis> <path to FileCheck>
+      [arguments passed to FileCheck] <path to bitcode file>
+
+"""
+
+
+import sys
+import subprocess
+
+llvm_dis = sys.argv[1]
+filecheck = sys.argv[2]
+filecheck_args = [filecheck, ]
+filecheck_args.extend(sys.argv[3:-1])
+bitcode_file = sys.argv[-1]
+
+disassemble = subprocess.Popen([llvm_dis, "-o", "-", bitcode_file],
+        stdout=subprocess.PIPE)
+check = subprocess.Popen(filecheck_args, stdin=disassemble.stdout)
+disassemble.stdout.close()
+check.communicate()
+sys.exit(check.returncode)

llvm/test/tools/llvm-reduce/temporary-files-as-bitcode.ll

+; RUN: llvm-reduce -write-tmp-files-as-bitcode --delta-passes=basic-blocks %s -o %t \
+; RUN:     --test %python --test-arg %p/Inputs/llvm-dis-and-filecheck.py --test-arg llvm-dis --test-arg FileCheck --test-arg --check-prefixes=CHECK-ALL,CHECK-INTERESTINGNESS --test-arg %s
+; RUN: cat %t | FileCheck --check-prefixes=CHECK-ALL,CHECK-FINAL %s
+
+; CHECK-INTERESTINGNESS: @callee(
+; CHECK-FINAL: declare void @callee()
+define void @callee() {
+  ret void
+}
+
+; CHECK-ALL: define void @caller()
+define void @caller() {
+entry:
+; CHECK-ALL: call void @callee()
+; CHECK-ALL: ret void
+  call void @callee()
+  ret void
+}

llvm/tools/llvm-reduce/deltas/Delta.cpp

 // it splits a given set of Targets (i.e. Functions, Instructions, BBs, etc.)
 // into chunks and tries to reduce the number chunks that are interesting.
 //
 //===----------------------------------------------------------------------===//
 
 #include "Delta.h"
 #include "ReducerWorkItem.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/Bitcode/BitcodeWriter.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include <fstream>
 #include <set>
 
 using namespace llvm;
 
 static cl::opt<bool> AbortOnInvalidReduction(
     "abort-on-invalid-reduction",
     cl::desc("Abort if any reduction results in invalid IR"));
 
 static cl::opt<unsigned int> StartingGranularityLevel(
     "starting-granularity-level",
     cl::desc("Number of times to divide chunks prior to first test"));
 
+static cl::opt<bool> TmpFilesAsBitcode(
+    "write-tmp-files-as-bitcode",
+    cl::desc("Write temporary files as bitcode, instead of textual IR"),
+    cl::init(false));
+
 void writeOutput(ReducerWorkItem &M, llvm::StringRef Message);
 
 bool isReduced(ReducerWorkItem &M, TestRunner &Test,
                SmallString<128> &CurrentFilepath) {
   // Write ReducerWorkItem to tmp file
   int FD;
   std::error_code EC = sys::fs::createTemporaryFile(
-      "llvm-reduce", M.isMIR() ? "mir" : "ll", FD, CurrentFilepath);
+      "llvm-reduce", M.isMIR() ? "mir" : (TmpFilesAsBitcode ? "bc" : "ll"), FD,
+      CurrentFilepath);
   if (EC) {
     errs() << "Error making unique filename: " << EC.message() << "!\n";
     exit(1);
   }
 
+  if (TmpFilesAsBitcode) {
+    llvm::raw_fd_ostream OutStream(FD, true);
+    WriteBitcodeToFile(M, OutStream);
+    OutStream.close();
+    if (OutStream.has_error()) {
+      errs() << "Error emitting bitcode to file '" << CurrentFilepath << "'!\n";
+      sys::fs::remove(CurrentFilepath);
+      exit(1);
+    }
+    bool Res = Test.run(CurrentFilepath);
+    sys::fs::remove(CurrentFilepath);
+    return Res;
+  }
   ToolOutputFile Out(CurrentFilepath, FD);
   M.print(Out.os(), /*AnnotationWriter=*/nullptr);
   Out.os().close();
   if (Out.os().has_error()) {
     errs() << "Error emitting bitcode to file '" << CurrentFilepath << "'!\n";
     exit(1);
   }