Remove nasty environment variables in MachineVerifier in favor of a build option

[resistor]

Summary

This patch removes two uses of getenv() to read environment variables related to the MachineVerifier. Instead, it creates a new CMake build variable LLVM_ENABLE_MACHINE_VERIFIER that can be used to achieve the same purpose. There's a little bit of CMake cleverness involved here to minimize the amount of rebuilding required when toggling the flag on or off, which is important in my envisioned use case of staged builders.

[echristo]

FWIW the autoconf changes for this should also be trivial :)

[resistor]

Patch-patches welcome. :-p

—Owen

[chandlerc]

So, I have no problem with this patch or this design. It seems like a nice cleanup.

However, I'll float another idea:

What about adding a getenv call to lit (and maybe a flag) to flip this flag? No rebuild required, etc. Thoughts?

lib/CodeGen/MachineVerifier.cpp

278

Why not just set this in the constructor (perhaps from a defaulted argument)? It can also be a reference then.

[mehdi_amini]

This is something I'd like!
Actually I was thinking that lit could replace "llc" with "llc --verify-machineinstrs" on the RUN line. Do you prefer to keep an environment variable inside LLVM?

[resistor]

Abandoning this, as Chandler pointed out a better approach on IRC. The patch coming soon.

CMakeLists.txt

 option(LLVM_ENABLE_LIBCXX "Use libc++ if available." OFF)
 option(LLVM_ENABLE_PEDANTIC "Compile with pedantic enabled." ON)
 option(LLVM_ENABLE_WERROR "Fail and stop if a warning is triggered." OFF)
+option(LLVM_ENABLE_MACHINE_VERIFIER "Enable the LLVM machine verifier by default" OFF)
 
 if( NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
   option(LLVM_ENABLE_ASSERTIONS "Enable assertions" OFF)

lib/CodeGen/CMakeLists.txt

 
 add_subdirectory(SelectionDAG)
 add_subdirectory(AsmPrinter)
+
+if (LLVM_ENABLE_MACHINE_VERIFIER)
+  set_property(SOURCE Passes.cpp APPEND_STRING
+    PROPERTY COMPILE_DEFINITIONS "LLVM_ENABLE_MACHINE_VERIFIER")
+endif()
+

lib/CodeGen/MachineVerifier.cpp

 
     MachineVerifier(Pass *pass, const char *b) :
       PASS(pass),
-      Banner(b),
-      OutFileName(getenv("LLVM_VERIFY_MACHINEINSTRS"))
+      Banner(b)
       {}
 
     bool runOnMachineFunction(MachineFunction &MF);
 
     Pass *const PASS;
     const char *Banner;
-    const char *const OutFileName;
     raw_ostream *OS;
     const MachineFunction *MF;
     const TargetMachine *TM;

 }
 
 bool MachineVerifier::runOnMachineFunction(MachineFunction &MF) {
-  raw_ostream *OutFile = nullptr;
-  if (OutFileName) {
-    std::error_code EC;
-    OutFile = new raw_fd_ostream(OutFileName, EC,
-                                 sys::fs::F_Append | sys::fs::F_Text);
-    if (EC) {
-      errs() << "Error opening '" << OutFileName << "': " << EC.message()
-             << '\n';
-      exit(1);
-    }
-
-    OS = OutFile;
-  } else {
-    OS = &errs();
-  }
+  OS = &errs();
 
   foundErrors = 0;
 

   }
   visitMachineFunctionAfter();
 
-  if (OutFile)
-    delete OutFile;
-  else if (foundErrors)
+  if (foundErrors)
     report_fatal_error("Found "+Twine(foundErrors)+" machine code errors.");
 
   // Clean up.

lib/CodeGen/Passes.cpp

     cl::desc("Dump garbage collector data"));
 static cl::opt<bool> VerifyMachineCode("verify-machineinstrs", cl::Hidden,
     cl::desc("Verify generated machine code"),
-    cl::init(getenv("LLVM_VERIFY_MACHINEINSTRS")!=nullptr));
+#ifdef LLVM_ENABLE_MACHINE_VERIFIER
+    cl::init(true));
+#else
+    cl::init(false));
+#endif
+
 static cl::opt<std::string>
 PrintMachineInstrs("print-machineinstrs", cl::ValueOptional,
                    cl::desc("Print machine instrs"),