Skip to content

Store GUIDs in metadata #133682

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 5 additions & 31 deletions llvm/include/llvm/Analysis/CtxProfAnalysis.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ class PGOContextualProfile {
// we'll need when we maintain the profiles during IPO transformations.
std::map<GlobalValue::GUID, FunctionInfo> FuncInfo;

/// Get the GUID of this Function if it's defined in this module.
GlobalValue::GUID getDefinedFunctionGUID(const Function &F) const;

// This is meant to be constructed from CtxProfAnalysis, which will also set
// its state piecemeal.
PGOContextualProfile() = default;
Expand All @@ -67,9 +64,7 @@ class PGOContextualProfile {

bool isInSpecializedModule() const;

bool isFunctionKnown(const Function &F) const {
return getDefinedFunctionGUID(F) != 0;
}
bool isFunctionKnown(const Function &F) const { return F.getGUID() != 0; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like getGUID will assert if there is no metadata, so will this function work as intended? Looks like most of the uses are themselves in asserts, but there is one non-assert call.


StringRef getFunctionName(GlobalValue::GUID GUID) const {
auto It = FuncInfo.find(GUID);
Expand All @@ -80,22 +75,22 @@ class PGOContextualProfile {

uint32_t getNumCounters(const Function &F) const {
assert(isFunctionKnown(F));
return FuncInfo.find(getDefinedFunctionGUID(F))->second.NextCounterIndex;
return FuncInfo.find(F.getGUID())->second.NextCounterIndex;
}

uint32_t getNumCallsites(const Function &F) const {
assert(isFunctionKnown(F));
return FuncInfo.find(getDefinedFunctionGUID(F))->second.NextCallsiteIndex;
return FuncInfo.find(F.getGUID())->second.NextCallsiteIndex;
}

uint32_t allocateNextCounterIndex(const Function &F) {
assert(isFunctionKnown(F));
return FuncInfo.find(getDefinedFunctionGUID(F))->second.NextCounterIndex++;
return FuncInfo.find(F.getGUID())->second.NextCounterIndex++;
}

uint32_t allocateNextCallsiteIndex(const Function &F) {
assert(isFunctionKnown(F));
return FuncInfo.find(getDefinedFunctionGUID(F))->second.NextCallsiteIndex++;
return FuncInfo.find(F.getGUID())->second.NextCallsiteIndex++;
}

using ConstVisitor = function_ref<void(const PGOCtxProfContext &)>;
Expand Down Expand Up @@ -185,26 +180,5 @@ class ProfileAnnotator {
~ProfileAnnotator();
};

/// Assign a GUID to functions as metadata. GUID calculation takes linkage into
/// account, which may change especially through and after thinlto. By
/// pre-computing and assigning as metadata, this mechanism is resilient to such
/// changes (as well as name changes e.g. suffix ".llvm." additions).

// FIXME(mtrofin): we can generalize this mechanism to calculate a GUID early in
// the pass pipeline, associate it with any Global Value, and then use it for
// PGO and ThinLTO.
// At that point, this should be moved elsewhere.
class AssignGUIDPass : public PassInfoMixin<AssignGUIDPass> {
public:
explicit AssignGUIDPass() = default;

/// Assign a GUID *if* one is not already assign, as a function metadata named
/// `GUIDMetadataName`.
PreservedAnalyses run(Module &M, ModuleAnalysisManager &MAM);
static const char *GUIDMetadataName;
// This should become GlobalValue::getGUID
static uint64_t getGUID(const Function &F);
};

} // namespace llvm
#endif // LLVM_ANALYSIS_CTXPROFANALYSIS_H
1 change: 1 addition & 0 deletions llvm/include/llvm/IR/FixedMetadataKinds.def
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,4 @@ LLVM_FIXED_MD_KIND(MD_DIAssignID, "DIAssignID", 38)
LLVM_FIXED_MD_KIND(MD_coro_outside_frame, "coro.outside.frame", 39)
LLVM_FIXED_MD_KIND(MD_mmra, "mmra", 40)
LLVM_FIXED_MD_KIND(MD_noalias_addrspace, "noalias.addrspace", 41)
LLVM_FIXED_MD_KIND(MD_unique_id, "unique_id", 42)
15 changes: 12 additions & 3 deletions llvm/include/llvm/IR/GlobalValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -594,11 +594,20 @@ class GlobalValue : public Constant {
/// the GUID computation will assume that the global has external linkage.
static GUID getGUIDAssumingExternalLinkage(StringRef GlobalName);

/// Assign a GUID to this value based on its current name and linkage.
/// This GUID will remain the same even if those change. This method is
/// idempotent -- if a GUID has already been assigned, calling it again
/// will do nothing.
///
/// Users don't need to call this method. They are assigned in batch by a
/// dedicated pass. The pass pipeline should be set up such that GUIDs are
/// always available when needed. If not, the GUID assignment pass should
/// be moved such that they are.
void assignGUID();

/// Return a 64-bit global unique ID constructed from global value name
/// (i.e. returned by getGlobalIdentifier()).
GUID getGUID() const {
return getGUIDAssumingExternalLinkage(getGlobalIdentifier());
}
GUID getGUID() const;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Description needs to be updated


/// @name Materialization
/// Materialization is used to construct functions only as they're needed.
Expand Down
34 changes: 34 additions & 0 deletions llvm/include/llvm/Transforms/Utils/AssignGUID.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
//===-- AssignGUID.h - Unique identifier assignment pass --------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
//
// This file provides a pass which assigns a a GUID (globally unique identifier)
// to every GlobalValue in the module, according to its current name, linkage,
// and originating file.
//
//===----------------------------------------------------------------------===//

#ifndef LLVM_TRANSFORMS_UTILS_ASSIGNGUID_H
#define LLVM_TRANSFORMS_UTILS_ASSIGNGUID_H

#include "llvm/IR/Module.h"
#include "llvm/IR/PassManager.h"

namespace llvm {

class AssignGUIDPass : public PassInfoMixin<AssignGUIDPass> {
public:
AssignGUIDPass() = default;

PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);

static bool isRequired() { return true; }
};

} // end namespace llvm

#endif // LLVM_TRANSFORMS_UTILS_ASSIGNGUID_H
15 changes: 3 additions & 12 deletions llvm/lib/Analysis/CtxProfAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ static cl::opt<bool> ForceIsInSpecializedModule(
cl::desc("Treat the given module as-if it were containing the "
"post-thinlink module containing the root"));

const char *AssignGUIDPass::GUIDMetadataName = "guid";

namespace llvm {
class ProfileAnnotatorImpl final {
friend class ProfileAnnotator;
Expand Down Expand Up @@ -513,7 +511,7 @@ PGOContextualProfile CtxProfAnalysis::run(Module &M,
for (const auto &F : M) {
if (F.isDeclaration())
continue;
auto GUID = AssignGUIDPass::getGUID(F);
auto GUID = F.getGUID();
assert(GUID && "guid not found for defined function");
const auto &Entry = F.begin();
uint32_t MaxCounters = 0; // we expect at least a counter.
Expand Down Expand Up @@ -547,13 +545,6 @@ PGOContextualProfile CtxProfAnalysis::run(Module &M,
return Result;
}

GlobalValue::GUID
PGOContextualProfile::getDefinedFunctionGUID(const Function &F) const {
if (auto It = FuncInfo.find(AssignGUIDPass::getGUID(F)); It != FuncInfo.end())
return It->first;
return 0;
}

CtxProfAnalysisPrinterPass::CtxProfAnalysisPrinterPass(raw_ostream &OS)
: OS(OS), Mode(PrintLevel) {}

Expand Down Expand Up @@ -669,7 +660,7 @@ bool PGOContextualProfile::isInSpecializedModule() const {

void PGOContextualProfile::update(Visitor V, const Function &F) {
assert(isFunctionKnown(F));
GlobalValue::GUID G = getDefinedFunctionGUID(F);
GlobalValue::GUID G = F.getGUID();
for (auto *Node = FuncInfo.find(G)->second.Index.Next; Node;
Node = Node->Next)
V(*reinterpret_cast<PGOCtxProfContext *>(Node));
Expand All @@ -680,7 +671,7 @@ void PGOContextualProfile::visit(ConstVisitor V, const Function *F) const {
return preorderVisit<const PGOCtxProfContext::CallTargetMapTy,
const PGOCtxProfContext>(Profiles.Contexts, V);
assert(isFunctionKnown(*F));
GlobalValue::GUID G = getDefinedFunctionGUID(*F);
GlobalValue::GUID G = F->getGUID();
for (const auto *Node = FuncInfo.find(G)->second.Index.Next; Node;
Node = Node->Next)
V(*reinterpret_cast<const PGOCtxProfContext *>(Node));
Expand Down
21 changes: 21 additions & 0 deletions llvm/lib/IR/Globals.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,27 @@ GlobalValue::getGUIDAssumingExternalLinkage(StringRef GlobalIdentifier) {
return MD5Hash(GlobalIdentifier);
}

void GlobalValue::assignGUID() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this need to be on GlobalValue? It can just be a detail of the implementation of AssignGUIDPass, and then there's no confusion on the user side about whether it's something they should call or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getMetadata is protected, so it does need to be on GlobalValue

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like setMetadata is as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would be better to make AssignGUIDPass::run a friend so that other users cannot access this functionality.

if (getMetadata(LLVMContext::MD_unique_id) != nullptr)
return;

const GUID G =
GlobalValue::getGUIDAssumingExternalLinkage(getGlobalIdentifier());
setMetadata(
LLVMContext::MD_unique_id,
MDNode::get(getContext(), {ConstantAsMetadata::get(ConstantInt::get(
Type::getInt64Ty(getContext()), G))}));
}

GlobalValue::GUID GlobalValue::getGUID() const {
auto *MD = getMetadata(LLVMContext::MD_unique_id);
assert(MD != nullptr && "GUID was not assigned before calling GetGUID()");
return cast<ConstantInt>(cast<ConstantAsMetadata>(MD->getOperand(0))
->getValue()
->stripPointerCasts())
->getZExtValue();
}

void GlobalValue::removeFromParent() {
switch (getValueID()) {
#define HANDLE_GLOBAL_VALUE(NAME) \
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Passes/PassBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@
#include "llvm/Transforms/Scalar/TailRecursionElimination.h"
#include "llvm/Transforms/Scalar/WarnMissedTransforms.h"
#include "llvm/Transforms/Utils/AddDiscriminators.h"
#include "llvm/Transforms/Utils/AssignGUID.h"
#include "llvm/Transforms/Utils/AssumeBundleBuilder.h"
#include "llvm/Transforms/Utils/BreakCriticalEdges.h"
#include "llvm/Transforms/Utils/CanonicalizeAliases.h"
Expand Down
9 changes: 7 additions & 2 deletions llvm/lib/Passes/PassBuilderPipelines.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@
#include "llvm/Transforms/Scalar/TailRecursionElimination.h"
#include "llvm/Transforms/Scalar/WarnMissedTransforms.h"
#include "llvm/Transforms/Utils/AddDiscriminators.h"
#include "llvm/Transforms/Utils/AssignGUID.h"
#include "llvm/Transforms/Utils/AssumeBundleBuilder.h"
#include "llvm/Transforms/Utils/CanonicalizeAliases.h"
#include "llvm/Transforms/Utils/CountVisits.h"
Expand Down Expand Up @@ -791,6 +792,7 @@ PassBuilder::buildFunctionSimplificationPipeline(OptimizationLevel Level,
void PassBuilder::addRequiredLTOPreLinkPasses(ModulePassManager &MPM) {
MPM.addPass(CanonicalizeAliasesPass());
MPM.addPass(NameAnonGlobalPass());
MPM.addPass(AssignGUIDPass());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be added as early as possible, so that (for example) PGO passes can use it. CtxProf definitely needs the GUID present.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(after reading further below) it seems like the pass is used to ensure there are GUIDs. Should we call it "EnsureGUIDs" then?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that just running it once as early as necessary is the way to go. I must admit I'm a bit lost amongst all the pass construction methods -- can you recommend the best place to put it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe buildPerModuleDefaultPipeline should be it.

}

void PassBuilder::addPreInlinerPasses(ModulePassManager &MPM,
Expand Down Expand Up @@ -1076,6 +1078,11 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,

ModulePassManager MPM;

if (Phase != ThinOrFullLTOPhase::ThinLTOPostLink) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll also want to do this at O0, where this function isn't called, for the supported case of ThinLTO + O0.

MPM.addPass(NameAnonGlobalPass());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently this is invoked at the very end of the pre-LTO pipelines (just before we will build the module summary) via addRequiredLTOPreLinkPasses. If it is going to be done early then it can presumably be removed from there. Is it possible for an anon global to be introduced in between this point and the end of the pre-LTO pipeline where we build the summary?

MPM.addPass(AssignGUIDPass());
}

// Place pseudo probe instrumentation as the first pass of the pipeline to
// minimize the impact of optimization changes.
if (PGOOpt && PGOOpt->PseudoProbeForProfiling &&
Expand Down Expand Up @@ -1239,8 +1246,6 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
// In pre-link, we just want the instrumented IR. We use the contextual
// profile in the post-thinlink phase.
// The instrumentation will be removed in post-thinlink after IPO.
// FIXME(mtrofin): move AssignGUIDPass if there is agreement to use this
// mechanism for GUIDs.
MPM.addPass(AssignGUIDPass());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can / should it be removed from here?

if (IsCtxProfUse) {
MPM.addPass(PGOCtxProfFlatteningPass(/*IsPreThinlink=*/true));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ PreservedAnalyses PGOCtxProfFlatteningPass::run(Module &M,
"Function has unreacheable basic blocks. The expectation was that "
"DCE was run before.");

auto It = FlattenedProfile.find(AssignGUIDPass::getGUID(F));
auto It = FlattenedProfile.find(F.getGUID());
// If this function didn't appear in the contextual profile, it's cold.
if (It == FlattenedProfile.end())
clearColdFunctionProfile(F);
Expand Down
3 changes: 1 addition & 2 deletions llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,7 @@ bool CtxInstrumentationLowerer::lowerFunction(Function &F) {
assert(Mark->getIndex()->isZero());

IRBuilder<> Builder(Mark);
Guid = Builder.getInt64(
AssignGUIDPass::getGUID(cast<Function>(*Mark->getNameValue())));
Guid = Builder.getInt64(cast<Function>(*Mark->getNameValue()).getGUID());
// The type of the context of this function is now knowable since we have
// NumCallsites and NumCounters. We delcare it here because it's more
// convenient - we have the Builder.
Expand Down
34 changes: 34 additions & 0 deletions llvm/lib/Transforms/Utils/AssignGUID.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
//===-- AssignGUID.cpp - Unique identifier assignment pass ------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
//
// This file provides a pass which assigns a a GUID (globally unique identifier)
// to every GlobalValue in the module, according to its current name, linkage,
// and originating file.
//
//===----------------------------------------------------------------------===//

#include "llvm/Transforms/Utils/AssignGUID.h"
#include "llvm/Support/Debug.h"

using namespace llvm;

PreservedAnalyses AssignGUIDPass::run(Module &M, ModuleAnalysisManager &AM) {
for (auto &GV : M.globals()) {
if (GV.isDeclaration())
continue;
GV.assignGUID();
dbgs() << "[Added GUID to GV:] " << GV.getName() << "\n";
}
for (auto &F : M.functions()) {
if (F.isDeclaration())
continue;
F.assignGUID();
dbgs() << "[Added GUID to F:] " << F.getName() << "\n";
}
return PreservedAnalyses::none();
}
1 change: 1 addition & 0 deletions llvm/lib/Transforms/Utils/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ add_llvm_component_library(LLVMTransformUtils
AddDiscriminators.cpp
AMDGPUEmitPrintf.cpp
ASanStackFrameLayout.cpp
AssignGUID.cpp
AssumeBundleBuilder.cpp
BasicBlockUtils.cpp
BreakCriticalEdges.cpp
Expand Down
5 changes: 2 additions & 3 deletions llvm/lib/Transforms/Utils/CallPromotionUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -616,11 +616,11 @@ CallBase *llvm::promoteCallWithIfThenElse(CallBase &CB, Function &Callee,
IndirectBBIns->setIndex(IndirectID);
IndirectBBIns->insertInto(&IndirectBB, IndirectBB.getFirstInsertionPt());

const GlobalValue::GUID CalleeGUID = AssignGUIDPass::getGUID(Callee);
const GlobalValue::GUID CalleeGUID = Callee.getGUID();
const uint32_t NewCountersSize = IndirectID + 1;

auto ProfileUpdater = [&](PGOCtxProfContext &Ctx) {
assert(Ctx.guid() == AssignGUIDPass::getGUID(Caller));
assert(Ctx.guid() == Caller.getGUID());
assert(NewCountersSize - 2 == Ctx.counters().size());
// All the ctx-es belonging to a function must have the same size counters.
Ctx.resizeCounters(NewCountersSize);
Expand Down Expand Up @@ -655,7 +655,6 @@ CallBase *llvm::promoteCallWithIfThenElse(CallBase &CB, Function &Callee,
// times, and the indirect BB, IndirectCount times
Ctx.counters()[DirectID] = DirectCount;
Ctx.counters()[IndirectID] = IndirectCount;

};
CtxProf.update(ProfileUpdater, Caller);
return &DirectCall;
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Transforms/Utils/InlineFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2373,7 +2373,7 @@ llvm::InlineResult llvm::InlineFunction(CallBase &CB, InlineFunctionInfo &IFI,
// Get some preliminary data about the callsite before it might get inlined.
// Inlining shouldn't delete the callee, but it's cleaner (and low-cost) to
// get this data upfront and rely less on InlineFunction's behavior.
const auto CalleeGUID = AssignGUIDPass::getGUID(Callee);
const auto CalleeGUID = Callee.getGUID();
auto *CallsiteIDIns = CtxProfAnalysis::getCallsiteInstrumentation(CB);
const auto CallsiteID =
static_cast<uint32_t>(CallsiteIDIns->getIndex()->getZExtValue());
Expand All @@ -2398,7 +2398,7 @@ llvm::InlineResult llvm::InlineFunction(CallBase &CB, InlineFunctionInfo &IFI,
const uint32_t NewCountersSize = CtxProf.getNumCounters(Caller);

auto Updater = [&](PGOCtxProfContext &Ctx) {
assert(Ctx.guid() == AssignGUIDPass::getGUID(Caller));
assert(Ctx.guid() == Caller.getGUID());
const auto &[CalleeCounterMap, CalleeCallsiteMap] = IndicesMaps;
assert(
(Ctx.counters().size() +
Expand Down
Loading