From 3864bfd2e0ce7e32fd623c550660885599383e6a Mon Sep 17 00:00:00 2001 From: Daniil Fukalov <1671137+dfukalov@users.noreply.github.com> Date: Tue, 28 May 2024 16:09:53 +0200 Subject: [PATCH] [IR] Fix ignoring `non-global-value-max-name-size` in `ValueSymbolTable::makeUniqueName()`. (#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. --- llvm/lib/IR/Function.cpp | 2 +- llvm/lib/IR/ValueSymbolTable.cpp | 33 ++++++++++++------- .../non-global-value-max-name-size-2.ll | 23 +++++++++++++ llvm/test/Bitcode/value-with-long-name-dbg.ll | 11 +++++++ llvm/test/Bitcode/value-with-long-name.ll | 4 +-- 5 files changed, 58 insertions(+), 15 deletions(-) create mode 100644 llvm/test/Assembler/non-global-value-max-name-size-2.ll create mode 100644 llvm/test/Bitcode/value-with-long-name-dbg.ll diff --git a/llvm/lib/IR/Function.cpp b/llvm/lib/IR/Function.cpp index bd06ff82a15a58..13fa1afeaaff24 100644 --- a/llvm/lib/IR/Function.cpp +++ b/llvm/lib/IR/Function.cpp @@ -79,7 +79,7 @@ using ProfileCount = Function::ProfileCount; // are not in the public header file... template class llvm::SymbolTableListTraits; -static cl::opt NonGlobalValueMaxNameSize( +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.")); diff --git a/llvm/lib/IR/ValueSymbolTable.cpp b/llvm/lib/IR/ValueSymbolTable.cpp index 52f7ddcdc65a2b..a020acf22a96c5 100644 --- a/llvm/lib/IR/ValueSymbolTable.cpp +++ b/llvm/lib/IR/ValueSymbolTable.cpp @@ -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(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(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) diff --git a/llvm/test/Assembler/non-global-value-max-name-size-2.ll b/llvm/test/Assembler/non-global-value-max-name-size-2.ll new file mode 100644 index 00000000000000..5eac003ddb4383 --- /dev/null +++ b/llvm/test/Assembler/non-global-value-max-name-size-2.ll @@ -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 +} \ No newline at end of file diff --git a/llvm/test/Bitcode/value-with-long-name-dbg.ll b/llvm/test/Bitcode/value-with-long-name-dbg.ll new file mode 100644 index 00000000000000..0cc3569d8617b3 --- /dev/null +++ b/llvm/test/Bitcode/value-with-long-name-dbg.ll @@ -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 +} diff --git a/llvm/test/Bitcode/value-with-long-name.ll b/llvm/test/Bitcode/value-with-long-name.ll index 1ca5d133e09ae3..aa7da5f5b7dba9 100644 --- a/llvm/test/Bitcode/value-with-long-name.ll +++ b/llvm/test/Bitcode/value-with-long-name.ll @@ -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]+}} @@ -14,5 +14,3 @@ define i32 @f(i32 %a, i32 %b) { %e = add i32 %d, %b ret i32 %e } - -