[llvm] r234921 - IR: Set -preserve-bc-uselistorder=false by default
Duncan P. N. Exon Smith
dexonsmith at apple.com
Tue Apr 14 20:22:40 PDT 2015
> On 2015-Apr-14, at 13:22, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Tue, Apr 14, 2015 at 1:18 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>
> > On 2015-Apr-14, at 12:41, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> >
> > On Tue, Apr 14, 2015 at 12:25 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> >
> > > On 2015-Apr-14, at 11:53, David Blaikie <dblaikie at gmail.com> wrote:
> > >
> > >
> > >
> > > On Tue, Apr 14, 2015 at 11:46 AM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> > >
> > > > On 2015-Apr-14, at 11:42, David Blaikie <dblaikie at gmail.com> wrote:
> > > >
> > > >
> > > >
> > > > On Tue, Apr 14, 2015 at 11:33 AM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> > > > Author: dexonsmith
> > > > Date: Tue Apr 14 13:33:00 2015
> > > > New Revision: 234921
> > > >
> > > > URL: http://llvm.org/viewvc/llvm-project?rev=234921&view=rev
> > > > Log:
> > > > IR: Set -preserve-bc-uselistorder=false by default
> > > >
> > > > But keep it on by default in `llvm-as`, `opt`, `bugpoint`, `llvm-link`,
> > > > `llvm-extract`, and `LTOCodeGenerator`.
> > > >
> > > > Hmm - what's LTOCodeGenerator used for? If it's a standard library utility used in production pipelines it might make more sense for it to default-off & users can opt-in when necessary?
> > >
> > > The usual flow for libLTO.dylib is "read bitcode, link, optimize, codegen,
> > > write object". Writing bitcode is used, e.g., by `ld -save-temps`.
> > >
> > > LTOCodeGenerator is only used to actually generating bitcode from LTO-merged modules?
> >
> > IIRC, that's the only time it outputs bitcode, so that's the only
> > place `-preserve-bc-uselistorder` will have any effect.
> >
> > I'm still a bit concerned about making that the default for what seems like a relatively common LLVM-as-a-library surface area.
>
> Is it common?
>
> Anyway, I don't understand the concern here. I thought we'd agreed
> that debug output should preserve use-lists?
>
> I think I'm still agreeing - I'm just not sure this is strictly debug output. It seems like a generic utility (& relatively accessible library surface area for LLVM) that I'd hesitate baking "this is a debug tool" assumptions into.
I just think bitcode output during LTO is debug output. I'll
see how hard it would be to hook up new API to ld64 though; if
it's easy enough then I don't mind exposing the bit through the
LTO C API.
>
>
> >
> > > Also, what's the deal with global state? (just realizing now that the "setPreserveBitcodeUseListOrder" is just a global function, not LLVMContext, etc) - that seems like the sort of thing I'd be even more inclined to push up into the actual utilities very directly, so there's no libraries quietly tweaking this global state in surprising ways.
> >
> > Wrong as it may be, this sort of thing is pervasive in LLVM.
> >
> > With varying degrees of "this is a hack & we'd like to remove it one day" depending on what it is exactly (eg: all the global state that we're trying to make module state for LTO semantics).
> >
> > But if you have specific ideas, I'm happy to tweak this.
> >
> > I'd imagine a parameter to "createBitcodeWriterPass" (& WriteBitcodeToFile) would be one way to avoid the global state in this instance.
>
> I'm surprised at how few callers there are. I'll have a go...
Done via the following:
r234956 = d01ee74ffad023328d7fc04c24238605e1935270
r234957 = 657aad675aaa9a9283809d4e7d357099ead3316a
r234958 = bff2c565dfa467b7717b4b6f94c18787d48c4fea
r234959 = e005d71e4b5447df00e4b8e17d14b1fbcbf29196
r234960 = 18aa9e9198e979b6d2d9a6087188cdcdbd29aea3 (clang)
r234962 = dce722961a8a421f2cfeb08d417834b4f008dc4e (clang)
r234966 = c9d5dea0fea11cc40a8b0a48865b4d572df8f080
r234968 = 8b376eb892a4667beb08a0cc0f68d370b99e10a5
r234969 = 2c7f24d654e2852a5005dc965a9a253476500cdf
r234970 = 8c10c7bfd12d6a2f8e4828312ab0f45b319c01da (clang)
r234972 = 4aba50f858f9c107c05b79a974b762c11accebac (clang)
r234973 = 8d61ee9e7df8ca3a3194dca4f1d16a5e731c1f07
>
> > > Not sure what sort of ideas on exceptions to the "avoid global state because we're a library" we have for these sort of things & how to limit their creep.
> > >
> > > - David
> >
> > I think Chris B. was working on fixing global state in `cl::opt`,
> > but I'm not sure where he is with that.
> >
> > *nod* that'd be one example/part of the sort of stuff I was alluding to.
> >
> > - David
> >
> >
> > >
> > > >
> > > > Part of PR5680.
> > > >
> > > > Modified:
> > > > llvm/trunk/lib/IR/UseListOrder.cpp
> > > > llvm/trunk/lib/LTO/LTOCodeGenerator.cpp
> > > > llvm/trunk/tools/bugpoint/bugpoint.cpp
> > > > llvm/trunk/tools/llvm-as/llvm-as.cpp
> > > > llvm/trunk/tools/llvm-extract/llvm-extract.cpp
> > > > llvm/trunk/tools/llvm-link/llvm-link.cpp
> > > > llvm/trunk/tools/opt/opt.cpp
> > > >
> > > > Modified: llvm/trunk/lib/IR/UseListOrder.cpp
> > > > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/UseListOrder.cpp?rev=234921&r1=234920&r2=234921&view=diff
> > > > ==============================================================================
> > > > --- llvm/trunk/lib/IR/UseListOrder.cpp (original)
> > > > +++ llvm/trunk/lib/IR/UseListOrder.cpp Tue Apr 14 13:33:00 2015
> > > > @@ -19,7 +19,7 @@ using namespace llvm;
> > > > static cl::opt<bool> PreserveBitcodeUseListOrder(
> > > > "preserve-bc-uselistorder",
> > > > cl::desc("Preserve use-list order when writing LLVM bitcode."),
> > > > - cl::init(true), cl::Hidden);
> > > > + cl::init(false), cl::Hidden);
> > > >
> > > > static cl::opt<bool> PreserveAssemblyUseListOrder(
> > > > "preserve-ll-uselistorder",
> > > >
> > > > Modified: llvm/trunk/lib/LTO/LTOCodeGenerator.cpp
> > > > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/LTOCodeGenerator.cpp?rev=234921&r1=234920&r2=234921&view=diff
> > > > ==============================================================================
> > > > --- llvm/trunk/lib/LTO/LTOCodeGenerator.cpp (original)
> > > > +++ llvm/trunk/lib/LTO/LTOCodeGenerator.cpp Tue Apr 14 13:33:00 2015
> > > > @@ -29,6 +29,7 @@
> > > > #include "llvm/IR/LegacyPassManager.h"
> > > > #include "llvm/IR/Mangler.h"
> > > > #include "llvm/IR/Module.h"
> > > > +#include "llvm/IR/UseListOrder.h"
> > > > #include "llvm/IR/Verifier.h"
> > > > #include "llvm/InitializePasses.h"
> > > > #include "llvm/LTO/LTOModule.h"
> > > > @@ -603,6 +604,10 @@ void LTOCodeGenerator::setCodeGenDebugOp
> > > > }
> > > >
> > > > void LTOCodeGenerator::parseCodeGenDebugOptions() {
> > > > + // Turn on -preserve-bc-uselistorder by default, but let the command-line
> > > > + // override it.
> > > > + setPreserveBitcodeUseListOrder(true);
> > > > +
> > > > // if options were requested, set them
> > > > if (!CodegenOptions.empty())
> > > > cl::ParseCommandLineOptions(CodegenOptions.size(),
> > > >
> > > > Modified: llvm/trunk/tools/bugpoint/bugpoint.cpp
> > > > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/bugpoint/bugpoint.cpp?rev=234921&r1=234920&r2=234921&view=diff
> > > > ==============================================================================
> > > > --- llvm/trunk/tools/bugpoint/bugpoint.cpp (original)
> > > > +++ llvm/trunk/tools/bugpoint/bugpoint.cpp Tue Apr 14 13:33:00 2015
> > > > @@ -18,6 +18,7 @@
> > > > #include "llvm/IR/LLVMContext.h"
> > > > #include "llvm/IR/LegacyPassManager.h"
> > > > #include "llvm/IR/LegacyPassNameParser.h"
> > > > +#include "llvm/IR/UseListOrder.h"
> > > > #include "llvm/LinkAllIR.h"
> > > > #include "llvm/LinkAllPasses.h"
> > > > #include "llvm/Support/CommandLine.h"
> > > > @@ -136,6 +137,10 @@ int main(int argc, char **argv) {
> > > > polly::initializePollyPasses(Registry);
> > > > #endif
> > > >
> > > > + // Turn on -preserve-bc-uselistorder by default, but let the command-line
> > > > + // override it.
> > > > + setPreserveBitcodeUseListOrder(true);
> > > > +
> > > > cl::ParseCommandLineOptions(argc, argv,
> > > > "LLVM automatic testcase reducer. See\nhttp://"
> > > > "llvm.org/cmds/bugpoint.html"
> > > >
> > > > Modified: llvm/trunk/tools/llvm-as/llvm-as.cpp
> > > > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-as/llvm-as.cpp?rev=234921&r1=234920&r2=234921&view=diff
> > > > ==============================================================================
> > > > --- llvm/trunk/tools/llvm-as/llvm-as.cpp (original)
> > > > +++ llvm/trunk/tools/llvm-as/llvm-as.cpp Tue Apr 14 13:33:00 2015
> > > > @@ -19,6 +19,7 @@
> > > > #include "llvm/AsmParser/Parser.h"
> > > > #include "llvm/Bitcode/ReaderWriter.h"
> > > > #include "llvm/IR/Module.h"
> > > > +#include "llvm/IR/UseListOrder.h"
> > > > #include "llvm/IR/Verifier.h"
> > > > #include "llvm/Support/CommandLine.h"
> > > > #include "llvm/Support/FileSystem.h"
> > > > @@ -90,6 +91,11 @@ int main(int argc, char **argv) {
> > > > PrettyStackTraceProgram X(argc, argv);
> > > > LLVMContext &Context = getGlobalContext();
> > > > llvm_shutdown_obj Y; // Call llvm_shutdown() on exit.
> > > > +
> > > > + // Turn on -preserve-bc-uselistorder by default, but let the command-line
> > > > + // override it.
> > > > + setPreserveBitcodeUseListOrder(true);
> > > > +
> > > > cl::ParseCommandLineOptions(argc, argv, "llvm .ll -> .bc assembler\n");
> > > >
> > > > // Parse the file now...
> > > >
> > > > Modified: llvm/trunk/tools/llvm-extract/llvm-extract.cpp
> > > > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-extract/llvm-extract.cpp?rev=234921&r1=234920&r2=234921&view=diff
> > > > ==============================================================================
> > > > --- llvm/trunk/tools/llvm-extract/llvm-extract.cpp (original)
> > > > +++ llvm/trunk/tools/llvm-extract/llvm-extract.cpp Tue Apr 14 13:33:00 2015
> > > > @@ -19,6 +19,7 @@
> > > > #include "llvm/IR/IRPrintingPasses.h"
> > > > #include "llvm/IR/LLVMContext.h"
> > > > #include "llvm/IR/Module.h"
> > > > +#include "llvm/IR/UseListOrder.h"
> > > > #include "llvm/IRReader/IRReader.h"
> > > > #include "llvm/IR/LegacyPassManager.h"
> > > > #include "llvm/Support/CommandLine.h"
> > > > @@ -97,6 +98,11 @@ int main(int argc, char **argv) {
> > > >
> > > > LLVMContext &Context = getGlobalContext();
> > > > llvm_shutdown_obj Y; // Call llvm_shutdown() on exit.
> > > > +
> > > > + // Turn on -preserve-bc-uselistorder by default, but let the command-line
> > > > + // override it.
> > > > + setPreserveBitcodeUseListOrder(true);
> > > > +
> > > > cl::ParseCommandLineOptions(argc, argv, "llvm extractor\n");
> > > >
> > > > // Use lazy loading, since we only care about selected global values.
> > > >
> > > > Modified: llvm/trunk/tools/llvm-link/llvm-link.cpp
> > > > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-link/llvm-link.cpp?rev=234921&r1=234920&r2=234921&view=diff
> > > > ==============================================================================
> > > > --- llvm/trunk/tools/llvm-link/llvm-link.cpp (original)
> > > > +++ llvm/trunk/tools/llvm-link/llvm-link.cpp Tue Apr 14 13:33:00 2015
> > > > @@ -20,6 +20,7 @@
> > > > #include "llvm/IR/DiagnosticPrinter.h"
> > > > #include "llvm/IR/LLVMContext.h"
> > > > #include "llvm/IR/Module.h"
> > > > +#include "llvm/IR/UseListOrder.h"
> > > > #include "llvm/IR/Verifier.h"
> > > > #include "llvm/IRReader/IRReader.h"
> > > > #include "llvm/Support/CommandLine.h"
> > > > @@ -104,6 +105,11 @@ int main(int argc, char **argv) {
> > > >
> > > > LLVMContext &Context = getGlobalContext();
> > > > llvm_shutdown_obj Y; // Call llvm_shutdown() on exit.
> > > > +
> > > > + // Turn on -preserve-bc-uselistorder by default, but let the command-line
> > > > + // override it.
> > > > + setPreserveBitcodeUseListOrder(true);
> > > > +
> > > > cl::ParseCommandLineOptions(argc, argv, "llvm linker\n");
> > > >
> > > > auto Composite = make_unique<Module>("llvm-link", Context);
> > > >
> > > > Modified: llvm/trunk/tools/opt/opt.cpp
> > > > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/opt/opt.cpp?rev=234921&r1=234920&r2=234921&view=diff
> > > > ==============================================================================
> > > > --- llvm/trunk/tools/opt/opt.cpp (original)
> > > > +++ llvm/trunk/tools/opt/opt.cpp Tue Apr 14 13:33:00 2015
> > > > @@ -30,6 +30,7 @@
> > > > #include "llvm/IR/LLVMContext.h"
> > > > #include "llvm/IR/LegacyPassNameParser.h"
> > > > #include "llvm/IR/Module.h"
> > > > +#include "llvm/IR/UseListOrder.h"
> > > > #include "llvm/IR/Verifier.h"
> > > > #include "llvm/IRReader/IRReader.h"
> > > > #include "llvm/InitializePasses.h"
> > > > @@ -344,6 +345,10 @@ int main(int argc, char **argv) {
> > > > polly::initializePollyPasses(Registry);
> > > > #endif
> > > >
> > > > + // Turn on -preserve-bc-uselistorder by default, but let the command-line
> > > > + // override it.
> > > > + setPreserveBitcodeUseListOrder(true);
> > > > +
> > > > cl::ParseCommandLineOptions(argc, argv,
> > > > "llvm .bc -> .bc modular optimizer and analysis printer\n");
> > > >
> > > >
> > > >
> > > > _______________________________________________
> > > > llvm-commits mailing list
> > > > llvm-commits at cs.uiuc.edu
> > > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > > >
More information about the llvm-commits
mailing list