-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
base: main
Are you sure you want to change the base?
Store GUIDs in metadata #133682
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -78,6 +78,27 @@ GlobalValue::getGUIDAssumingExternalLinkage(StringRef GlobalIdentifier) { | |
return MD5Hash(GlobalIdentifier); | ||
} | ||
|
||
void GlobalValue::assignGUID() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does this need to be on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like setMetadata is as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) \ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" | ||
|
@@ -791,6 +792,7 @@ PassBuilder::buildFunctionSimplificationPipeline(OptimizationLevel Level, | |
void PassBuilder::addRequiredLTOPreLinkPasses(ModulePassManager &MPM) { | ||
MPM.addPass(CanonicalizeAliasesPass()); | ||
MPM.addPass(NameAnonGlobalPass()); | ||
MPM.addPass(AssignGUIDPass()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe |
||
} | ||
|
||
void PassBuilder::addPreInlinerPasses(ModulePassManager &MPM, | ||
|
@@ -1076,6 +1078,11 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, | |
|
||
ModulePassManager MPM; | ||
|
||
if (Phase != ThinOrFullLTOPhase::ThinLTOPostLink) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 && | ||
|
@@ -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()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)); | ||
|
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(); | ||
} |
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.
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.