-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[nfc][thinlto] remove unnecessary return from renameModuleForThinLTO
#121851
Conversation
Same goes for `FunctionImportGlobalProcessing::run`.
@llvm/pr-subscribers-llvm-transforms Author: Mircea Trofin (mtrofin) ChangesSame goes for Full diff: https://github.com/llvm/llvm-project/pull/121851.diff 5 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Utils/FunctionImportUtils.h b/llvm/include/llvm/Transforms/Utils/FunctionImportUtils.h
index 749b7b2bb5d869..18cd923d5601d0 100644
--- a/llvm/include/llvm/Transforms/Utils/FunctionImportUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/FunctionImportUtils.h
@@ -120,12 +120,12 @@ class FunctionImportGlobalProcessing {
#endif
}
- bool run();
+ void run();
};
/// Perform in-place global value handling on the given Module for
/// exported local functions renamed and promoted for ThinLTO.
-bool renameModuleForThinLTO(
+void renameModuleForThinLTO(
Module &M, const ModuleSummaryIndex &Index,
bool ClearDSOLocalOnDeclarations,
SetVector<GlobalValue *> *GlobalsToImport = nullptr);
diff --git a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
index 4522f4adcebe68..189f2876b61c0f 100644
--- a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
+++ b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
@@ -160,8 +160,7 @@ generateModuleMap(std::vector<std::unique_ptr<lto::InputFile>> &Modules) {
static void promoteModule(Module &TheModule, const ModuleSummaryIndex &Index,
bool ClearDSOLocalOnDeclarations) {
- if (renameModuleForThinLTO(TheModule, Index, ClearDSOLocalOnDeclarations))
- report_fatal_error("renameModuleForThinLTO failed");
+ renameModuleForThinLTO(TheModule, Index, ClearDSOLocalOnDeclarations);
}
namespace {
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index fde43bb354e83e..c4ef819466df94 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -1950,9 +1950,8 @@ Expected<bool> FunctionImporter::importFunctions(
SrcModule->setPartialSampleProfileRatio(Index);
// Link in the specified functions.
- if (renameModuleForThinLTO(*SrcModule, Index, ClearDSOLocalOnDeclarations,
- &GlobalsToImport))
- return true;
+ renameModuleForThinLTO(*SrcModule, Index, ClearDSOLocalOnDeclarations,
+ &GlobalsToImport);
if (PrintImports) {
for (const auto *GV : GlobalsToImport)
@@ -2026,11 +2025,8 @@ static bool doImportingForModuleForTest(
// Next we need to promote to global scope and rename any local values that
// are potentially exported to other modules.
- if (renameModuleForThinLTO(M, *Index, /*ClearDSOLocalOnDeclarations=*/false,
- /*GlobalsToImport=*/nullptr)) {
- errs() << "Error renaming module\n";
- return true;
- }
+ renameModuleForThinLTO(M, *Index, /*ClearDSOLocalOnDeclarations=*/false,
+ /*GlobalsToImport=*/nullptr);
// Perform the import now.
auto ModuleLoader = [&M](StringRef Identifier) {
diff --git a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
index 766c7501550da5..6f0c349320bc88 100644
--- a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
+++ b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
@@ -331,15 +331,14 @@ void FunctionImportGlobalProcessing::processGlobalsForThinLTO() {
}
}
-bool FunctionImportGlobalProcessing::run() {
+void FunctionImportGlobalProcessing::run() {
processGlobalsForThinLTO();
- return false;
}
-bool llvm::renameModuleForThinLTO(Module &M, const ModuleSummaryIndex &Index,
+void llvm::renameModuleForThinLTO(Module &M, const ModuleSummaryIndex &Index,
bool ClearDSOLocalOnDeclarations,
SetVector<GlobalValue *> *GlobalsToImport) {
FunctionImportGlobalProcessing ThinLTOProcessing(M, Index, GlobalsToImport,
ClearDSOLocalOnDeclarations);
- return ThinLTOProcessing.run();
+ ThinLTOProcessing.run();
}
diff --git a/llvm/tools/llvm-link/llvm-link.cpp b/llvm/tools/llvm-link/llvm-link.cpp
index 34bb6ce30b7668..2adcbb8840a88a 100644
--- a/llvm/tools/llvm-link/llvm-link.cpp
+++ b/llvm/tools/llvm-link/llvm-link.cpp
@@ -449,9 +449,8 @@ static bool linkFiles(const char *argv0, LLVMContext &Context, Linker &L,
}
// Promotion
- if (renameModuleForThinLTO(*M, *Index,
- /*ClearDSOLocalOnDeclarations=*/false))
- return true;
+ renameModuleForThinLTO(*M, *Index,
+ /*ClearDSOLocalOnDeclarations=*/false);
}
if (Verbose)
|
@llvm/pr-subscribers-lto Author: Mircea Trofin (mtrofin) ChangesSame goes for Full diff: https://github.com/llvm/llvm-project/pull/121851.diff 5 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Utils/FunctionImportUtils.h b/llvm/include/llvm/Transforms/Utils/FunctionImportUtils.h
index 749b7b2bb5d869..18cd923d5601d0 100644
--- a/llvm/include/llvm/Transforms/Utils/FunctionImportUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/FunctionImportUtils.h
@@ -120,12 +120,12 @@ class FunctionImportGlobalProcessing {
#endif
}
- bool run();
+ void run();
};
/// Perform in-place global value handling on the given Module for
/// exported local functions renamed and promoted for ThinLTO.
-bool renameModuleForThinLTO(
+void renameModuleForThinLTO(
Module &M, const ModuleSummaryIndex &Index,
bool ClearDSOLocalOnDeclarations,
SetVector<GlobalValue *> *GlobalsToImport = nullptr);
diff --git a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
index 4522f4adcebe68..189f2876b61c0f 100644
--- a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
+++ b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
@@ -160,8 +160,7 @@ generateModuleMap(std::vector<std::unique_ptr<lto::InputFile>> &Modules) {
static void promoteModule(Module &TheModule, const ModuleSummaryIndex &Index,
bool ClearDSOLocalOnDeclarations) {
- if (renameModuleForThinLTO(TheModule, Index, ClearDSOLocalOnDeclarations))
- report_fatal_error("renameModuleForThinLTO failed");
+ renameModuleForThinLTO(TheModule, Index, ClearDSOLocalOnDeclarations);
}
namespace {
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index fde43bb354e83e..c4ef819466df94 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -1950,9 +1950,8 @@ Expected<bool> FunctionImporter::importFunctions(
SrcModule->setPartialSampleProfileRatio(Index);
// Link in the specified functions.
- if (renameModuleForThinLTO(*SrcModule, Index, ClearDSOLocalOnDeclarations,
- &GlobalsToImport))
- return true;
+ renameModuleForThinLTO(*SrcModule, Index, ClearDSOLocalOnDeclarations,
+ &GlobalsToImport);
if (PrintImports) {
for (const auto *GV : GlobalsToImport)
@@ -2026,11 +2025,8 @@ static bool doImportingForModuleForTest(
// Next we need to promote to global scope and rename any local values that
// are potentially exported to other modules.
- if (renameModuleForThinLTO(M, *Index, /*ClearDSOLocalOnDeclarations=*/false,
- /*GlobalsToImport=*/nullptr)) {
- errs() << "Error renaming module\n";
- return true;
- }
+ renameModuleForThinLTO(M, *Index, /*ClearDSOLocalOnDeclarations=*/false,
+ /*GlobalsToImport=*/nullptr);
// Perform the import now.
auto ModuleLoader = [&M](StringRef Identifier) {
diff --git a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
index 766c7501550da5..6f0c349320bc88 100644
--- a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
+++ b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
@@ -331,15 +331,14 @@ void FunctionImportGlobalProcessing::processGlobalsForThinLTO() {
}
}
-bool FunctionImportGlobalProcessing::run() {
+void FunctionImportGlobalProcessing::run() {
processGlobalsForThinLTO();
- return false;
}
-bool llvm::renameModuleForThinLTO(Module &M, const ModuleSummaryIndex &Index,
+void llvm::renameModuleForThinLTO(Module &M, const ModuleSummaryIndex &Index,
bool ClearDSOLocalOnDeclarations,
SetVector<GlobalValue *> *GlobalsToImport) {
FunctionImportGlobalProcessing ThinLTOProcessing(M, Index, GlobalsToImport,
ClearDSOLocalOnDeclarations);
- return ThinLTOProcessing.run();
+ ThinLTOProcessing.run();
}
diff --git a/llvm/tools/llvm-link/llvm-link.cpp b/llvm/tools/llvm-link/llvm-link.cpp
index 34bb6ce30b7668..2adcbb8840a88a 100644
--- a/llvm/tools/llvm-link/llvm-link.cpp
+++ b/llvm/tools/llvm-link/llvm-link.cpp
@@ -449,9 +449,8 @@ static bool linkFiles(const char *argv0, LLVMContext &Context, Linker &L,
}
// Promotion
- if (renameModuleForThinLTO(*M, *Index,
- /*ClearDSOLocalOnDeclarations=*/false))
- return true;
+ renameModuleForThinLTO(*M, *Index,
+ /*ClearDSOLocalOnDeclarations=*/false);
}
if (Verbose)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. probably want to clarify that it isn't that the return value is unused (it is checked), but we never return anything other than false.
renameModuleForThinLTO
renameModuleForThinLTO
Done. Also s/unused/unnecessary in the title. |
See llvm/llvm-project#121851 For LLVM 20+, this function (`renameModuleForThinLTO`) has no return value. For prior versions of LLVM, this never failed, but had a signature which allowed an error value people were handling.
See llvm/llvm-project#121851 For LLVM 20+, this function (`renameModuleForThinLTO`) has no return value. For prior versions of LLVM, this never failed, but had a signature which allowed an error value people were handling.
llvm: Ignore error value that is always false See llvm/llvm-project#121851 For LLVM 20+, this function (`renameModuleForThinLTO`) has no return value. For prior versions of LLVM, this never failed, but had a signature which allowed an error value people were handling. `@rustbot` label: +llvm-main r? `@nikic` Wait a moment before approving while the llvm-main infrastructure picks it up.
Rollup merge of rust-lang#135177 - maurer:rename-module, r=nikic llvm: Ignore error value that is always false See llvm/llvm-project#121851 For LLVM 20+, this function (`renameModuleForThinLTO`) has no return value. For prior versions of LLVM, this never failed, but had a signature which allowed an error value people were handling. `@rustbot` label: +llvm-main r? `@nikic` Wait a moment before approving while the llvm-main infrastructure picks it up.
See llvm/llvm-project#121851 For LLVM 20+, this function (`renameModuleForThinLTO`) has no return value. For prior versions of LLVM, this never failed, but had a signature which allowed an error value people were handling.
Same goes for
FunctionImportGlobalProcessing::run
.The return value was used, but it was always
false
.