<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Wed, Mar 9, 2016 at 10:46 AM Justin Bogner <<a href="mailto:mail@justinbogner.com">mail@justinbogner.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Mehdi AMINI <<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>> writes:<br>
> joker.eph updated this revision to Diff 50104.<br>
> joker.eph added a comment.<br>
><br>
> Rename preserveNames() to preserveNonGlobalValueNames()<br>
<br>
preserveNonGlobalValueNames is confusing and awkward. It's much worse<br>
than preserveNames. I'm not convinced preservedNames is bad, but if<br>
you're concerned that it's not precise enough I'd suggest<br>
'preserveAllNames', 'preserveInconsequentialNames', or<br>
'preserveDebugNames'.<br></blockquote><div><br></div><div>'preserveLocalNames'?</div><div><br></div><div>We always will preserve Argument names, Global names, etc.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
><br>
> <a href="http://reviews.llvm.org/D17946" rel="noreferrer" target="_blank">http://reviews.llvm.org/D17946</a><br>
><br>
> Files:<br>
> include/llvm/IR/LLVMContext.h<br>
> lib/AsmParser/LLParser.cpp<br>
> lib/IR/LLVMContext.cpp<br>
> lib/IR/LLVMContextImpl.h<br>
> lib/IR/Value.cpp<br>
> lib/LTO/LTOCodeGenerator.cpp<br>
> test/Feature/strip_names.ll<br>
> tools/llc/llc.cpp<br>
> tools/opt/opt.cpp<br>
><br>
> Index: tools/opt/opt.cpp<br>
> ===================================================================<br>
> --- tools/opt/opt.cpp<br>
> +++ tools/opt/opt.cpp<br>
> @@ -196,6 +196,11 @@<br>
> cl::desc("Run all passes twice, re-using the same pass manager."),<br>
> cl::init(false), cl::Hidden);<br>
><br>
> +static cl::opt<bool> DisableNamedValue(<br>
> + "disable-named-value",<br>
> + cl::desc("Strip name from Value (other than GlobalValue)."),<br>
> + cl::init(false), cl::Hidden);<br>
> +<br>
> static inline void addPass(legacy::PassManagerBase &PM, Pass *P) {<br>
> // Add the pass to the pass manager...<br>
> PM.add(P);<br>
> @@ -345,6 +350,9 @@<br>
><br>
> SMDiagnostic Err;<br>
><br>
> + // Honor "-disable-named-value" command line option<br>
> + Context.setPreserveNonGlobalValueNames(!DisableNamedValue);<br>
> +<br>
> // Load the input module...<br>
> std::unique_ptr<Module> M = parseIRFile(InputFilename, Err, Context);<br>
><br>
> Index: tools/llc/llc.cpp<br>
> ===================================================================<br>
> --- tools/llc/llc.cpp<br>
> +++ tools/llc/llc.cpp<br>
> @@ -103,6 +103,11 @@<br>
> "manager and verify the result is the same."),<br>
> cl::init(false));<br>
><br>
> +static cl::opt<bool> DisableNamedValue(<br>
> + "disable-named-value",<br>
> + cl::desc("Strip name from Value (other than GlobalValue)."),<br>
> + cl::init(false), cl::Hidden);<br>
> +<br>
> static int compileModule(char **, LLVMContext &);<br>
><br>
> static std::unique_ptr<tool_output_file><br>
> @@ -205,6 +210,9 @@<br>
><br>
> cl::ParseCommandLineOptions(argc, argv, "llvm system compiler\n");<br>
><br>
> + // Honor "-disable-named-value" command line option<br>
> + Context.setPreserveNonGlobalValueNames(!DisableNamedValue);<br>
> +<br>
> // Compile the module TimeCompilations times to give better compile time<br>
> // metrics.<br>
> for (unsigned I = TimeCompilations; I; --I)<br>
> Index: test/Feature/strip_names.ll<br>
> ===================================================================<br>
> --- /dev/null<br>
> +++ test/Feature/strip_names.ll<br>
> @@ -0,0 +1,26 @@<br>
> +; RUN: opt < %s -S | FileCheck %s<br>
> +; RUN: opt < %s | opt -S -disable-named-value | FileCheck --check-prefix=NONAME %s<br>
> +<br>
> +<br>
> +; CHECK: @GlobalValueName<br>
> +; CHECK: @foo(i32 %in)<br>
> +; CHECK: somelabel:<br>
> +; CHECK: %GV = load i32, i32* @GlobalValueName<br>
> +; CHECK: %add = add i32 %in, %GV<br>
> +; CHECK: ret i32 %add<br>
> +<br>
> +; NONAME: @GlobalValueName<br>
> +; NONAME: @foo(i32)<br>
> +; NONAME-NOT: somelabel:<br>
> +; NONAME: %2 = load i32, i32* @GlobalValueName<br>
> +; NONAME: %3 = add i32 %0, %2<br>
> +; NONAME: ret i32 %3<br>
> +<br>
> +@GlobalValueName = global i32 0<br>
> +<br>
> +define i32 @foo(i32 %in) {<br>
> +somelabel:<br>
> + %GV = load i32, i32* @GlobalValueName<br>
> + %add = add i32 %in, %GV<br>
> + ret i32 %add<br>
> +}<br>
> Index: lib/LTO/LTOCodeGenerator.cpp<br>
> ===================================================================<br>
> --- lib/LTO/LTOCodeGenerator.cpp<br>
> +++ lib/LTO/LTOCodeGenerator.cpp<br>
> @@ -76,6 +76,9 @@<br>
> LTOCodeGenerator::LTOCodeGenerator(LLVMContext &Context)<br>
> : Context(&Context), MergedModule(new Module("ld-temp.o", Context)),<br>
> TheLinker(new Linker(*MergedModule)) {<br>
> +#ifdef NDEBUG<br>
> + Context.setPreserveNonGlobalValueNames(false);<br>
> +#endif<br>
> initializeLTOPasses();<br>
> }<br>
><br>
> Index: lib/IR/Value.cpp<br>
> ===================================================================<br>
> --- lib/IR/Value.cpp<br>
> +++ lib/IR/Value.cpp<br>
> @@ -195,6 +195,10 @@<br>
> }<br>
><br>
> void Value::setNameImpl(const Twine &NewName) {<br>
> + // Fast-path: LLVMContext can be set to strip out non-GlobalValue names<br>
> + if (!getContext().preserveNonGlobalValueNames() && !isa<GlobalValue>(this))<br>
> + return;<br>
> +<br>
> // Fast path for common IRBuilder case of setName("") when there is no name.<br>
> if (NewName.isTriviallyEmpty() && !hasName())<br>
> return;<br>
> Index: lib/IR/LLVMContextImpl.h<br>
> ===================================================================<br>
> --- lib/IR/LLVMContextImpl.h<br>
> +++ lib/IR/LLVMContextImpl.h<br>
> @@ -1034,6 +1034,10 @@<br>
> /// clients which do use GC.<br>
> DenseMap<const Function*, std::string> GCNames;<br>
><br>
> + /// Flag to indicate if Value (other than GlobalValue) retains their name or<br>
> + /// not.<br>
> + bool PreserveNames = true;<br>
> +<br>
> LLVMContextImpl(LLVMContext &C);<br>
> ~LLVMContextImpl();<br>
><br>
> Index: lib/IR/LLVMContext.cpp<br>
> ===================================================================<br>
> --- lib/IR/LLVMContext.cpp<br>
> +++ lib/IR/LLVMContext.cpp<br>
> @@ -320,3 +320,9 @@<br>
> void LLVMContext::deleteGC(const Function &Fn) {<br>
> pImpl->GCNames.erase(&Fn);<br>
> }<br>
> +<br>
> +bool LLVMContext::preserveNonGlobalValueNames() { return pImpl->PreserveNames; }<br>
> +<br>
> +void LLVMContext::setPreserveNonGlobalValueNames(bool Preserve) {<br>
> + pImpl->PreserveNames = Preserve;<br>
> +}<br>
> Index: lib/AsmParser/LLParser.cpp<br>
> ===================================================================<br>
> --- lib/AsmParser/LLParser.cpp<br>
> +++ lib/AsmParser/LLParser.cpp<br>
> @@ -45,6 +45,11 @@<br>
> // Prime the lexer.<br>
> Lex.Lex();<br>
><br>
> + if (!Context.preserveNonGlobalValueNames())<br>
> + return Error(<br>
> + Lex.getLoc(),<br>
> + "Can read textual IR with a Context that doesn't support named Values");<br>
> +<br>
> return ParseTopLevelEntities() ||<br>
> ValidateEndOfModule();<br>
> }<br>
> Index: include/llvm/IR/LLVMContext.h<br>
> ===================================================================<br>
> --- include/llvm/IR/LLVMContext.h<br>
> +++ include/llvm/IR/LLVMContext.h<br>
> @@ -103,6 +103,15 @@<br>
> /// Remove the GC for a function<br>
> void deleteGC(const Function &Fn);<br>
><br>
> + /// Return true if the Context runtime configuration is set to preserve all<br>
> + /// value name. When false, only GlobalValue names will be available in the<br>
> + /// IR.<br>
> + bool preserveNonGlobalValueNames();<br>
> +<br>
> + /// Set the Context runtime configuration to preserve all value name (true,<br>
> + /// default) or only GlobalValue names (false). Clients can use this flag to<br>
> + /// save memory and runtime, especially in release mode.<br>
> + void setPreserveNonGlobalValueNames(bool Preserve);<br>
><br>
> typedef void (*InlineAsmDiagHandlerTy)(const SMDiagnostic&, void *Context,<br>
> unsigned LocCookie);<br>
</blockquote></div></div>