llvm-reduce: Refine missing argument behavior

[arsenm]

Summary

We required the test and input arguments for --print-delta-passes
which is unhelpful. Also, start printing the help output if no
arguments were supplied.

It looks like there's more sophisticated ways to accomplish this with
the opt library, but it was less work to manually emit these errors.

[arsenm]

Forgot to save StringRef changes

[arsenm]

ping

[regehr]

LGTM

(I actually wanted the "Don't require any other arguments for --print-delta-passes" just the other day)

[arsenm]

9c8b89f580d0a6ee43c9ac5a491ee1c56078a62f

llvm/test/tools/llvm-reduce/command-line-behavior.test

+# Print the help output if no arguments are specified
+# RUN: llvm-reduce --help | grep "LLVM automatic testcase reducer"
+# RUN: llvm-reduce | grep "LLVM automatic testcase reducer"
+
+# Don't require any other arguments for --print-delta-passes
+# RUN: llvm-reduce --print-delta-passes  | grep "Delta passes (pass to \`--delta-passes=\` as a comma separated list)"
+
+# Missing test input
+# RUN: not llvm-reduce --test FileCheck 2>&1 | grep "error: reduction testcase positional argument must be specified"
+
+# Missing test script
+# RUN: not llvm-reduce some-input 2>&1 | grep "error: --test option must be specified"

llvm/tools/llvm-reduce/ReducerWorkItem.h

public:
   }
 
 private:
   uint64_t computeIRComplexityScore() const;
   uint64_t computeMIRComplexityScore() const;
 };
 
 std::pair<std::unique_ptr<ReducerWorkItem>, bool>
-parseReducerWorkItem(const char *ToolName, StringRef Filename,
-                     LLVMContext &Ctxt, std::unique_ptr<TargetMachine> &TM,
-                     bool IsMIR);
+parseReducerWorkItem(StringRef ToolName, StringRef Filename, LLVMContext &Ctxt,
+                     std::unique_ptr<TargetMachine> &TM, bool IsMIR);
 
 std::unique_ptr<ReducerWorkItem>
 cloneReducerWorkItem(const ReducerWorkItem &MMM, const TargetMachine *TM);
 
 bool verifyReducerWorkItem(const ReducerWorkItem &MMM, raw_fd_ostream *OS);
 
 #endif

llvm/tools/llvm-reduce/ReducerWorkItem.cpp

 #include "llvm/Transforms/Utils/Cloning.h"
 #include <optional>
 
 extern cl::OptionCategory LLVMReduceOptions;
 static cl::opt<std::string> TargetTriple("mtriple",
                                          cl::desc("Set the target triple"),
                                          cl::cat(LLVMReduceOptions));
 
