From aa98e6ea8a35e86daa3783921881b58b15d50317 Mon Sep 17 00:00:00 2001 From: Hasyimi Bahrudin Date: Thu, 27 May 2021 04:01:20 +0000 Subject: [PATCH] Fix non-global-value-max-name-size not considered by LLParser `non-global-value-max-name-size` is used by `Value` to cap the length of local value name. However, this flag is not considered by `LLParser`, which leads to unexpected `use of undefined value error`. The fix is to move the responsibility of capping the length to `ValueSymbolTable`. The test is the one provided by [[ https://bugs.llvm.org/show_bug.cgi?id=45899 | Mikael in the bug report ]]. Reviewed By: mehdi_amini Differential Revision: https://reviews.llvm.org/D102707 --- include/llvm/IR/ValueSymbolTable.h | 17 ++++++++++++----- lib/IR/Function.cpp | 7 ++++++- lib/IR/Module.cpp | 2 +- lib/IR/Value.cpp | 10 ---------- lib/IR/ValueSymbolTable.cpp | 3 +++ .../Assembler/non-global-value-max-name-size.ll | 10 ++++++++++ 6 files changed, 32 insertions(+), 17 deletions(-) create mode 100644 test/Assembler/non-global-value-max-name-size.ll diff --git a/include/llvm/IR/ValueSymbolTable.h b/include/llvm/IR/ValueSymbolTable.h index 105ea73857a..43d00268f4b 100644 --- a/include/llvm/IR/ValueSymbolTable.h +++ b/include/llvm/IR/ValueSymbolTable.h @@ -60,18 +60,23 @@ public: /// @name Constructors /// @{ - ValueSymbolTable() : vmap(0) {} + ValueSymbolTable(int MaxNameSize = -1) : vmap(0), MaxNameSize(MaxNameSize) {} ~ValueSymbolTable(); -/// @} -/// @name Accessors -/// @{ + /// @} + /// @name Accessors + /// @{ /// This method finds the value with the given \p Name in the /// the symbol table. /// @returns the value associated with the \p Name /// Lookup a named Value. - Value *lookup(StringRef Name) const { return vmap.lookup(Name); } + Value *lookup(StringRef Name) const { + if (MaxNameSize > -1 && Name.size() > (unsigned)MaxNameSize) + Name = Name.substr(0, std::max(1u, (unsigned)MaxNameSize)); + + return vmap.lookup(Name); + } /// @returns true iff the symbol table is empty /// Determine if the symbol table is empty @@ -128,6 +133,8 @@ private: /// @{ ValueMap vmap; ///< The map that holds the symbol table. + int MaxNameSize; ///< The maximum size for each name. If the limit is + ///< exceeded, the name is capped. mutable uint32_t LastUnique = 0; ///< Counter for tracking unique names /// @} diff --git a/lib/IR/Function.cpp b/lib/IR/Function.cpp index a5c47563c47..29e0c64c4ea 100644 --- a/lib/IR/Function.cpp +++ b/lib/IR/Function.cpp @@ -60,6 +60,7 @@ #include "llvm/IR/Value.h" #include "llvm/IR/ValueSymbolTable.h" #include "llvm/Support/Casting.h" +#include "llvm/Support/CommandLine.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/ErrorHandling.h" #include @@ -76,6 +77,10 @@ using ProfileCount = Function::ProfileCount; // are not in the public header file... template class llvm::SymbolTableListTraits; +static cl::opt NonGlobalValueMaxNameSize( + "non-global-value-max-name-size", cl::Hidden, cl::init(1024), + cl::desc("Maximum size for the name of non-global values.")); + //===----------------------------------------------------------------------===// // Argument Implementation //===----------------------------------------------------------------------===// @@ -391,7 +396,7 @@ Function::Function(FunctionType *Ty, LinkageTypes Linkage, unsigned AddrSpace, // We only need a symbol table for a function if the context keeps value names if (!getContext().shouldDiscardValueNames()) - SymTab = std::make_unique(); + SymTab = std::make_unique(NonGlobalValueMaxNameSize); // If the function has arguments, mark them as lazily built. if (Ty->getNumParams()) diff --git a/lib/IR/Module.cpp b/lib/IR/Module.cpp index d38b2d1de24..eae4e69e76f 100644 --- a/lib/IR/Module.cpp +++ b/lib/IR/Module.cpp @@ -72,7 +72,7 @@ template class llvm::SymbolTableListTraits; // Module::Module(StringRef MID, LLVMContext &C) - : Context(C), ValSymTab(std::make_unique()), + : Context(C), ValSymTab(std::make_unique(-1)), Materializer(), ModuleID(std::string(MID)), SourceFileName(std::string(MID)), DL("") { Context.addModule(this); diff --git a/lib/IR/Value.cpp b/lib/IR/Value.cpp index d5587b42e6c..33b61c900e7 100644 --- a/lib/IR/Value.cpp +++ b/lib/IR/Value.cpp @@ -42,11 +42,6 @@ static cl::opt UseDerefAtPointSemantics( "use-dereferenceable-at-point-semantics", cl::Hidden, cl::init(false), cl::desc("Deref attributes and metadata infer facts at definition only")); - -static cl::opt NonGlobalValueMaxNameSize( - "non-global-value-max-name-size", cl::Hidden, cl::init(1024), - cl::desc("Maximum size for the name of non-global values.")); - //===----------------------------------------------------------------------===// // Value Class //===----------------------------------------------------------------------===// @@ -323,11 +318,6 @@ void Value::setNameImpl(const Twine &NewName) { if (getName() == NameRef) return; - // Cap the size of non-GlobalValue names. - if (NameRef.size() > NonGlobalValueMaxNameSize && !isa(this)) - NameRef = - NameRef.substr(0, std::max(1u, (unsigned)NonGlobalValueMaxNameSize)); - assert(!getType()->isVoidTy() && "Cannot assign a name to void values!"); // Get the symbol table to update for this object. diff --git a/lib/IR/ValueSymbolTable.cpp b/lib/IR/ValueSymbolTable.cpp index b49842315f3..6e5330ecc5f 100644 --- a/lib/IR/ValueSymbolTable.cpp +++ b/lib/IR/ValueSymbolTable.cpp @@ -100,6 +100,9 @@ void ValueSymbolTable::removeValueName(ValueName *V) { /// it into the symbol table with the specified name. If it conflicts, it /// auto-renames the name and returns that instead. ValueName *ValueSymbolTable::createValueName(StringRef Name, Value *V) { + if (MaxNameSize > -1 && Name.size() > (unsigned)MaxNameSize) + Name = Name.substr(0, std::max(1u, (unsigned)MaxNameSize)); + // In the common case, the name is not already in the symbol table. auto IterBool = vmap.insert(std::make_pair(Name, V)); if (IterBool.second) { diff --git a/test/Assembler/non-global-value-max-name-size.ll b/test/Assembler/non-global-value-max-name-size.ll new file mode 100644 index 00000000000..4536ce20568 --- /dev/null +++ b/test/Assembler/non-global-value-max-name-size.ll @@ -0,0 +1,10 @@ +; RUN: opt < %s -S -non-global-value-max-name-size=4 +; Test that local value name lookup works if the name is capped + +define void @f() { +bb0: + br label %testz + +testz: + br label %testz +}