<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body dir="auto"><div><br><br>Sent from my iPhone</div><div><br>On Mar 9, 2016, at 2:05 AM, Chandler Carruth <<a href="mailto:chandlerc@gmail.com">chandlerc@gmail.com</a>> wrote:<br><br></div><blockquote type="cite"><div><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></div></div></div></blockquote><div><br></div><div>Argument like "function argument"?</div><div>They are stripped right now (see unittest)</div><div><br></div><div>Mehdi</div><br><blockquote type="cite"><div><div dir="ltr"><div class="gmail_quote"><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>
</div></blockquote></body></html>