-
Notifications
You must be signed in to change notification settings - Fork 14.8k
Move the preserve-{bc,ll}-uselistorder options out of individual tools, make them global defaults for AsmWriter and BitcodeWriter #160079
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
static cl::opt<bool> PreserveBitcodeUseListOrder( | ||
"preserve-bc-uselistorder", | ||
cl::desc("Preserve use-list order when writing LLVM bitcode."), | ||
cl::init(false), cl::Hidden); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be true? Looks like all the old options defaulted the bitcode case to true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, -preserve-bc-uselistorder
defaults to true in all individual tools.
But in llvm::WriteBitcodeToFile
, parameter ShouldPreserveUseListOrder
defaults to false. https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Bitcode/BitcodeWriter.h#L134
To make global -preserve-bc-uselistorder
taking precedence, ShouldPreserveUseListOrder || PreserveBitcodeUseListOrder
is used in ModuleBitcodeWriterBase
's constructor,
so this has to be false here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this makes it effectively impossible to set -preserve-bc-uselistorder=0
, as the explicit argument will take precedence.
I think the way this needs to work is that the argument is a std::optional<bool>
where true/false forces one or the other and nullopt makes it use the option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Appreciate the feedback.
I've updated to use std::optional<bool>
for -preserve-bc-uselistorder
and -preserve-ll-uselistorder
as suggested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw there is cl::boolOrDefault
which is widely used than cl::opt<std::optional<bool>>
, not sure which way do you prefer :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There shouldn't be a need to use an actual std::optional<bool>
with a custom parser. The standard pattern for this is something like Opt.getNumOccurences() ? Opt : Default
.
We should probably add some kind of get_or
helper to cl::opt, given how common this is...
(My std::optional suggestion for for making the function parameters optional, but your approach of making the option "optional" instead works as well.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
Updated to use Opt.getNumOccurences() ? Opt : Default
for preserve-ll-uselistorder
and preserve-bc-uselistorder
as suggested.
@llvm/pr-subscribers-llvm-ir Author: Mingjie Xu (Enna1) ChangesThis patch moves the These options are useful when we use Full diff: https://github.com/llvm/llvm-project/pull/160079.diff 8 Files Affected:
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index c4070e1f44688..c3113890cad81 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -118,6 +118,11 @@ static cl::opt<bool>
#endif
cl::desc(""));
+static cl::opt<bool> PreserveBitcodeUseListOrder(
+ "preserve-bc-uselistorder",
+ cl::desc("Preserve use-list order when writing LLVM bitcode."),
+ cl::init(false), cl::Hidden);
+
namespace llvm {
extern FunctionSummary::ForceSummaryHotnessType ForceSummaryEdgesCold;
}
@@ -217,7 +222,8 @@ class ModuleBitcodeWriterBase : public BitcodeWriterBase {
bool ShouldPreserveUseListOrder,
const ModuleSummaryIndex *Index)
: BitcodeWriterBase(Stream, StrtabBuilder), M(M),
- VE(M, ShouldPreserveUseListOrder), Index(Index) {
+ VE(M, ShouldPreserveUseListOrder || PreserveBitcodeUseListOrder),
+ Index(Index) {
// Assign ValueIds to any callee values in the index that came from
// indirect call profiles and were recorded as a GUID not a Value*
// (which would have been assigned an ID by the ValueEnumerator).
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 60e86d4d2b5a5..6e45f25797fa7 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -102,6 +102,11 @@ static cl::opt<bool> PrintProfData(
"print-prof-data", cl::Hidden,
cl::desc("Pretty print perf data (branch weights, etc) when dumping"));
+static cl::opt<bool> PreserveAssemblyUseListOrder(
+ "preserve-ll-uselistorder",
+ cl::desc("Preserve use-list order when writing LLVM assembly."),
+ cl::init(false), cl::Hidden);
+
// Make virtual table appear in this compilation unit.
AssemblyAnnotationWriter::~AssemblyAnnotationWriter() = default;
@@ -2972,7 +2977,8 @@ AssemblyWriter::AssemblyWriter(formatted_raw_ostream &o, SlotTracker &Mac,
bool IsForDebug, bool ShouldPreserveUseListOrder)
: Out(o), TheModule(M), Machine(Mac), TypePrinter(M), AnnotationWriter(AAW),
IsForDebug(IsForDebug),
- ShouldPreserveUseListOrder(ShouldPreserveUseListOrder) {
+ ShouldPreserveUseListOrder(ShouldPreserveUseListOrder ||
+ PreserveAssemblyUseListOrder) {
if (!TheModule)
return;
for (const GlobalObject &GO : TheModule->global_objects())
@@ -2983,7 +2989,8 @@ AssemblyWriter::AssemblyWriter(formatted_raw_ostream &o, SlotTracker &Mac,
AssemblyWriter::AssemblyWriter(formatted_raw_ostream &o, SlotTracker &Mac,
const ModuleSummaryIndex *Index, bool IsForDebug)
: Out(o), TheIndex(Index), Machine(Mac), TypePrinter(/*Module=*/nullptr),
- IsForDebug(IsForDebug), ShouldPreserveUseListOrder(false) {}
+ IsForDebug(IsForDebug),
+ ShouldPreserveUseListOrder(PreserveAssemblyUseListOrder) {}
void AssemblyWriter::writeOperand(const Value *Operand, bool PrintType) {
if (!Operand) {
diff --git a/llvm/tools/bugpoint/OptimizerDriver.cpp b/llvm/tools/bugpoint/OptimizerDriver.cpp
index 56a0fa4d5ec9e..3daacfdb99b09 100644
--- a/llvm/tools/bugpoint/OptimizerDriver.cpp
+++ b/llvm/tools/bugpoint/OptimizerDriver.cpp
@@ -38,11 +38,6 @@ 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<std::string>
OptCmd("opt-command", cl::init(""),
cl::desc("Path to opt. (default: search path "
@@ -51,7 +46,7 @@ static cl::opt<std::string>
/// 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(), /* ShouldPreserveUseListOrder */ true);
Out.os().close();
if (!Out.os().has_error()) {
Out.keep();
@@ -68,7 +63,7 @@ bool BugDriver::writeProgramToFile(const std::string &Filename, int FD,
bool BugDriver::writeProgramToFile(int FD, const Module &M) const {
raw_fd_ostream OS(FD, /*shouldClose*/ false);
- WriteBitcodeToFile(M, OS, PreserveBitcodeUseListOrder);
+ WriteBitcodeToFile(M, OS, /* ShouldPreserveUseListOrder */ true);
OS.flush();
if (!OS.has_error())
return false;
@@ -155,7 +150,7 @@ bool BugDriver::runPasses(Module &Program,
DiscardTemp Discard{*Temp};
raw_fd_ostream OS(Temp->FD, /*shouldClose*/ false);
- WriteBitcodeToFile(Program, OS, PreserveBitcodeUseListOrder);
+ WriteBitcodeToFile(Program, OS, /* ShouldPreserveUseListOrder */ true);
OS.flush();
if (OS.has_error()) {
errs() << "Error writing bitcode file: " << Temp->TmpName << "\n";
diff --git a/llvm/tools/llvm-as/llvm-as.cpp b/llvm/tools/llvm-as/llvm-as.cpp
index 21648674b51f1..200e6a5c564da 100644
--- a/llvm/tools/llvm-as/llvm-as.cpp
+++ b/llvm/tools/llvm-as/llvm-as.cpp
@@ -57,11 +57,6 @@ static cl::opt<bool>
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<std::string> ClDataLayout("data-layout",
cl::desc("data layout string to use"),
cl::value_desc("layout-string"),
@@ -100,7 +95,7 @@ static void WriteOutputFile(const Module *M, const ModuleSummaryIndex *Index) {
// 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,
+ WriteBitcodeToFile(*M, Out->os(), /* ShouldPreserveUseListOrder */ true,
IndexToWrite, EmitModuleHash);
else
// Otherwise, with an empty Module but non-empty Index, we write a
diff --git a/llvm/tools/llvm-dis/llvm-dis.cpp b/llvm/tools/llvm-dis/llvm-dis.cpp
index 2b43d27f292a0..35c540963a487 100644
--- a/llvm/tools/llvm-dis/llvm-dis.cpp
+++ b/llvm/tools/llvm-dis/llvm-dis.cpp
@@ -80,11 +80,6 @@ static cl::opt<bool>
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>
MaterializeMetadata("materialize-metadata",
cl::desc("Load module without materializing metadata, "
@@ -255,7 +250,8 @@ int main(int argc, char **argv) {
if (!DontPrint) {
if (M) {
M->removeDebugIntrinsicDeclarations();
- M->print(Out->os(), Annotator.get(), PreserveAssemblyUseListOrder);
+ M->print(Out->os(), Annotator.get(),
+ /* ShouldPreserveUseListOrder */ false);
}
if (Index)
Index->print(Out->os());
diff --git a/llvm/tools/llvm-extract/llvm-extract.cpp b/llvm/tools/llvm-extract/llvm-extract.cpp
index 69636ca018dcb..439a4a48b350a 100644
--- a/llvm/tools/llvm-extract/llvm-extract.cpp
+++ b/llvm/tools/llvm-extract/llvm-extract.cpp
@@ -129,16 +129,6 @@ 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."),
- cl::init(false), cl::Hidden, cl::cat(ExtractCat));
-
int main(int argc, char **argv) {
InitLLVM X(argc, argv);
@@ -421,9 +411,11 @@ int main(int argc, char **argv) {
}
if (OutputAssembly)
- PM.addPass(PrintModulePass(Out.os(), "", PreserveAssemblyUseListOrder));
+ PM.addPass(
+ PrintModulePass(Out.os(), "", /* ShouldPreserveUseListOrder */ false));
else if (Force || !CheckBitcodeOutputToConsole(Out.os()))
- PM.addPass(BitcodeWriterPass(Out.os(), PreserveBitcodeUseListOrder));
+ PM.addPass(
+ BitcodeWriterPass(Out.os(), /* ShouldPreserveUseListOrder */ true));
PM.run(*M, MAM);
diff --git a/llvm/tools/llvm-link/llvm-link.cpp b/llvm/tools/llvm-link/llvm-link.cpp
index 22ea54e68358a..93b1fb6031cec 100644
--- a/llvm/tools/llvm-link/llvm-link.cpp
+++ b/llvm/tools/llvm-link/llvm-link.cpp
@@ -110,16 +110,6 @@ 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."),
- 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));
@@ -525,9 +515,10 @@ int main(int argc, char **argv) {
errs() << "Writing bitcode...\n";
Composite->removeDebugIntrinsicDeclarations();
if (OutputAssembly) {
- Composite->print(Out.os(), nullptr, PreserveAssemblyUseListOrder);
+ Composite->print(Out.os(), nullptr, /* ShouldPreserveUseListOrder */ false);
} else if (Force || !CheckBitcodeOutputToConsole(Out.os())) {
- WriteBitcodeToFile(*Composite, Out.os(), PreserveBitcodeUseListOrder);
+ WriteBitcodeToFile(*Composite, Out.os(),
+ /* ShouldPreserveUseListOrder */ true);
}
// Declare success.
diff --git a/llvm/tools/opt/optdriver.cpp b/llvm/tools/opt/optdriver.cpp
index 26902b213571f..fcc60efe58d31 100644
--- a/llvm/tools/opt/optdriver.cpp
+++ b/llvm/tools/opt/optdriver.cpp
@@ -232,16 +232,6 @@ static cl::opt<std::string> ClDataLayout("data-layout",
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."),
- 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)."),
@@ -753,9 +743,9 @@ extern "C" int optMain(
return runPassPipeline(
argv[0], *M, TM.get(), &TLII, Out.get(), ThinLinkOut.get(),
RemarksFile.get(), Pipeline, PluginList, PassBuilderCallbacks,
- OK, VK, PreserveAssemblyUseListOrder,
- PreserveBitcodeUseListOrder, EmitSummaryIndex, EmitModuleHash,
- EnableDebugify, VerifyDebugInfoPreserve,
+ OK, VK, /* ShouldPreserveAssemblyUseListOrder */ false,
+ /* ShouldPreserveBitcodeUseListOrder */ true, EmitSummaryIndex,
+ EmitModuleHash, EnableDebugify, VerifyDebugInfoPreserve,
EnableProfileVerification, UnifiedLTO)
? 0
: 1;
@@ -877,9 +867,11 @@ extern "C" int optMain(
OS = BOS.get();
}
if (OutputAssembly)
- Passes.add(createPrintModulePass(*OS, "", PreserveAssemblyUseListOrder));
+ Passes.add(createPrintModulePass(
+ *OS, "", /* ShouldPreserveAssemblyUseListOrder */ false));
else
- Passes.add(createBitcodeWriterPass(*OS, PreserveBitcodeUseListOrder));
+ Passes.add(createBitcodeWriterPass(
+ *OS, /* ShouldPreserveBitcodeUseListOrder */ true));
}
// Before executing passes, print the final values of the LLVM options.
|
…s and make them global defaults for AsmWriter and BitcodeWriter This patch moves the `preserve-bc-uselistorder` and `preserve-ll-uselistorder` options out of individual tools(opt, llvm-as, llvm-dis, llvm-link, llvm-extract) and make them global defaults for AsmWriter and BitcodeWriter. These options are useful when we use `-print-*` options to dump LLVM IR.
…order options" This reverts commit 5b05667.
7b09bb9
to
4b541fc
Compare
llvm/lib/IR/AsmWriter.cpp
Outdated
ShouldPreserveUseListOrder( | ||
PreserveAssemblyUseListOrder.getNumOccurrences() | ||
? PreserveAssemblyUseListOrder | ||
: false) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can directly use PreserveAssemblyUseListOrder, as false is already the default.
…s, make them global defaults for AsmWriter and BitcodeWriter (llvm#160079) This patch moves the `preserve-bc-uselistorder` and `preserve-ll-uselistorder` options out of individual tools(opt, llvm-as, llvm-dis, llvm-link, llvm-extract) and make them global defaults for AsmWriter and BitcodeWriter. These options are useful when we use `-print-*` options to dump LLVM IR.
This patch moves the
preserve-bc-uselistorder
andpreserve-ll-uselistorder
options out of individual tools(opt, llvm-as, llvm-dis, llvm-link, llvm-extract) and make them global defaults for AsmWriter and BitcodeWriter.These options are useful when we use
-print-*
options to dump LLVM IR.