Skip to content

Commit

Permalink
[IR] Fix ignoring non-global-value-max-name-size in `ValueSymbolTab…
Browse files Browse the repository at this point in the history
…le::makeUniqueName()`. (llvm#89057)

E.g. during inlining new symbol name can be duplicated and then
`ValueSymbolTable::makeUniqueName()` will add unique suffix, exceeding
the `non-global-value-max-name-size` restriction.

Also fixed `unsigned` type of the option to `int` since `ValueSymbolTable`'
constructor can use `-1` value that means unrestricted name size.
  • Loading branch information
dfukalov authored May 28, 2024
1 parent 46a30df commit 3864bfd
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 15 deletions.
2 changes: 1 addition & 1 deletion llvm/lib/IR/Function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ using ProfileCount = Function::ProfileCount;
// are not in the public header file...
template class llvm::SymbolTableListTraits<BasicBlock>;

static cl::opt<unsigned> NonGlobalValueMaxNameSize(
static cl::opt<int> NonGlobalValueMaxNameSize(
"non-global-value-max-name-size", cl::Hidden, cl::init(1024),
cl::desc("Maximum size for the name of non-global values."));

Expand Down
33 changes: 22 additions & 11 deletions llvm/lib/IR/ValueSymbolTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,23 +43,34 @@ ValueSymbolTable::~ValueSymbolTable() {
ValueName *ValueSymbolTable::makeUniqueName(Value *V,
SmallString<256> &UniqueName) {
unsigned BaseSize = UniqueName.size();
bool AppenDot = false;
if (auto *GV = dyn_cast<GlobalValue>(V)) {
// A dot is appended to mark it as clone during ABI demangling so that
// for example "_Z1fv" and "_Z1fv.1" both demangle to "f()", the second
// one being a clone.
// On NVPTX we cannot use a dot because PTX only allows [A-Za-z0-9_$] for
// identifiers. This breaks ABI demangling but at least ptxas accepts and
// compiles the program.
const Module *M = GV->getParent();
if (!(M && Triple(M->getTargetTriple()).isNVPTX()))
AppenDot = true;
}

while (true) {
// Trim any suffix off and append the next number.
UniqueName.resize(BaseSize);
raw_svector_ostream S(UniqueName);
if (auto *GV = dyn_cast<GlobalValue>(V)) {
// A dot is appended to mark it as clone during ABI demangling so that
// for example "_Z1fv" and "_Z1fv.1" both demangle to "f()", the second
// one being a clone.
// On NVPTX we cannot use a dot because PTX only allows [A-Za-z0-9_$] for
// identifiers. This breaks ABI demangling but at least ptxas accepts and
// compiles the program.
const Module *M = GV->getParent();
if (!(M && Triple(M->getTargetTriple()).isNVPTX()))
S << ".";
}
if (AppenDot)
S << ".";
S << ++LastUnique;

// Retry if MaxNameSize has been exceeded.
if (MaxNameSize > -1 && UniqueName.size() > (size_t)MaxNameSize) {
assert(BaseSize >= UniqueName.size() - (size_t)MaxNameSize &&
"Can't generate unique name: MaxNameSize is too small.");
BaseSize -= UniqueName.size() - (size_t)MaxNameSize;
continue;
}
// Try insert the vmap entry with this suffix.
auto IterBool = vmap.insert(std::make_pair(UniqueName.str(), V));
if (IterBool.second)
Expand Down
23 changes: 23 additions & 0 deletions llvm/test/Assembler/non-global-value-max-name-size-2.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
; RUN: opt < %s -S -passes='always-inline' -non-global-value-max-name-size=5 | opt -non-global-value-max-name-size=5 -passes=verify -disable-output

; Opt should not generate too long name for labels during inlining.

define internal i32 @inner(i32 %flag) alwaysinline {
entry:
%icmp = icmp slt i32 %flag, 0
br i1 %icmp, label %one, label %two

one:
ret i32 42

two:
ret i32 44
}

define i32 @outer(i32 %x) {
entry:
%call1 = call i32 @inner(i32 %x)
%call2 = call i32 @inner(i32 %x)
%ret = add i32 %call1, %call2
ret i32 %ret
}
11 changes: 11 additions & 0 deletions llvm/test/Bitcode/value-with-long-name-dbg.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
; REQUIRES: asserts
; Force the size to be small to check assertion message.
; RUN: not --crash opt -S %s -O2 -o - -non-global-value-max-name-size=0 2>&1 | FileCheck %s
; CHECK: Can't generate unique name: MaxNameSize is too small.

define i32 @f(i32 %a, i32 %b) {
%c = add i32 %a, %b
%d = add i32 %c, %a
%e = add i32 %d, %b
ret i32 %e
}
4 changes: 1 addition & 3 deletions llvm/test/Bitcode/value-with-long-name.ll
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
; Check the size of generated variable when no option is set
; RUN: opt -S %s -O2 -o - | FileCheck -check-prefix=CHECK-LONG %s
; RUN: opt -S %s -O2 -o - -non-global-value-max-name-size=-1 | FileCheck -check-prefix=CHECK-LONG %s
; CHECK-LONG: %{{[a-z]{4}[a-z]+}}

; Then check we correctly cap the size of newly generated non-global values name
; Force the size to be small so that the check works on release and debug build
; RUN: opt -S %s -O2 -o - -non-global-value-max-name-size=0 | FileCheck -check-prefix=CHECK-SHORT %s
; RUN: opt -S %s -O2 -o - -non-global-value-max-name-size=1 | FileCheck -check-prefix=CHECK-SHORT %s
; CHECK-SHORT-NOT: %{{[a-z][a-z]+}}

Expand All @@ -14,5 +14,3 @@ define i32 @f(i32 %a, i32 %b) {
%e = add i32 %d, %b
ret i32 %e
}


0 comments on commit 3864bfd

Please sign in to comment.