[LLVM] Merge -preserve-ll-uselistorder/preserve-bc-uselistorder into -preserve-uselistorder

[aeubanks]

Summary

It seems unnecessary to have separate flags for assembly vs bitcode.

[dexonsmith]

Separate flags allows vendors (and the default!) to have use-list order on-by-default in bitcode and off-by-default in textual IR.

IMO, the best path to simplification here:

1. Make it illegal or impossible to observe the use-lists of ConstantData (most importantly, ConstantInt and ConstantFloat). E.g., change replaceAllUsesWith() to crash for those types and require people to check first.
2. Stop serializing the use-lists of ConstantData (since we know there are no observers). This removes 99% of (a) the space overhead in bitcode and (b) the noise in textual assembly, since we're no longer tracking the use-list of i1 0, i1 1, i8 0, etc.
3. Consider merging the two options and flipping them both on by default since the noise in the textual IR is no longer terrible?
4. Consider removing -preserve-bc-use-list-order and just turn it on all the time since it no longer has major overhead?
5. Consider removing -preserve-ll-use-list-order and just turn it on all the time since it no longer adds undue noise to the IR?

Note that walking the uses of i1 0 (or similar) is dangerous since it's operating on all functions/modules/etc. in the LLVMContext. Any code that's doing this should probably be rewritten for other soundness reasons.

I used to have a series of patches to the above effect; there were only a few more test failures to fix before I could land it. Unfortunately I dropped the work since I got busy with something else. Here were some of the fixes I landed:

• r263853 / c3fa1eded258aa38a30911af6602892846efd6d1 (this one was pretty bad)
• r282320 / c82c11428e0ab2ff58a342ca3a615315929a2633
• r282333 / b4798739127cd7748177ac506e6d45cc8cf5e4f8
• r282334 / 4fd9b7e16f98173d730ac1a347f7eae96a9fd2db
• r282337 / b1b208a1f514ccb90b5afba0af49d6aa39dee21b
• r282338 / 11c06ea55a060d5460f716b00ca593e5dd829b17

llvm/tools/opt/opt.cpp

217

Notice that bitcode defaults to on...

222

... but textual assembly defaults to off.

[dexonsmith]

Here's a link to a related RFC:
https://discourse.llvm.org/t/rfc-constantdata-should-not-have-use-lists/42606

Patch 0003 removed use-lists from ConstantData; likely no longer applies, but possibly not too hard to rebase.

At the time, @mehdi_amini and @chandlerc both favoured a subsequent follow up to change the type hierarchy. I agree! But it's not necessary for the initial step.

llvm/test/Analysis/MemorySSA/nondeterminism.ll

-; RUN: opt -simplifycfg -S --preserve-ll-uselistorder %s | FileCheck %s
+; RUN: opt -simplifycfg -S --preserve-uselistorder %s | FileCheck %s
 ; REQUIRES: x86-registered-target
 ; CHECK-LABEL: @n
 ; CHECK: uselistorder i16 0, { 3, 2, 4, 1, 5, 0, 6 }
 
 ; Note: test was added in an effort to ensure determinism when updating memoryssa. See PR42574.
 ; If the uselistorder check becomes no longer relevant, the test can be disabled or removed.
 
 %rec9 = type { i16, i32, i32 }

llvm/test/Assembler/uselistorder_global.ll

