[NFC][WPD] Pass the module analysis manager instead of lambdas #155338
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This stack of pull requests is managed by Graphite. Learn more about stacking. |
mtrofin
changed the title
|
@llvm/pr-subscribers-llvm-transforms Author: Mircea Trofin (mtrofin) ChangesEasier to evolve - if we need more analyses, it becomes clumsy to keep passing around lambdas. Full diff: https://github.com/llvm/llvm-project/pull/155338.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
index aec484f8a18f9..cb98ed838f5d7 100644
--- a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
+++ b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
@@ -68,6 +68,7 @@
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/DebugLoc.h"
#include "llvm/IR/DerivedTypes.h"
+#include "llvm/IR/DiagnosticInfo.h"
#include "llvm/IR/Dominators.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/GlobalAlias.h"
@@ -82,6 +83,7 @@
#include "llvm/IR/Metadata.h"
#include "llvm/IR/Module.h"
#include "llvm/IR/ModuleSummaryIndexYAML.h"
+#include "llvm/IR/PassManager.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Errc.h"
@@ -451,21 +453,21 @@ struct VirtualCallSite {
void
emitRemark(const StringRef OptName, const StringRef TargetName,
- function_ref<OptimizationRemarkEmitter &(Function *)> OREGetter) {
+ function_ref<OptimizationRemarkEmitter &(Function &)> OREGetter) {
Function *F = CB.getCaller();
DebugLoc DLoc = CB.getDebugLoc();
BasicBlock *Block = CB.getParent();
using namespace ore;
- OREGetter(F).emit(OptimizationRemark(DEBUG_TYPE, OptName, DLoc, Block)
- << NV("Optimization", OptName)
- << ": devirtualized a call to "
- << NV("FunctionName", TargetName));
+ OREGetter(*F).emit(OptimizationRemark(DEBUG_TYPE, OptName, DLoc, Block)
+ << NV("Optimization", OptName)
+ << ": devirtualized a call to "
+ << NV("FunctionName", TargetName));
}
void replaceAndErase(
const StringRef OptName, const StringRef TargetName, bool RemarksEnabled,
- function_ref<OptimizationRemarkEmitter &(Function *)> OREGetter,
+ function_ref<OptimizationRemarkEmitter &(Function &)> OREGetter,
Value *New) {
if (RemarksEnabled)
emitRemark(OptName, TargetName, OREGetter);
@@ -570,25 +572,24 @@ void VTableSlotInfo::addCallSite(Value *VTable, CallBase &CB,
struct DevirtModule {
Module &M;
- function_ref<AAResults &(Function &)> AARGetter;
- function_ref<DominatorTree &(Function &)> LookupDomTree;
+ ModuleAnalysisManager &MAM;
+ FunctionAnalysisManager &FAM;
- ModuleSummaryIndex *ExportSummary;
- const ModuleSummaryIndex *ImportSummary;
+ ModuleSummaryIndex *const ExportSummary;
+ const ModuleSummaryIndex *const ImportSummary;
- IntegerType *Int8Ty;
- PointerType *Int8PtrTy;
- IntegerType *Int32Ty;
- IntegerType *Int64Ty;
- IntegerType *IntPtrTy;
+ IntegerType *const Int8Ty;
+ PointerType *const Int8PtrTy;
+ IntegerType *const Int32Ty;
+ IntegerType *const Int64Ty;
+ IntegerType *const IntPtrTy;
/// Sizeless array type, used for imported vtables. This provides a signal
/// to analyzers that these imports may alias, as they do for example
/// when multiple unique return values occur in the same vtable.
- ArrayType *Int8Arr0Ty;
-
- bool RemarksEnabled;
- function_ref<OptimizationRemarkEmitter &(Function *)> OREGetter;
+ ArrayType *const Int8Arr0Ty;
+ const bool RemarksEnabled;
+ std::function<OptimizationRemarkEmitter &(Function &)> OREGetter;
MapVector<VTableSlot, VTableSlotInfo> CallSlots;
// Calls that have already been optimized. We may add a call to multiple
@@ -611,12 +612,11 @@ struct DevirtModule {
std::map<CallInst *, unsigned> NumUnsafeUsesForTypeTest;
PatternList FunctionsToSkip;
- DevirtModule(Module &M, function_ref<AAResults &(Function &)> AARGetter,
- function_ref<OptimizationRemarkEmitter &(Function *)> OREGetter,
- function_ref<DominatorTree &(Function &)> LookupDomTree,
+ DevirtModule(Module &M, ModuleAnalysisManager &MAM,
ModuleSummaryIndex *ExportSummary,
const ModuleSummaryIndex *ImportSummary)
- : M(M), AARGetter(AARGetter), LookupDomTree(LookupDomTree),
+ : M(M), MAM(MAM),
+ FAM(MAM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager()),
ExportSummary(ExportSummary), ImportSummary(ImportSummary),
Int8Ty(Type::getInt8Ty(M.getContext())),
Int8PtrTy(PointerType::getUnqual(M.getContext())),
@@ -624,7 +624,10 @@ struct DevirtModule {
Int64Ty(Type::getInt64Ty(M.getContext())),
IntPtrTy(M.getDataLayout().getIntPtrType(M.getContext(), 0)),
Int8Arr0Ty(ArrayType::get(Type::getInt8Ty(M.getContext()), 0)),
- RemarksEnabled(areRemarksEnabled()), OREGetter(OREGetter) {
+ RemarksEnabled(areRemarksEnabled()),
+ OREGetter([&](Function &F) -> OptimizationRemarkEmitter & {
+ return FAM.getResult<OptimizationRemarkEmitterAnalysis>(F);
+ }) {
assert(!(ExportSummary && ImportSummary));
FunctionsToSkip.init(SkipFunctionNames);
}
@@ -738,10 +741,7 @@ struct DevirtModule {
// Lower the module using the action and summary passed as command line
// arguments. For testing purposes only.
- static bool
- runForTesting(Module &M, function_ref<AAResults &(Function &)> AARGetter,
- function_ref<OptimizationRemarkEmitter &(Function *)> OREGetter,
- function_ref<DominatorTree &(Function &)> LookupDomTree);
+ static bool runForTesting(Module &M, ModuleAnalysisManager &MAM);
};
struct DevirtIndex {
@@ -782,25 +782,13 @@ struct DevirtIndex {
} // end anonymous namespace
PreservedAnalyses WholeProgramDevirtPass::run(Module &M,
- ModuleAnalysisManager &AM) {
- auto &FAM = AM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
- auto AARGetter = [&](Function &F) -> AAResults & {
- return FAM.getResult<AAManager>(F);
- };
- auto OREGetter = [&](Function *F) -> OptimizationRemarkEmitter & {
- return FAM.getResult<OptimizationRemarkEmitterAnalysis>(*F);
- };
- auto LookupDomTree = [&FAM](Function &F) -> DominatorTree & {
- return FAM.getResult<DominatorTreeAnalysis>(F);
- };
+ ModuleAnalysisManager &MAM) {
if (UseCommandLine) {
- if (!DevirtModule::runForTesting(M, AARGetter, OREGetter, LookupDomTree))
+ if (!DevirtModule::runForTesting(M, MAM))
return PreservedAnalyses::all();
return PreservedAnalyses::none();
}
- if (!DevirtModule(M, AARGetter, OREGetter, LookupDomTree, ExportSummary,
- ImportSummary)
- .run())
+ if (!DevirtModule(M, MAM, ExportSummary, ImportSummary).run())
return PreservedAnalyses::all();
return PreservedAnalyses::none();
}
@@ -832,8 +820,8 @@ typeIDVisibleToRegularObj(StringRef TypeID,
// function for the base type and thus only contains a reference to the
// type info (_ZTI). To catch this case we query using the type info
// symbol corresponding to the TypeID.
- std::string typeInfo = ("_ZTI" + TypeID).str();
- return IsVisibleToRegularObj(typeInfo);
+ std::string TypeInfo = ("_ZTI" + TypeID).str();
+ return IsVisibleToRegularObj(TypeInfo);
}
static bool
@@ -842,7 +830,7 @@ skipUpdateDueToValidation(GlobalVariable &GV,
SmallVector<MDNode *, 2> Types;
GV.getMetadata(LLVMContext::MD_type, Types);
- for (auto Type : Types)
+ for (auto *Type : Types)
if (auto *TypeID = dyn_cast<MDString>(Type->getOperand(1).get()))
return typeIDVisibleToRegularObj(TypeID->getString(),
IsVisibleToRegularObj);
@@ -912,9 +900,9 @@ void llvm::getVisibleToRegularObjVtableGUIDs(
ModuleSummaryIndex &Index,
DenseSet<GlobalValue::GUID> &VisibleToRegularObjSymbols,
function_ref<bool(StringRef)> IsVisibleToRegularObj) {
- for (const auto &typeID : Index.typeIdCompatibleVtableMap()) {
- if (typeIDVisibleToRegularObj(typeID.first, IsVisibleToRegularObj))
- for (const TypeIdOffsetVtableInfo &P : typeID.second)
+ for (const auto &TypeID : Index.typeIdCompatibleVtableMap()) {
+ if (typeIDVisibleToRegularObj(TypeID.first, IsVisibleToRegularObj))
+ for (const TypeIdOffsetVtableInfo &P : TypeID.second)
VisibleToRegularObjSymbols.insert(P.VTableVI.getGUID());
}
}
@@ -957,7 +945,7 @@ void llvm::runWholeProgramDevirtOnIndex(
void llvm::updateIndexWPDForExports(
ModuleSummaryIndex &Summary,
- function_ref<bool(StringRef, ValueInfo)> isExported,
+ function_ref<bool(StringRef, ValueInfo)> IsExported,
std::map<ValueInfo, std::vector<VTableSlotSummary>> &LocalWPDTargetsMap) {
for (auto &T : LocalWPDTargetsMap) {
auto &VI = T.first;
@@ -965,7 +953,7 @@ void llvm::updateIndexWPDForExports(
assert(VI.getSummaryList().size() == 1 &&
"Devirt of local target has more than one copy");
auto &S = VI.getSummaryList()[0];
- if (!isExported(S->modulePath(), VI))
+ if (!IsExported(S->modulePath(), VI))
continue;
// It's been exported by a cross module import.
@@ -995,10 +983,7 @@ static Error checkCombinedSummaryForTesting(ModuleSummaryIndex *Summary) {
return ErrorSuccess();
}
-bool DevirtModule::runForTesting(
- Module &M, function_ref<AAResults &(Function &)> AARGetter,
- function_ref<OptimizationRemarkEmitter &(Function *)> OREGetter,
- function_ref<DominatorTree &(Function &)> LookupDomTree) {
+bool DevirtModule::runForTesting(Module &M, ModuleAnalysisManager &MAM) {
std::unique_ptr<ModuleSummaryIndex> Summary =
std::make_unique<ModuleSummaryIndex>(/*HaveGVs=*/false);
@@ -1023,7 +1008,7 @@ bool DevirtModule::runForTesting(
}
bool Changed =
- DevirtModule(M, AARGetter, OREGetter, LookupDomTree,
+ DevirtModule(M, MAM,
ClSummaryAction == PassSummaryAction::Export ? Summary.get()
: nullptr,
ClSummaryAction == PassSummaryAction::Import ? Summary.get()
@@ -1071,7 +1056,7 @@ void DevirtModule::buildTypeIdentifierMap(
}
for (MDNode *Type : Types) {
- auto TypeID = Type->getOperand(1).get();
+ auto *TypeID = Type->getOperand(1).get();
uint64_t Offset =
cast<ConstantInt>(
@@ -1120,7 +1105,7 @@ bool DevirtModule::tryFindVirtualCallTargets(
// Save the symbol used in the vtable to use as the devirtualization
// target.
- auto GV = dyn_cast<GlobalValue>(C);
+ auto *GV = dyn_cast<GlobalValue>(C);
assert(GV);
TargetsForSlot.push_back({GV, &TM});
}
@@ -1284,7 +1269,7 @@ void DevirtModule::applySingleImplDevirt(VTableSlotInfo &SlotInfo,
Apply(P.second);
}
-static bool AddCalls(VTableSlotInfo &SlotInfo, const ValueInfo &Callee) {
+static bool addCalls(VTableSlotInfo &SlotInfo, const ValueInfo &Callee) {
// We can't add calls if we haven't seen a definition
if (Callee.getSummaryList().empty())
return false;
@@ -1359,7 +1344,7 @@ bool DevirtModule::trySingleImplDevirt(
if (ValueInfo TheFnVI = ExportSummary->getValueInfo(TheFn->getGUID()))
// Any needed promotion of 'TheFn' has already been done during
// LTO unit split, so we can ignore return value of AddCalls.
- AddCalls(SlotInfo, TheFnVI);
+ addCalls(SlotInfo, TheFnVI);
Res->TheKind = WholeProgramDevirtResolution::SingleImpl;
Res->SingleImplName = std::string(TheFn->getName());
@@ -1400,7 +1385,7 @@ bool DevirtIndex::trySingleImplDevirt(MutableArrayRef<ValueInfo> TargetsForSlot,
DevirtTargets.insert(TheFn);
auto &S = TheFn.getSummaryList()[0];
- bool IsExported = AddCalls(SlotInfo, TheFn);
+ bool IsExported = addCalls(SlotInfo, TheFn);
if (IsExported)
ExportedGUIDs.insert(TheFn.getGUID());
@@ -1597,7 +1582,7 @@ bool DevirtModule::tryEvaluateFunctionsWithArgs(
// TODO: Skip for now if the vtable symbol was an alias to a function,
// need to evaluate whether it would be correct to analyze the aliasee
// function for this optimization.
- auto Fn = dyn_cast<Function>(Target.Fn);
+ auto *Fn = dyn_cast<Function>(Target.Fn);
if (!Fn)
return false;
@@ -1836,11 +1821,11 @@ bool DevirtModule::tryVirtualConstProp(
// TODO: Skip for now if the vtable symbol was an alias to a function,
// need to evaluate whether it would be correct to analyze the aliasee
// function for this optimization.
- auto Fn = dyn_cast<Function>(TargetsForSlot[0].Fn);
+ auto *Fn = dyn_cast<Function>(TargetsForSlot[0].Fn);
if (!Fn)
return false;
// This only works if the function returns an integer.
- auto RetType = dyn_cast<IntegerType>(Fn->getReturnType());
+ auto *RetType = dyn_cast<IntegerType>(Fn->getReturnType());
if (!RetType)
return false;
unsigned BitWidth = RetType->getBitWidth();
@@ -1871,12 +1856,12 @@ bool DevirtModule::tryVirtualConstProp(
// TODO: Skip for now if the vtable symbol was an alias to a function,
// need to evaluate whether it would be correct to analyze the aliasee
// function for this optimization.
- auto Fn = dyn_cast<Function>(Target.Fn);
+ auto *Fn = dyn_cast<Function>(Target.Fn);
if (!Fn)
return false;
if (Fn->isDeclaration() ||
- !computeFunctionBodyMemoryAccess(*Fn, AARGetter(*Fn))
+ !computeFunctionBodyMemoryAccess(*Fn, FAM.getResult<AAManager>(*Fn))
.doesNotAccessMemory() ||
Fn->arg_empty() || !Fn->arg_begin()->use_empty() ||
Fn->getReturnType() != RetType)
@@ -1992,11 +1977,11 @@ void DevirtModule::rebuildGlobal(VTableBits &B) {
// Build an anonymous global containing the before bytes, followed by the
// original initializer, followed by the after bytes.
- auto NewInit = ConstantStruct::getAnon(
+ auto *NewInit = ConstantStruct::getAnon(
{ConstantDataArray::get(M.getContext(), B.Before.Bytes),
B.GV->getInitializer(),
ConstantDataArray::get(M.getContext(), B.After.Bytes)});
- auto NewGV =
+ auto *NewGV =
new GlobalVariable(M, NewInit->getType(), B.GV->isConstant(),
GlobalVariable::PrivateLinkage, NewInit, "", B.GV);
NewGV->setSection(B.GV->getSection());
@@ -2009,7 +1994,7 @@ void DevirtModule::rebuildGlobal(VTableBits &B) {
// Build an alias named after the original global, pointing at the second
// element (the original initializer).
- auto Alias = GlobalAlias::create(
+ auto *Alias = GlobalAlias::create(
B.GV->getInitializer()->getType(), 0, B.GV->getLinkage(), "",
ConstantExpr::getInBoundsGetElementPtr(
NewInit->getType(), NewGV,
@@ -2050,7 +2035,7 @@ void DevirtModule::scanTypeTestUsers(
// Search for virtual calls based on %p and add them to DevirtCalls.
SmallVector<DevirtCallSite, 1> DevirtCalls;
SmallVector<CallInst *, 1> Assumes;
- auto &DT = LookupDomTree(*CI->getFunction());
+ auto &DT = FAM.getResult<DominatorTreeAnalysis>(*CI->getFunction());
findDevirtualizableCallsForTypeTest(DevirtCalls, Assumes, CI, DT);
Metadata *TypeId =
@@ -2127,7 +2112,7 @@ void DevirtModule::scanTypeCheckedLoadUsers(Function *TypeCheckedLoadFunc) {
SmallVector<Instruction *, 1> LoadedPtrs;
SmallVector<Instruction *, 1> Preds;
bool HasNonCallUses = false;
- auto &DT = LookupDomTree(*CI->getFunction());
+ auto &DT = FAM.getResult<DominatorTreeAnalysis>(*CI->getFunction());
findDevirtualizableCallsForTypeCheckedLoad(DevirtCalls, LoadedPtrs, Preds,
HasNonCallUses, CI, DT);
@@ -2270,7 +2255,7 @@ void DevirtModule::importResolution(VTableSlot Slot, VTableSlotInfo &SlotInfo) {
}
void DevirtModule::removeRedundantTypeTests() {
- auto True = ConstantInt::getTrue(M.getContext());
+ auto *True = ConstantInt::getTrue(M.getContext());
for (auto &&U : NumUnsafeUsesForTypeTest) {
if (U.second == 0) {
U.first->replaceAllUsesWith(True);
@@ -2490,18 +2475,17 @@ bool DevirtModule::run() {
// Generate remarks for each devirtualized function.
for (const auto &DT : DevirtTargets) {
GlobalValue *GV = DT.second;
- auto F = dyn_cast<Function>(GV);
+ auto *F = dyn_cast<Function>(GV);
if (!F) {
- auto A = dyn_cast<GlobalAlias>(GV);
+ auto *A = dyn_cast<GlobalAlias>(GV);
assert(A && isa<Function>(A->getAliasee()));
F = dyn_cast<Function>(A->getAliasee());
assert(F);
}
using namespace ore;
- OREGetter(F).emit(OptimizationRemark(DEBUG_TYPE, "Devirtualized", F)
- << "devirtualized "
- << NV("FunctionName", DT.first));
+ OREGetter(*F).emit(OptimizationRemark(DEBUG_TYPE, "Devirtualized", F)
+ << "devirtualized " << NV("FunctionName", DT.first));
}
}
|
mtrofin
changed the base branch from
main
to
users/mtrofin/08-26-_nfc_wpd_code_style_fixes
mtrofin
force-pushed
the
users/mtrofin/08-26-_nfc_wpd_less_lambdas_where_just_the_analysis_managers_would_work
branch
from
f8fc252 to
929616e
Compare
Yup, see the change below in the stack. |
teresajohnson
approved these changes
Aug 26, 2025
Base automatically changed from
users/mtrofin/08-26-_nfc_wpd_code_style_fixes
to
main
August 26, 2025 19:37
mtrofin
force-pushed
the
users/mtrofin/08-26-_nfc_wpd_less_lambdas_where_just_the_analysis_managers_would_work
branch
from
929616e to
6d370e2
Compare
…ere_just_the_analysis_managers_would_work
mtrofin
deleted the
users/mtrofin/08-26-_nfc_wpd_less_lambdas_where_just_the_analysis_managers_would_work
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Easier to evolve - if we need more analyses, it becomes clumsy to keep passing around lambdas.