[llvm] 5a2888d - Revert "[CGData] Refactor Global Merge Functions (#115750)"
Kyungwoo Lee via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 13 21:24:00 PST 2024
Author: Kyungwoo Lee
Date: 2024-11-13T21:23:16-08:00
New Revision: 5a2888ddbd7a601c8ad6bf7b5f13bf77318e4a4d
URL: https://github.com/llvm/llvm-project/commit/5a2888ddbd7a601c8ad6bf7b5f13bf77318e4a4d
DIFF: https://github.com/llvm/llvm-project/commit/5a2888ddbd7a601c8ad6bf7b5f13bf77318e4a4d.diff
LOG: Revert "[CGData] Refactor Global Merge Functions (#115750)"
This reverts commit d3da78863c7021fa2447a168dc03ad791db69dc6.
Added:
Modified:
llvm/lib/CodeGen/GlobalMergeFunctions.cpp
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/GlobalMergeFunctions.cpp b/llvm/lib/CodeGen/GlobalMergeFunctions.cpp
index 502e1a1f274354..2b367ca87d9008 100644
--- a/llvm/lib/CodeGen/GlobalMergeFunctions.cpp
+++ b/llvm/lib/CodeGen/GlobalMergeFunctions.cpp
@@ -31,6 +31,14 @@ static cl::opt<bool> DisableCGDataForMerging(
"merging is still enabled within a module."),
cl::init(false));
+STATISTIC(NumMismatchedFunctionHash,
+ "Number of mismatched function hash for global merge function");
+STATISTIC(NumMismatchedInstCount,
+ "Number of mismatched instruction count for global merge function");
+STATISTIC(NumMismatchedConstHash,
+ "Number of mismatched const hash for global merge function");
+STATISTIC(NumMismatchedModuleId,
+ "Number of mismatched Module Id for global merge function");
STATISTIC(NumMergedFunctions,
"Number of functions that are actually merged using function hash");
STATISTIC(NumAnalyzedModues, "Number of modules that are analyzed");
@@ -102,7 +110,7 @@ bool isEligibleFunction(Function *F) {
return true;
}
-static bool isEligibleInstructionForConstantSharing(const Instruction *I) {
+static bool isEligibleInstrunctionForConstantSharing(const Instruction *I) {
switch (I->getOpcode()) {
case Instruction::Load:
case Instruction::Store:
@@ -114,15 +122,10 @@ static bool isEligibleInstructionForConstantSharing(const Instruction *I) {
}
}
-// This function takes an instruction, \p I, and an operand index, \p OpIdx.
-// It returns true if the operand should be ignored in the hash computation.
-// If \p OpIdx is out of range based on the other instruction context, it cannot
-// be ignored.
static bool ignoreOp(const Instruction *I, unsigned OpIdx) {
- if (OpIdx >= I->getNumOperands())
- return false;
+ assert(OpIdx < I->getNumOperands() && "Invalid operand index");
- if (!isEligibleInstructionForConstantSharing(I))
+ if (!isEligibleInstrunctionForConstantSharing(I))
return false;
if (!isa<Constant>(I->getOperand(OpIdx)))
@@ -200,9 +203,9 @@ void GlobalMergeFunc::analyze(Module &M) {
struct FuncMergeInfo {
StableFunctionMap::StableFunctionEntry *SF;
Function *F;
- IndexInstrMap *IndexInstruction;
+ std::unique_ptr<IndexInstrMap> IndexInstruction;
FuncMergeInfo(StableFunctionMap::StableFunctionEntry *SF, Function *F,
- IndexInstrMap *IndexInstruction)
+ std::unique_ptr<IndexInstrMap> IndexInstruction)
: SF(SF), F(F), IndexInstruction(std::move(IndexInstruction)) {}
};
@@ -417,75 +420,101 @@ static ParamLocsVecTy computeParamInfo(
bool GlobalMergeFunc::merge(Module &M, const StableFunctionMap *FunctionMap) {
bool Changed = false;
- // Collect stable functions related to the current module.
- DenseMap<stable_hash, SmallVector<std::pair<Function *, FunctionHashInfo>>>
- HashToFuncs;
- auto &Maps = FunctionMap->getFunctionMap();
- for (auto &F : M) {
- if (!isEligibleFunction(&F))
- continue;
- auto FI = llvm::StructuralHashWithDifferences(F, ignoreOp);
- if (Maps.contains(FI.FunctionHash))
- HashToFuncs[FI.FunctionHash].emplace_back(&F, std::move(FI));
- }
-
- for (auto &[Hash, Funcs] : HashToFuncs) {
+ // Build a map from stable function name to function.
+ StringMap<Function *> StableNameToFuncMap;
+ for (auto &F : M)
+ StableNameToFuncMap[get_stable_name(F.getName())] = &F;
+ // Track merged functions
+ DenseSet<Function *> MergedFunctions;
+
+ auto ModId = M.getModuleIdentifier();
+ for (auto &[Hash, SFS] : FunctionMap->getFunctionMap()) {
+ // Parameter locations based on the unique hash sequences
+ // across the candidates.
std::optional<ParamLocsVecTy> ParamLocsVec;
+ Function *MergedFunc = nullptr;
+ std::string MergedModId;
SmallVector<FuncMergeInfo> FuncMergeInfos;
- auto &SFS = Maps.at(Hash);
- assert(!SFS.empty());
- auto &RFS = SFS[0];
-
- // Iterate functions with the same hash.
- for (auto &[F, FI] : Funcs) {
- // Check if the function is compatible with any stable function
- // in terms of the number of instructions and ignored operands.
- if (RFS->InstCount != FI.IndexInstruction->size())
+ for (auto &SF : SFS) {
+ // Get the function from the stable name.
+ auto I = StableNameToFuncMap.find(
+ *FunctionMap->getNameForId(SF->FunctionNameId));
+ if (I == StableNameToFuncMap.end())
+ continue;
+ Function *F = I->second;
+ assert(F);
+ // Skip if the function has been merged before.
+ if (MergedFunctions.count(F))
+ continue;
+ // Consider the function if it is eligible for merging.
+ if (!isEligibleFunction(F))
continue;
- auto hasValidSharedConst =
- [&](StableFunctionMap::StableFunctionEntry *SF) {
- for (auto &[Index, Hash] : *SF->IndexOperandHashMap) {
- auto [InstIndex, OpndIndex] = Index;
- assert(InstIndex < FI.IndexInstruction->size());
- auto *Inst = FI.IndexInstruction->lookup(InstIndex);
- if (!ignoreOp(Inst, OpndIndex))
- return false;
- }
- return true;
- };
- if (!hasValidSharedConst(RFS.get()))
+ auto FI = llvm::StructuralHashWithDifferences(*F, ignoreOp);
+ uint64_t FuncHash = FI.FunctionHash;
+ if (Hash != FuncHash) {
+ ++NumMismatchedFunctionHash;
continue;
+ }
- for (auto &SF : SFS) {
- assert(SF->InstCount == FI.IndexInstruction->size());
- assert(hasValidSharedConst(SF.get()));
- // Check if there is any stable function that is compatiable with the
- // current one.
- if (!checkConstHashCompatible(*SF->IndexOperandHashMap,
- *FI.IndexOperandHashMap))
- continue;
- if (!ParamLocsVec.has_value()) {
- ParamLocsVec = computeParamInfo(SFS);
- LLVM_DEBUG(dbgs() << "[GlobalMergeFunc] Merging hash: " << Hash
- << " with Params " << ParamLocsVec->size() << "\n");
+ if (SF->InstCount != FI.IndexInstruction->size()) {
+ ++NumMismatchedInstCount;
+ continue;
+ }
+ bool HasValidSharedConst = true;
+ for (auto &[Index, Hash] : *SF->IndexOperandHashMap) {
+ auto [InstIndex, OpndIndex] = Index;
+ assert(InstIndex < FI.IndexInstruction->size());
+ auto *Inst = FI.IndexInstruction->lookup(InstIndex);
+ if (!ignoreOp(Inst, OpndIndex)) {
+ HasValidSharedConst = false;
+ break;
}
- if (!checkConstLocationCompatible(*SF, *FI.IndexInstruction,
- *ParamLocsVec))
- continue;
+ }
+ if (!HasValidSharedConst) {
+ ++NumMismatchedConstHash;
+ continue;
+ }
+ if (!checkConstHashCompatible(*SF->IndexOperandHashMap,
+ *FI.IndexOperandHashMap)) {
+ ++NumMismatchedConstHash;
+ continue;
+ }
+ if (!ParamLocsVec.has_value()) {
+ ParamLocsVec = computeParamInfo(SFS);
+ LLVM_DEBUG(dbgs() << "[GlobalMergeFunc] Merging hash: " << Hash
+ << " with Params " << ParamLocsVec->size() << "\n");
+ }
+ if (!checkConstLocationCompatible(*SF, *FI.IndexInstruction,
+ *ParamLocsVec)) {
+ ++NumMismatchedConstHash;
+ continue;
+ }
- // If a stable function matching the current one is found,
- // create a candidate for merging and proceed to the next function.
- FuncMergeInfos.emplace_back(SF.get(), F, FI.IndexInstruction.get());
- break;
+ if (MergedFunc) {
+ // Check if the matched functions fall into the same (first) module.
+ // This module check is not strictly necessary as the functions can move
+ // around. We just want to avoid merging functions from
diff erent
+ // modules than the first one in the function map, as they may not end
+ // up with being ICFed by the linker.
+ if (MergedModId != *FunctionMap->getNameForId(SF->ModuleNameId)) {
+ ++NumMismatchedModuleId;
+ continue;
+ }
+ } else {
+ MergedFunc = F;
+ MergedModId = *FunctionMap->getNameForId(SF->ModuleNameId);
}
+
+ FuncMergeInfos.emplace_back(SF.get(), F, std::move(FI.IndexInstruction));
+ MergedFunctions.insert(F);
}
unsigned FuncMergeInfoSize = FuncMergeInfos.size();
if (FuncMergeInfoSize == 0)
continue;
LLVM_DEBUG(dbgs() << "[GlobalMergeFunc] Merging function count "
- << FuncMergeInfoSize << " for hash: " << Hash << "\n");
+ << FuncMergeInfoSize << " in " << ModId << "\n");
for (auto &FMI : FuncMergeInfos) {
Changed = true;
More information about the llvm-commits
mailing list