[PATCH] D17946: Add a flag to the LLVMContext to disable name for Value other than GlobalValue

[Justin Bogner via llvm-commits]

Mehdi AMINI <mehdi.amini at apple.com> writes:
> joker.eph updated this revision to Diff 50175.
> joker.eph added a comment.
>
> Take Chandler's comment into account

This version LGTM, and I like the discard-value-names terminology.

>
> http://reviews.llvm.org/D17946
>
> Files:
>   include/llvm/IR/LLVMContext.h
>   lib/AsmParser/LLParser.cpp
>   lib/IR/LLVMContext.cpp
>   lib/IR/LLVMContextImpl.h
>   lib/IR/Value.cpp
>   lib/LTO/LTOCodeGenerator.cpp
>   test/Feature/strip_names.ll
>   tools/llc/llc.cpp
>   tools/opt/opt.cpp
>
> Index: tools/opt/opt.cpp
> ===================================================================
> --- tools/opt/opt.cpp
> +++ tools/opt/opt.cpp
> @@ -196,6 +196,11 @@
>               cl::desc("Run all passes twice, re-using the same pass manager."),
>               cl::init(false), cl::Hidden);
>  
> +static cl::opt<bool> DiscardValueNames(
> +    "discard-value-names",
> +    cl::desc("Discard names from Value (other than GlobalValue)."),
> +    cl::init(false), cl::Hidden);
> +
>  static inline void addPass(legacy::PassManagerBase &PM, Pass *P) {
>    // Add the pass to the pass manager...
>    PM.add(P);
> @@ -345,6 +350,8 @@
>  
>    SMDiagnostic Err;
>  
> +  Context.setDiscardValueNames(DiscardValueNames);
> +
>    // Load the input module...
>    std::unique_ptr<Module> M = parseIRFile(InputFilename, Err, Context);
>  
> Index: tools/llc/llc.cpp
> ===================================================================
> --- tools/llc/llc.cpp
> +++ tools/llc/llc.cpp
> @@ -103,6 +103,11 @@
>                            "manager and verify the result is the same."),
>                   cl::init(false));
>  
> +static cl::opt<bool> DiscardValueNames(
> +    "discard-value-names",
> +    cl::desc("Discard names from Value (other than GlobalValue)."),
> +    cl::init(false), cl::Hidden);
> +
>  static int compileModule(char **, LLVMContext &);
>  
>  static std::unique_ptr<tool_output_file>
> @@ -205,6 +210,8 @@
>  
>    cl::ParseCommandLineOptions(argc, argv, "llvm system compiler\n");
>  
> +  Context.setDiscardValueNames(DiscardValueNames);
> +
>    // Compile the module TimeCompilations times to give better compile time
>    // metrics.
>    for (unsigned I = TimeCompilations; I; --I)
> Index: test/Feature/strip_names.ll
> ===================================================================
> --- /dev/null
> +++ test/Feature/strip_names.ll
> @@ -0,0 +1,26 @@
> +; RUN: opt < %s -S | FileCheck %s
> +; RUN: opt < %s  | opt -S -discard-value-names | FileCheck --check-prefix=NONAME %s
> +
> +
> +; CHECK: @GlobalValueName
> +; CHECK: @foo(i32 %in)
> +; CHECK: somelabel:
> +; CHECK:  %GV = load i32, i32* @GlobalValueName
> +; CHECK:  %add = add i32 %in, %GV
> +; CHECK:  ret i32 %add
> +
> +; NONAME: @GlobalValueName
> +; NONAME: @foo(i32)
> +; NONAME-NOT: somelabel:
> +; NONAME:  %2 = load i32, i32* @GlobalValueName
> +; NONAME:  %3 = add i32 %0, %2
> +; NONAME:  ret i32 %3
> +
> + at GlobalValueName = global i32 0
> +
> +define i32 @foo(i32 %in) {
> +somelabel:
> +  %GV = load i32, i32* @GlobalValueName
> +  %add = add i32 %in, %GV
> +  ret i32 %add
> +}
> Index: lib/LTO/LTOCodeGenerator.cpp
> ===================================================================
> --- lib/LTO/LTOCodeGenerator.cpp
> +++ lib/LTO/LTOCodeGenerator.cpp
> @@ -73,9 +73,20 @@
>  #endif
>  }
>  
> +static cl::opt<bool> DiscardValueNames(
> +    "discard-value-names",
> +    cl::desc("Strip names from Value (other than GlobalValue)."),
> +#ifdef NDEBUG
> +    cl::init(true),
> +#else
> +    cl::init(false),
> +#endif
> +    cl::Hidden);
> +
>  LTOCodeGenerator::LTOCodeGenerator(LLVMContext &Context)
>      : Context(&Context), MergedModule(new Module("ld-temp.o", Context)),
>        TheLinker(new Linker(*MergedModule)) {
> +  Context.setDiscardValueNames(DiscardValueNames);
>    initializeLTOPasses();
>  }
>  
> Index: lib/IR/Value.cpp
> ===================================================================
> --- lib/IR/Value.cpp
> +++ lib/IR/Value.cpp
> @@ -195,6 +195,10 @@
>  }
>  
>  void Value::setNameImpl(const Twine &NewName) {
> +  // Fast-path: LLVMContext can be set to strip out non-GlobalValue names
> +  if (getContext().discardValueNames() && !isa<GlobalValue>(this))
> +    return;
> +
>    // Fast path for common IRBuilder case of setName("") when there is no name.
>    if (NewName.isTriviallyEmpty() && !hasName())
>      return;
> Index: lib/IR/LLVMContextImpl.h
> ===================================================================
> --- lib/IR/LLVMContextImpl.h
> +++ lib/IR/LLVMContextImpl.h
> @@ -1034,6 +1034,10 @@
>    /// clients which do use GC.
>    DenseMap<const Function*, std::string> GCNames;
>  
> +  /// Flag to indicate if Value (other than GlobalValue) retains their name or
> +  /// not.
> +  bool DiscardValueNames = false;
> +
>    LLVMContextImpl(LLVMContext &C);
>    ~LLVMContextImpl();
>  
> Index: lib/IR/LLVMContext.cpp
> ===================================================================
> --- lib/IR/LLVMContext.cpp
> +++ lib/IR/LLVMContext.cpp
> @@ -320,3 +320,9 @@
>  void LLVMContext::deleteGC(const Function &Fn) {
>    pImpl->GCNames.erase(&Fn);
>  }
> +
> +bool LLVMContext::discardValueNames() { return pImpl->DiscardValueNames; }
> +
> +void LLVMContext::setDiscardValueNames(bool Discard) {
> +  pImpl->DiscardValueNames = Discard;
> +}
> Index: lib/AsmParser/LLParser.cpp
> ===================================================================
> --- lib/AsmParser/LLParser.cpp
> +++ lib/AsmParser/LLParser.cpp
> @@ -45,6 +45,11 @@
>    // Prime the lexer.
>    Lex.Lex();
>  
> +  if (Context.discardValueNames())
> +    return Error(
> +        Lex.getLoc(),
> +        "Can't read textual IR with a Context that discards named Values");
> +
>    return ParseTopLevelEntities() ||
>           ValidateEndOfModule();
>  }
> Index: include/llvm/IR/LLVMContext.h
> ===================================================================
> --- include/llvm/IR/LLVMContext.h
> +++ include/llvm/IR/LLVMContext.h
> @@ -103,6 +103,15 @@
>    /// Remove the GC for a function
>    void deleteGC(const Function &Fn);
>  
> +  /// Return true if the Context runtime configuration is set to discard all
> +  /// value names. When true, only GlobalValue names will be available in the
> +  /// IR.
> +  bool discardValueNames();
> +
> +  /// Set the Context runtime configuration to discard all value name (but
> +  /// GlobalValue). Clients can use this flag to save memory and runtime,
> +  /// especially in release mode.
> +  void setDiscardValueNames(bool Discard);
>  
>    typedef void (*InlineAsmDiagHandlerTy)(const SMDiagnostic&, void *Context,
>                                           unsigned LocCookie);