-void readBitcode(ReducerWorkItem &M, MemoryBufferRef Data, LLVMContext &Ctx, const char *ToolName);
+void readBitcode(ReducerWorkItem &M, MemoryBufferRef Data, LLVMContext &Ctx,
+                 StringRef ToolName);
 
 static void cloneFrameInfo(
     MachineFrameInfo &DstMFI, const MachineFrameInfo &SrcMFI,
     const DenseMap<MachineBasicBlock *, MachineBasicBlock *> &Src2DstMBB) {
   DstMFI.setFrameAddressIsTaken(SrcMFI.isFrameAddressTaken());
   DstMFI.setReturnAddressIsTaken(SrcMFI.isReturnAddressTaken());
   DstMFI.setHasStackMap(SrcMFI.hasStackMap());
   DstMFI.setHasPatchPoint(SrcMFI.hasPatchPoint());

 static void initializeTargetInfo() {
   InitializeAllTargets();
   InitializeAllTargetMCs();
   InitializeAllAsmPrinters();
   InitializeAllAsmParsers();
 }
 
 std::pair<std::unique_ptr<ReducerWorkItem>, bool>
-parseReducerWorkItem(const char *ToolName, StringRef Filename,
-                     LLVMContext &Ctxt, std::unique_ptr<TargetMachine> &TM,
-                     bool IsMIR) {
+parseReducerWorkItem(StringRef ToolName, StringRef Filename, LLVMContext &Ctxt,
+                     std::unique_ptr<TargetMachine> &TM, bool IsMIR) {
   bool IsBitcode = false;
   Triple TheTriple;
 
   auto MMM = std::make_unique<ReducerWorkItem>();
 
   if (IsMIR) {
     initializeTargetInfo();
 
if (std::error_code EC = MB.getError()) {
       WithColor::error(errs(), ToolName) << Filename << ": " << EC.message() << "\n";
       return {nullptr, false};
     }
 
     if (!isBitcode((const unsigned char *)(*MB)->getBufferStart(),
                   (const unsigned char *)(*MB)->getBufferEnd())) {
       std::unique_ptr<Module> Result = parseIRFile(Filename, Err, Ctxt);
       if (!Result) {
-        Err.print(ToolName, errs());
+        Err.print(ToolName.data(), errs());
         return {nullptr, false};
       }
       MMM->M = std::move(Result);
     } else {
       IsBitcode = true;
       readBitcode(*MMM, MemoryBufferRef(**MB), Ctxt, ToolName);
 
       if (MMM->LTOInfo->IsThinLTO && MMM->LTOInfo->EnableSplitLTOUnit)

llvm/tools/llvm-reduce/TestRunner.h

 
 // This class contains all the info necessary for running the provided
 // interesting-ness test, as well as the most reduced module and its
 // respective filename.
 class TestRunner {
 public:
   TestRunner(StringRef TestName, const std::vector<std::string> &TestArgs,
              std::unique_ptr<ReducerWorkItem> Program,
-             std::unique_ptr<TargetMachine> TM, const char *ToolName,
+             std::unique_ptr<TargetMachine> TM, StringRef ToolName,
              StringRef OutputFilename, bool InputIsBitcode, bool OutputBitcode);
 
   /// Runs the interesting-ness test for the specified file
   /// @returns 0 if test was successful, 1 if otherwise
   int run(StringRef Filename, const std::atomic<bool> &Killed) const;
 
   /// Returns the most reduced version of the original testcase
   ReducerWorkItem &getProgram() const { return *Program; }
 
   void setProgram(std::unique_ptr<ReducerWorkItem> P);
 
   const TargetMachine *getTargetMachine() const { return TM.get(); }
 
-  const char *getToolName() const { return ToolName; }
+  StringRef getToolName() const { return ToolName; }
 
   void writeOutput(StringRef Message);
 
   bool inputIsBitcode() const {
     return InputIsBitcode;
   }
 
 private:
   StringRef TestName;
-  const char *ToolName;
+  StringRef ToolName;
   const std::vector<std::string> &TestArgs;
   std::unique_ptr<ReducerWorkItem> Program;
   std::unique_ptr<TargetMachine> TM;
   StringRef OutputFilename;
   const bool InputIsBitcode;
   bool EmitBitcode;
 };
 
 } // namespace llvm
 
 #endif

llvm/tools/llvm-reduce/TestRunner.cpp

 static cl::opt<unsigned> PollInterval("process-poll-interval",
                                       cl::desc("child process wait polling"),
                                       cl::init(5), cl::Hidden,
                                       cl::cat(LLVMReduceOptions));
 
 TestRunner::TestRunner(StringRef TestName,
                        const std::vector<std::string> &TestArgs,
                        std::unique_ptr<ReducerWorkItem> Program,
-                       std::unique_ptr<TargetMachine> TM, const char *ToolName,
+                       std::unique_ptr<TargetMachine> TM, StringRef ToolName,
                        StringRef OutputName, bool InputIsBitcode,
                        bool OutputBitcode)
     : TestName(TestName), ToolName(ToolName), TestArgs(TestArgs),
       Program(std::move(Program)), TM(std::move(TM)),
       OutputFilename(OutputName), InputIsBitcode(InputIsBitcode),
       EmitBitcode(OutputBitcode) {
   assert(this->Program && "Initialized with null program?");
 }

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

static cl::opt<unsigned> NumJobs(
     cl::init(1), cl::cat(LLVMReduceOptions));
 #else
 unsigned NumJobs = 1;
 #endif
 
 void writeBitcode(ReducerWorkItem &M, raw_ostream &OutStream);
 
 void readBitcode(ReducerWorkItem &M, MemoryBufferRef Data, LLVMContext &Ctx,
-                 const char *ToolName);
+                 StringRef ToolName);
 
 bool isReduced(ReducerWorkItem &M, const TestRunner &Test,
                const std::atomic<bool> &Killed) {
   const bool UseBitcode = Test.inputIsBitcode() || TmpFilesAsBitcode;
 
   SmallString<128> CurrentFilepath;
 
   // Write ReducerWorkItem to tmp file

llvm/tools/llvm-reduce/llvm-reduce.cpp

static cl::opt<bool> Version("v", cl::desc("Alias for -version"), cl::Hidden,
                              cl::cat(LLVMReduceOptions));
 
 static cl::opt<bool> PreserveDebugEnvironment(
     "preserve-debug-environment",
     cl::desc("Don't disable features used for crash "
              "debugging (crash reports, llvm-symbolizer and core dumps)"),
     cl::cat(LLVMReduceOptions));
 
+// FIXME: should be able to do this without other arguments, also add in help.
 static cl::opt<bool>
     PrintDeltaPasses("print-delta-passes",
                      cl::desc("Print list of delta passes, passable to "
                               "--delta-passes as a comma separated list"),
                      cl::cat(LLVMReduceOptions));
 
-static cl::opt<std::string> InputFilename(cl::Positional, cl::Required,
+static cl::opt<std::string> InputFilename(cl::Positional,
                                           cl::desc("<input llvm ll/bc file>"),
                                           cl::cat(LLVMReduceOptions));
 
 static cl::opt<std::string>
-    TestFilename("test", cl::Required,
+    TestFilename("test",
                  cl::desc("Name of the interesting-ness test to be run"),
                  cl::cat(LLVMReduceOptions));
 
 static cl::list<std::string>
     TestArguments("test-arg",
                   cl::desc("Arguments passed onto the interesting-ness test"),
                   cl::cat(LLVMReduceOptions));
 
if (ReplaceInput) { // In-place
 
     OutputFilename =
         IsMIR ? "reduced.mir" : (OutputBitcode ? "reduced.bc" : "reduced.ll");
   }
 
   return {OutputFilename, OutputBitcode};
 }
 
-void readBitcode(ReducerWorkItem &M, MemoryBufferRef Data, LLVMContext &Ctx, const char *ToolName) {
+void readBitcode(ReducerWorkItem &M, MemoryBufferRef Data, LLVMContext &Ctx,
+                 StringRef ToolName) {
   Expected<BitcodeFileContents> IF = llvm::getBitcodeFileContents(Data);
   if (!IF) {
     WithColor::error(errs(), ToolName) << IF.takeError();
     exit(1);
   }
   BitcodeModule BM = IF->Mods[0];
   Expected<BitcodeLTOInfo> LI = BM.getLTOInfo();
   Expected<std::unique_ptr<Module>> MOrErr = BM.parseModule(Ctx);
   if (!LI || !MOrErr) {
     WithColor::error(errs(), ToolName) << IF.takeError();
     exit(1);
   }
   M.LTOInfo = std::make_unique<BitcodeLTOInfo>(*LI);
   M.M = std::move(MOrErr.get());
 }
 
 int main(int Argc, char **Argv) {
   InitLLVM X(Argc, Argv);
+  const StringRef ToolName(Argv[0]);
 
   cl::HideUnrelatedOptions({&LLVMReduceOptions, &getColorCategory()});
   cl::ParseCommandLineOptions(Argc, Argv, "LLVM automatic testcase reducer.\n");
 
+  if (Argc == 1) {
+    cl::PrintHelpMessage();
+    return 0;
+  }
+
+  if (PrintDeltaPasses) {
+    printDeltaPasses(outs());
+    return 0;
+  }
+
   bool ReduceModeMIR = false;
   if (InputLanguage != InputLanguages::None) {
     if (InputLanguage == InputLanguages::MIR)
       ReduceModeMIR = true;
   } else if (StringRef(InputFilename).endswith(".mir")) {
     ReduceModeMIR = true;
   }
 
-  if (PrintDeltaPasses) {
-    printDeltaPasses(errs());
-    return 0;
+  if (InputFilename.empty()) {
+    WithColor::error(errs(), ToolName)
+        << "reduction testcase positional argument must be specified\n";
+    return 1;
+  }
+
+  if (TestFilename.empty()) {
+    WithColor::error(errs(), ToolName) << "--test option must be specified\n";
+    return 1;
   }
 
   if (!PreserveDebugEnvironment)
     disableEnvironmentDebugFeatures();
 
   LLVMContext Context;
   std::unique_ptr<TargetMachine> TM;
 
   auto [OriginalProgram, InputIsBitcode] =
-      parseReducerWorkItem(Argv[0], InputFilename, Context, TM, ReduceModeMIR);
+      parseReducerWorkItem(ToolName, InputFilename, Context, TM, ReduceModeMIR);
   if (!OriginalProgram) {
     return 1;
   }
 
   StringRef OutputFilename;
   bool OutputBitcode;
   std::tie(OutputFilename, OutputBitcode) =
       determineOutputType(ReduceModeMIR, InputIsBitcode);
 
   // Initialize test environment
   TestRunner Tester(TestFilename, TestArguments, std::move(OriginalProgram),
-                    std::move(TM), Argv[0], OutputFilename, InputIsBitcode,
+                    std::move(TM), ToolName, OutputFilename, InputIsBitcode,
                     OutputBitcode);
 
   // This parses and writes out the testcase into a temporary file copy for the
   // test, rather than evaluating the source IR directly. This is for the
   // convenience of lit tests; the stripped out comments may have broken the
   // interestingness checks.
   if (!isReduced(Tester.getProgram(), Tester, std::atomic<bool>())) {
     errs() << "\nInput isn't interesting! Verify interesting-ness test\n";