-; RUN: opt -S -preserve-ll-uselistorder < %s | FileCheck %s
+; RUN: opt -S -preserve-uselistorder < %s | FileCheck %s
 ; RUN: verify-uselistorder %s
 
 ; CHECK: @g = external global i32
 ; CHECK: define void @func1() {
 ; CHECK-NOT: uselistorder
 ; CHECK: }
 ; CHECK: define void @func2() {
 ; CHECK-NOT: uselistorder

llvm/test/Transforms/ADCE/blocks-with-dead-term-nondeterministic.ll

-; RUN: opt < %s -passes=adce --preserve-ll-uselistorder -S | FileCheck %s
+; RUN: opt < %s -passes=adce --preserve-uselistorder -S | FileCheck %s
 
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.10.0"
 
 ; CHECK: uselistorder label %bb16, { 1, 0 }
 ; Function Attrs: noinline nounwind ssp uwtable
 define void @ham() local_unnamed_addr #0 {
 bb:

llvm/tools/bugpoint/OptimizerDriver.cpp

 using namespace llvm;
 
 #define DEBUG_TYPE "bugpoint"
 
 namespace llvm {
 extern cl::opt<std::string> OutputPrefix;
 }
 
-static cl::opt<bool> PreserveBitcodeUseListOrder(
-    "preserve-bc-uselistorder",
-    cl::desc("Preserve use-list order when writing LLVM bitcode."),
-    cl::init(true), cl::Hidden);
+static cl::opt<bool> PreserveUseListOrder(
+    "preserve-uselistorder",
+    cl::desc("Preserve use-list order when writing LLVM IR."), cl::init(true),
+    cl::Hidden);
 
 static cl::opt<std::string>
     OptCmd("opt-command", cl::init(""),
            cl::desc("Path to opt. (default: search path "
                     "for 'opt'.)"));
 
 /// This writes the current "Program" to the named bitcode file.  If an error
 /// occurs, true is returned.
 static bool writeProgramToFileAux(ToolOutputFile &Out, const Module &M) {
-  WriteBitcodeToFile(M, Out.os(), PreserveBitcodeUseListOrder);
+  WriteBitcodeToFile(M, Out.os(), PreserveUseListOrder);
   Out.os().close();
   if (!Out.os().has_error()) {
     Out.keep();
     return false;
   }
   return true;
 }
 
 bool BugDriver::writeProgramToFile(const std::string &Filename, int FD,
                                    const Module &M) const {
   ToolOutputFile Out(Filename, FD);
   return writeProgramToFileAux(Out, M);
 }
 
 bool BugDriver::writeProgramToFile(int FD, const Module &M) const {
   raw_fd_ostream OS(FD, /*shouldClose*/ false);
-  WriteBitcodeToFile(M, OS, PreserveBitcodeUseListOrder);
+  WriteBitcodeToFile(M, OS, PreserveUseListOrder);
   OS.flush();
   if (!OS.has_error())
     return false;
   OS.clear_error();
   return true;
 }
 
 bool BugDriver::writeProgramToFile(const std::string &Filename,
if (!Temp) {
     errs() << getToolName()
            << ": Error making unique filename: " << toString(Temp.takeError())
            << "\n";
     return true;
   }
   DiscardTemp Discard{*Temp};
   raw_fd_ostream OS(Temp->FD, /*shouldClose*/ false);
 
-  WriteBitcodeToFile(Program, OS, PreserveBitcodeUseListOrder);
+  WriteBitcodeToFile(Program, OS, PreserveUseListOrder);
   OS.flush();
   if (OS.has_error()) {
     errs() << "Error writing bitcode file: " << Temp->TmpName << "\n";
     OS.clear_error();
     return true;
   }
 
   std::string tool = OptCmd;

llvm/tools/llvm-as/llvm-as.cpp

 static cl::opt<bool> DumpAsm("d", cl::desc("Print assembly as parsed"),
                              cl::Hidden, cl::cat(AsCat));
 
 static cl::opt<bool>
     DisableVerify("disable-verify", cl::Hidden,
                   cl::desc("Do not run verifier on input LLVM (dangerous!)"),
                   cl::cat(AsCat));
 
-static cl::opt<bool> PreserveBitcodeUseListOrder(
-    "preserve-bc-uselistorder",
-    cl::desc("Preserve use-list order when writing LLVM bitcode."),
-    cl::init(true), cl::Hidden, cl::cat(AsCat));
+static cl::opt<bool> PreserveUseListOrder(
+    "preserve-uselistorder",
+    cl::desc("Preserve use-list order when writing LLVM IR."), cl::init(true),
+    cl::Hidden, cl::cat(AsCat));
 
 static cl::opt<std::string> ClDataLayout("data-layout",
                                          cl::desc("data layout string to use"),
                                          cl::value_desc("layout-string"),
                                          cl::init(""), cl::cat(AsCat));
 
 static void WriteOutputFile(const Module *M, const ModuleSummaryIndex *Index) {
   // Infer the output filename if needed.
if (Force || !CheckBitcodeOutputToConsole(Out->os())) {
     // summary section.
     if (Index && (Index->begin() != Index->end() || Index->getFlags()))
       IndexToWrite = Index;
     if (!IndexToWrite || (M && (!M->empty() || !M->global_empty())))
       // If we have a non-empty Module, then we write the Module plus
       // any non-null Index along with it as a per-module Index.
       // If both are empty, this will give an empty module block, which is
       // the expected behavior.
-      WriteBitcodeToFile(*M, Out->os(), PreserveBitcodeUseListOrder,
-                         IndexToWrite, EmitModuleHash);
+      WriteBitcodeToFile(*M, Out->os(), PreserveUseListOrder, IndexToWrite,
+                         EmitModuleHash);
     else
       // Otherwise, with an empty Module but non-empty Index, we write a
       // combined index.
       writeIndexToFile(*IndexToWrite, Out->os());
   }
 
   // Declare success.
   Out->keep();

llvm/tools/llvm-dis/llvm-dis.cpp

SetImporting("set-importing",
                  cl::desc("Set lazy loading to pretend to import a module"),
                  cl::Hidden, cl::cat(DisCategory));
 
 static cl::opt<bool>
     ShowAnnotations("show-annotations",
                     cl::desc("Add informational comments to the .ll file"),
                     cl::cat(DisCategory));
 
-static cl::opt<bool> PreserveAssemblyUseListOrder(
-    "preserve-ll-uselistorder",
-    cl::desc("Preserve use-list order when writing LLVM assembly."),
-    cl::init(false), cl::Hidden, cl::cat(DisCategory));
+static cl::opt<bool> PreserveUseListOrder(
+    "preserve-uselistorder",
+    cl::desc("Preserve use-list order when writing LLVM IR."), cl::init(false),
+    cl::Hidden, cl::cat(DisCategory));
 
 static cl::opt<bool>
     MaterializeMetadata("materialize-metadata",
                         cl::desc("Load module without materializing metadata, "
                                  "then materialize only the metadata"),
                         cl::cat(DisCategory));
 
 static cl::opt<bool> PrintThinLTOIndexOnly(
for (size_t I = 0; I < N; ++I) {
 
       std::unique_ptr<AssemblyAnnotationWriter> Annotator;
       if (ShowAnnotations)
         Annotator.reset(new CommentWriter());
 
       // All that llvm-dis does is write the assembly to a file.
       if (!DontPrint) {
         if (M)
-          M->print(Out->os(), Annotator.get(), PreserveAssemblyUseListOrder);
+          M->print(Out->os(), Annotator.get(), PreserveUseListOrder);
         if (Index)
           Index->print(Out->os());
       }
 
       // Declare success.
       Out->keep();
     }
   }
 
   return 0;
 }

llvm/tools/llvm-extract/llvm-extract.cpp

ExtractRegExpGlobals("rglob",
                          cl::desc("Specify global(s) to extract using a "
                                   "regular expression"),
                          cl::value_desc("rglobal"), cl::cat(ExtractCat));
 
 static cl::opt<bool> OutputAssembly("S",
                                     cl::desc("Write output as LLVM assembly"),
                                     cl::Hidden, cl::cat(ExtractCat));
 
-static cl::opt<bool> PreserveBitcodeUseListOrder(
-    "preserve-bc-uselistorder",
-    cl::desc("Preserve use-list order when writing LLVM bitcode."),
-    cl::init(true), cl::Hidden, cl::cat(ExtractCat));
-
-static cl::opt<bool> PreserveAssemblyUseListOrder(
-    "preserve-ll-uselistorder",
-    cl::desc("Preserve use-list order when writing LLVM assembly."),
+static cl::opt<bool> PreserveUseListOrder(
+    "preserve-uselistorder",
+    cl::desc("Preserve use-list order when writing LLVM IR."),
     cl::init(false), cl::Hidden, cl::cat(ExtractCat));
 
 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
 
   LLVMContext Context;
   cl::HideUnrelatedOptions(ExtractCat);
   cl::ParseCommandLineOptions(argc, argv, "llvm extractor\n");
int main(int argc, char **argv) {
   ToolOutputFile Out(OutputFilename, EC, sys::fs::OF_None);
   if (EC) {
     errs() << EC.message() << '\n';
     return 1;
   }
 
   if (OutputAssembly)
     Passes.add(
-        createPrintModulePass(Out.os(), "", PreserveAssemblyUseListOrder));
+        createPrintModulePass(Out.os(), "", PreserveUseListOrder));
   else if (Force || !CheckBitcodeOutputToConsole(Out.os()))
-    Passes.add(createBitcodeWriterPass(Out.os(), PreserveBitcodeUseListOrder));
+    Passes.add(createBitcodeWriterPass(Out.os(), PreserveUseListOrder));
 
   Passes.run(*M.get());
 
   // Declare success.
   Out.keep();
 
   return 0;
 }

llvm/tools/llvm-link/llvm-link.cpp

 
 static cl::opt<bool> DumpAsm("d", cl::desc("Print assembly as linked"),
                              cl::Hidden, cl::cat(LinkCategory));
 
 static cl::opt<bool> SuppressWarnings("suppress-warnings",
                                       cl::desc("Suppress all linking warnings"),
                                       cl::init(false), cl::cat(LinkCategory));
 
-static cl::opt<bool> PreserveBitcodeUseListOrder(
-    "preserve-bc-uselistorder",
-    cl::desc("Preserve use-list order when writing LLVM bitcode."),
-    cl::init(true), cl::Hidden, cl::cat(LinkCategory));
-
-static cl::opt<bool> PreserveAssemblyUseListOrder(
-    "preserve-ll-uselistorder",
-    cl::desc("Preserve use-list order when writing LLVM assembly."),
+static cl::opt<bool> PreserveUseListOrder(
+    "preserve-uselistorder",
+    cl::desc("Preserve use-list order when writing LLVM IR."),
     cl::init(false), cl::Hidden, cl::cat(LinkCategory));
 
 static cl::opt<bool> NoVerify("disable-verify",
                               cl::desc("Do not run the verifier"), cl::Hidden,
                               cl::cat(LinkCategory));
 
 static cl::opt<bool> IgnoreNonBitcode(
     "ignore-non-bitcode",
if (!NoVerify && verifyModule(*Composite, &errs())) {
     errs() << argv[0] << ": ";
     WithColor::error() << "linked module is broken!\n";
     return 1;
   }
 
   if (Verbose)
     errs() << "Writing bitcode...\n";
   if (OutputAssembly) {
-    Composite->print(Out.os(), nullptr, PreserveAssemblyUseListOrder);
+    Composite->print(Out.os(), nullptr, PreserveUseListOrder);
   } else if (Force || !CheckBitcodeOutputToConsole(Out.os()))
-    WriteBitcodeToFile(*Composite, Out.os(), PreserveBitcodeUseListOrder);
+    WriteBitcodeToFile(*Composite, Out.os(), PreserveUseListOrder);
 
   // Declare success.
   Out.keep();
 
   return 0;
 }

llvm/tools/opt/opt.cpp

 PrintBreakpoints("print-breakpoints-for-testing",
                  cl::desc("Print select breakpoints location for testing"));
 
 static cl::opt<std::string> ClDataLayout("data-layout",
                                          cl::desc("data layout string to use"),
                                          cl::value_desc("layout-string"),
                                          cl::init(""));
 
-static cl::opt<bool> PreserveBitcodeUseListOrder(
-    "preserve-bc-uselistorder",
-    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."),
+static cl::opt<bool> PreserveUseListOrder(
+    "preserve-uselistorder",
+    cl::desc("Preserve use-list order when writing LLVM IR."),
     cl::init(false), cl::Hidden);
 
 static cl::opt<bool> RunTwice("run-twice",
                               cl::desc("Run all passes twice, re-using the "
                                        "same pass manager (legacy PM only)."),
                               cl::init(false), cl::Hidden);
 
 static cl::opt<bool> DiscardValueNames(
     "discard-value-names",
if (UseNPM) {
     else if (VerifyEach)
       VK = VK_VerifyEachPass;
 
     // The user has asked to use the new pass manager and provided a pipeline
     // string. Hand off the rest of the functionality to the new code for that
     // layer.
     return runPassPipeline(argv[0], *M, TM.get(), &TLII, Out.get(),
                            ThinLinkOut.get(), RemarksFile.get(), Pipeline,
-                           Passes, PluginList, OK, VK, PreserveAssemblyUseListOrder,
-                           PreserveBitcodeUseListOrder, EmitSummaryIndex,
+                           Passes, PluginList, OK, VK, PreserveUseListOrder,
+                           PreserveUseListOrder, EmitSummaryIndex,
                            EmitModuleHash, EnableDebugify,
                            VerifyDebugInfoPreserve)
                ? 0
                : 1;
   }
 
   if (OptLevelO0 || OptLevelO1 || OptLevelO2 || OptLevelOs || OptLevelOz ||
       OptLevelO3) {
#endif
   if (ShouldEmitOutput || RunTwice) {
     assert(Out);
     OS = &Out->os();
     if (RunTwice) {
       BOS = std::make_unique<raw_svector_ostream>(Buffer);
       OS = BOS.get();
     }
     if (OutputAssembly)
-      Passes.add(createPrintModulePass(*OS, "", PreserveAssemblyUseListOrder));
+      Passes.add(createPrintModulePass(*OS, "", PreserveUseListOrder));
     else
-      Passes.add(createBitcodeWriterPass(*OS, PreserveBitcodeUseListOrder));
+      Passes.add(createBitcodeWriterPass(*OS, PreserveUseListOrder));
   }
 
   // Before executing passes, print the final values of the LLVM options.
   cl::PrintOptionValues();
 
   if (!RunTwice) {
     // Now that we have all of the passes ready, run them.
     Passes.run(*M);