Skip to content

[DirectX][NFC] Refactor DXILRootSignature to follow the same pattern as other analysis #146783

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 10 commits 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
10 changes: 5 additions & 5 deletions llvm/lib/Target/DirectX/DXContainerGlobals.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,18 +160,18 @@ void DXContainerGlobals::addRootSignature(Module &M,

assert(MMI.EntryPropertyVec.size() == 1);

auto &RSA = getAnalysis<RootSignatureAnalysisWrapper>();
auto &RSA = getAnalysis<RootSignatureAnalysisWrapper>().getRSInfo();
const Function *EntryFunction = MMI.EntryPropertyVec[0].Entry;
const auto &FuncRs = RSA.find(EntryFunction);
const std::optional<mcdxbc::RootSignatureDesc> &RS =
RSA.getDescForFunction(EntryFunction);

if (FuncRs == RSA.end())
if (!RS)
return;

const RootSignatureDesc &RS = FuncRs->second;
SmallString<256> Data;
raw_svector_ostream OS(Data);

RS.write(OS);
RS->write(OS);
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't RS a reference, why are you using the arrow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the type to make it clearer, but the type is const std::optional<mcdxbc::RootSignatureDesc> &RS


Constant *Constant =
ConstantDataArray::getString(M.getContext(), Data, /*AddNull*/ false);
Expand Down
12 changes: 6 additions & 6 deletions llvm/lib/Target/DirectX/DXILRootSignature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -554,18 +554,17 @@ analyzeModule(Module &M) {

AnalysisKey RootSignatureAnalysis::Key;

SmallDenseMap<const Function *, mcdxbc::RootSignatureDesc>
RootSignatureAnalysis::Result
RootSignatureAnalysis::run(Module &M, ModuleAnalysisManager &AM) {
return analyzeModule(M);
return RootSignatureBindingInfo(analyzeModule(M));
}

//===----------------------------------------------------------------------===//

PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M,
ModuleAnalysisManager &AM) {

SmallDenseMap<const Function *, mcdxbc::RootSignatureDesc> &RSDMap =
AM.getResult<RootSignatureAnalysis>(M);
RootSignatureBindingInfo &RSDMap = AM.getResult<RootSignatureAnalysis>(M);

OS << "Root Signature Definitions"
<< "\n";
Expand Down Expand Up @@ -636,13 +635,14 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M,

//===----------------------------------------------------------------------===//
bool RootSignatureAnalysisWrapper::runOnModule(Module &M) {
FuncToRsMap = analyzeModule(M);
FuncToRsMap = std::make_unique<RootSignatureBindingInfo>(
RootSignatureBindingInfo(analyzeModule(M)));
return false;
}

void RootSignatureAnalysisWrapper::getAnalysisUsage(AnalysisUsage &AU) const {
AU.setPreservesAll();
AU.addRequired<DXILMetadataAnalysisWrapperPass>();
AU.addPreserved<DXILMetadataAnalysisWrapperPass>();
}

char RootSignatureAnalysisWrapper::ID = 0;
Expand Down
46 changes: 35 additions & 11 deletions llvm/lib/Target/DirectX/DXILRootSignature.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
/// Root Signatures.
///
//===----------------------------------------------------------------------===//
#ifndef LLVM_LIB_TARGET_DIRECTX_DXILROOTSIGNATURE_H
#define LLVM_LIB_TARGET_DIRECTX_DXILROOTSIGNATURE_H

#include "llvm/ADT/DenseMap.h"
#include "llvm/Analysis/DXILMetadataAnalysis.h"
Expand All @@ -33,17 +35,44 @@ enum class RootSignatureElementKind {
CBV = 5,
DescriptorTable = 6,
};

class RootSignatureBindingInfo {
private:
SmallDenseMap<const Function *, mcdxbc::RootSignatureDesc> FuncToRsMap;

public:
using iterator =
SmallDenseMap<const Function *, mcdxbc::RootSignatureDesc>::iterator;

RootSignatureBindingInfo() = default;
RootSignatureBindingInfo(
SmallDenseMap<const Function *, mcdxbc::RootSignatureDesc> Map)
: FuncToRsMap(Map) {};

iterator find(const Function *F) { return FuncToRsMap.find(F); }

iterator end() { return FuncToRsMap.end(); }

std::optional<mcdxbc::RootSignatureDesc>
getDescForFunction(const Function *F) {
const auto FuncRs = find(F);
if (FuncRs == end())
return std::nullopt;

return FuncRs->second;
}
};

class RootSignatureAnalysis : public AnalysisInfoMixin<RootSignatureAnalysis> {
friend AnalysisInfoMixin<RootSignatureAnalysis>;
static AnalysisKey Key;

public:
RootSignatureAnalysis() = default;

using Result = SmallDenseMap<const Function *, mcdxbc::RootSignatureDesc>;
using Result = RootSignatureBindingInfo;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why create a type alias here? Why not just put 'RootSignatureBindingInfo' inline? It looks like its only used one place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Result type alias is required in analysis passes or passes that can return something, like in this case, it can contain the Root Signature data representation.


SmallDenseMap<const Function *, mcdxbc::RootSignatureDesc>
run(Module &M, ModuleAnalysisManager &AM);
Result run(Module &M, ModuleAnalysisManager &AM);
};

/// Wrapper pass for the legacy pass manager.
Expand All @@ -52,19 +81,13 @@ class RootSignatureAnalysis : public AnalysisInfoMixin<RootSignatureAnalysis> {
/// passes which run through the legacy pass manager.
class RootSignatureAnalysisWrapper : public ModulePass {
private:
SmallDenseMap<const Function *, mcdxbc::RootSignatureDesc> FuncToRsMap;
std::unique_ptr<RootSignatureBindingInfo> FuncToRsMap;

public:
static char ID;

RootSignatureAnalysisWrapper() : ModulePass(ID) {}

using iterator =
SmallDenseMap<const Function *, mcdxbc::RootSignatureDesc>::iterator;

iterator find(const Function *F) { return FuncToRsMap.find(F); }

iterator end() { return FuncToRsMap.end(); }
RootSignatureBindingInfo &getRSInfo() { return *FuncToRsMap; }

bool runOnModule(Module &M) override;

Expand All @@ -83,3 +106,4 @@ class RootSignatureAnalysisPrinter

} // namespace dxil
} // namespace llvm
#endif