[llvm] [IR] Fix ignoring `non-global-value-max-name-size` in `ValueSymbolTab… (PR #89057)
Daniil Fukalov via llvm-commits
llvm-commits at lists.llvm.org
Fri May 17 08:16:37 PDT 2024
https://github.com/dfukalov updated https://github.com/llvm/llvm-project/pull/89057
>From daa21880e1455eb25abddbe11fdabd455172646f Mon Sep 17 00:00:00 2001
From: dfukalov <1671137+dfukalov at users.noreply.github.com>
Date: Wed, 17 Apr 2024 13:43:36 +0200
Subject: [PATCH 1/4] [IR] Fix ignoring `non-global-value-max-name-size` in
`ValueSymbolTable::makeUniqueName()`.
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.
---
llvm/lib/IR/ValueSymbolTable.cpp | 31 ++++++++++++-------
.../non-global-value-max-name-size-2.ll | 23 ++++++++++++++
2 files changed, 43 insertions(+), 11 deletions(-)
create mode 100644 llvm/test/Assembler/non-global-value-max-name-size-2.ll
diff --git a/llvm/lib/IR/ValueSymbolTable.cpp b/llvm/lib/IR/ValueSymbolTable.cpp
index 52f7ddcdc65a2..7b4cecac9a2cc 100644
--- a/llvm/lib/IR/ValueSymbolTable.cpp
+++ b/llvm/lib/IR/ValueSymbolTable.cpp
@@ -43,23 +43,32 @@ 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() > (unsigned)MaxNameSize) {
+ BaseSize -= UniqueName.size() - (unsigned)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 0000000000000..5eac003ddb438
--- /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
>From 0c376ad20206580b257032da0f9e9379a64aab3b Mon Sep 17 00:00:00 2001
From: dfukalov <1671137+dfukalov at users.noreply.github.com>
Date: Wed, 17 Apr 2024 14:54:56 +0200
Subject: [PATCH 2/4] Fix `Bitcode/value-with-long-name.ll` test
---
llvm/lib/IR/ValueSymbolTable.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/IR/ValueSymbolTable.cpp b/llvm/lib/IR/ValueSymbolTable.cpp
index 7b4cecac9a2cc..040a84228f11a 100644
--- a/llvm/lib/IR/ValueSymbolTable.cpp
+++ b/llvm/lib/IR/ValueSymbolTable.cpp
@@ -65,7 +65,7 @@ ValueName *ValueSymbolTable::makeUniqueName(Value *V,
S << ++LastUnique;
// Retry if MaxNameSize has been exceeded.
- if (MaxNameSize > -1 && UniqueName.size() > (unsigned)MaxNameSize) {
+ if (MaxNameSize > 0 && UniqueName.size() > (unsigned)MaxNameSize) {
BaseSize -= UniqueName.size() - (unsigned)MaxNameSize;
continue;
}
>From 94678f113d32c4198f47f621eae2db055287d12c Mon Sep 17 00:00:00 2001
From: dfukalov <1671137+dfukalov at users.noreply.github.com>
Date: Fri, 17 May 2024 17:11:24 +0200
Subject: [PATCH 3/4] amend! Fix `Bitcode/value-with-long-name.ll` test
Fix `Bitcode/value-with-long-name.ll` test
---
llvm/lib/IR/Function.cpp | 2 +-
llvm/lib/IR/ValueSymbolTable.cpp | 6 ++++--
llvm/test/Bitcode/value-with-long-name-dbg.ll | 11 +++++++++++
llvm/test/Bitcode/value-with-long-name.ll | 4 +---
4 files changed, 17 insertions(+), 6 deletions(-)
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 96953ac49c19b..d8b4e69f13b77 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<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."));
diff --git a/llvm/lib/IR/ValueSymbolTable.cpp b/llvm/lib/IR/ValueSymbolTable.cpp
index 040a84228f11a..a020acf22a96c 100644
--- a/llvm/lib/IR/ValueSymbolTable.cpp
+++ b/llvm/lib/IR/ValueSymbolTable.cpp
@@ -65,8 +65,10 @@ ValueName *ValueSymbolTable::makeUniqueName(Value *V,
S << ++LastUnique;
// Retry if MaxNameSize has been exceeded.
- if (MaxNameSize > 0 && UniqueName.size() > (unsigned)MaxNameSize) {
- BaseSize -= UniqueName.size() - (unsigned)MaxNameSize;
+ 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.
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 0000000000000..0cc3569d8617b
--- /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 1ca5d133e09ae..aa7da5f5b7dba 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
}
-
-
>From e389f10717ce72cd8cc51f7864609166c8fb17d2 Mon Sep 17 00:00:00 2001
From: dfukalov <1671137+dfukalov at users.noreply.github.com>
Date: Fri, 17 May 2024 17:12:37 +0200
Subject: [PATCH 4/4] amend! [IR] Fix ignoring `non-global-value-max-name-size`
in `ValueSymbolTable::makeUniqueName()`.
[IR] Fix ignoring `non-global-value-max-name-size` in `ValueSymbolTable::makeUniqueName()`.
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.
More information about the llvm-commits
mailing list