[llvm] 239c831 - Add switch to use "source_filename" instead of a hash ID for globally promoted local
Bill Wendling via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 3 16:42:09 PDT 2022
Author: Bill Wendling
Date: 2022-08-03T16:41:56-07:00
New Revision: 239c831de4d09530ebd4e254f9248764792afa5e
URL: https://github.com/llvm/llvm-project/commit/239c831de4d09530ebd4e254f9248764792afa5e
DIFF: https://github.com/llvm/llvm-project/commit/239c831de4d09530ebd4e254f9248764792afa5e.diff
LOG: Add switch to use "source_filename" instead of a hash ID for globally promoted local
During LTO a local promoted to a global gets a unique suffix based on
a hash of the module IR. This means that changes in the local's module
can affect the contents in another module that imported it (because the name
of the imported promoted local is changed, but that doesn't reflect a
real change in the importing module). So any tool that's
validating changes to the importing module will see a superficial change.
Instead of using the module hash, we can use the "source_filename" if it
exists to generate a unique identifier that doesn't change due to LTO
shenanigans.
Differential Revision: https://reviews.llvm.org/D128863
Added:
llvm/test/ThinLTO/X86/Inputs/promote-local-name-1.ll
llvm/test/ThinLTO/X86/promote-local-name.ll
Modified:
llvm/include/llvm/IR/ModuleSummaryIndex.h
llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 468773ac59090..203f7b3a18ba9 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1465,10 +1465,15 @@ class ModuleSummaryIndex {
/// Convenience method for creating a promoted global name
/// for the given value name of a local, and its original module's ID.
static std::string getGlobalNameForLocal(StringRef Name, ModuleHash ModHash) {
+ std::string Suffix = utostr((uint64_t(ModHash[0]) << 32) |
+ ModHash[1]); // Take the first 64 bits
+ return getGlobalNameForLocal(Name, Suffix);
+ }
+
+ static std::string getGlobalNameForLocal(StringRef Name, StringRef Suffix) {
SmallString<256> NewName(Name);
NewName += ".llvm.";
- NewName += utostr((uint64_t(ModHash[0]) << 32) |
- ModHash[1]); // Take the first 64 bits
+ NewName += Suffix;
return std::string(NewName.str());
}
diff --git a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
index 8e6d4626c9fd2..a30f920d0036a 100644
--- a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
+++ b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
@@ -12,8 +12,18 @@
//===----------------------------------------------------------------------===//
#include "llvm/Transforms/Utils/FunctionImportUtils.h"
+#include "llvm/Support/CommandLine.h"
using namespace llvm;
+/// Uses the "source_filename" instead of a Module hash ID for the suffix of
+/// promoted locals during LTO. NOTE: This requires that the source filename
+/// has a unique name / path to avoid name collisions.
+static cl::opt<bool> UseSourceFilenameForPromotedLocals(
+ "use-source-filename-for-promoted-locals", cl::Hidden,
+ cl::desc("Uses the source file name instead of the Module hash. "
+ "This requires that the source filename has a unique name / "
+ "path to avoid name collisions."));
+
/// Checks if we should import SGV as a definition, otherwise import as a
/// declaration.
bool FunctionImportGlobalProcessing::doImportAsDefinition(
@@ -94,9 +104,19 @@ bool FunctionImportGlobalProcessing::isNonRenamableLocal(
std::string
FunctionImportGlobalProcessing::getPromotedName(const GlobalValue *SGV) {
assert(SGV->hasLocalLinkage());
+
// For locals that must be promoted to global scope, ensure that
// the promoted name uniquely identifies the copy in the original module,
// using the ID assigned during combined index creation.
+ if (UseSourceFilenameForPromotedLocals &&
+ !SGV->getParent()->getSourceFileName().empty()) {
+ SmallString<256> Suffix(SGV->getParent()->getSourceFileName());
+ std::replace_if(std::begin(Suffix), std::end(Suffix),
+ [&](char ch) { return !isAlnum(ch); }, '_');
+ return ModuleSummaryIndex::getGlobalNameForLocal(
+ SGV->getName(), Suffix);
+ }
+
return ModuleSummaryIndex::getGlobalNameForLocal(
SGV->getName(),
ImportIndex.getModuleHash(SGV->getParent()->getModuleIdentifier()));
diff --git a/llvm/test/ThinLTO/X86/Inputs/promote-local-name-1.ll b/llvm/test/ThinLTO/X86/Inputs/promote-local-name-1.ll
new file mode 100644
index 0000000000000..5e161fb182102
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/Inputs/promote-local-name-1.ll
@@ -0,0 +1,20 @@
+source_filename = "llvm/test/LTO/X86/promote-local-name-1.ll"
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+ at baz = internal constant i32 10, align 4
+
+; Function Attrs: noinline nounwind uwtable
+define i32 @b() {
+entry:
+ %call = call i32 @foo()
+ ret i32 %call
+}
+
+; Function Attrs: noinline nounwind uwtable
+define internal i32 @foo() {
+entry:
+ %0 = load i32, i32* @baz, align 4
+ ret i32 %0
+}
diff --git a/llvm/test/ThinLTO/X86/promote-local-name.ll b/llvm/test/ThinLTO/X86/promote-local-name.ll
new file mode 100644
index 0000000000000..65c62985c8d62
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/promote-local-name.ll
@@ -0,0 +1,33 @@
+; Test the "-use-source-filename-for-promoted-locals" flag.
+
+; Do setup work for all below tests: generate bitcode and combined index
+; RUN: opt -module-summary -module-hash %s -o %t.bc
+; RUN: opt -module-summary -module-hash %p/Inputs/promote-local-name-1.ll -o %t1.bc
+; RUN: llvm-lto -thinlto-action=thinlink -o %t2.bc %t.bc %t1.bc
+
+; This module will import b() which should cause the copy of foo and baz from
+; that module (%t1.bc) to be imported. Check that the imported reference's
+; promoted name matches the imported copy. We check for a specific name,
+; because the result of the "-use-source-filename-for-promoted-locals" flag
+; should be deterministic.
+
+; RUN: llvm-lto -use-source-filename-for-promoted-locals -thinlto-action=import %t.bc -thinlto-index=%t2.bc -o - | llvm-dis -o - | FileCheck %s
+
+; CHECK: @baz.llvm.llvm_test_LTO_X86_promote_local_name_1_ll = internal constant i32 10, align 4
+; CHECK: call i32 @foo.llvm.llvm_test_LTO_X86_promote_local_name_1_ll
+; CHECK: define available_externally hidden i32 @foo.llvm.llvm_test_LTO_X86_promote_local_name_1_ll()
+
+source_filename = "llvm/test/ThinLTO/X86/promote-local-name.ll"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline nounwind uwtable
+define i32 @main() {
+entry:
+ %retval = alloca i32, align 4
+ store i32 0, i32* %retval, align 4
+ %call = call i32 (...) @b()
+ ret i32 %call
+}
+
+declare i32 @b(...)
More information about the llvm-commits
mailing list