[DXIL] Consume Metadata Analysis information in passes #108034
Merged
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
|
@llvm/pr-subscribers-backend-directx @llvm/pr-subscribers-llvm-analysis Author: S. Bharadwaj Yadavalli (bharadwajy) Changes
Patch is 36.31 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/108034.diff 13 Files Affected:
diff --git a/llvm/include/llvm/Analysis/DXILMetadataAnalysis.h b/llvm/include/llvm/Analysis/DXILMetadataAnalysis.h
index ed342c28b2d78b..cb442669a24dfe 100644
--- a/llvm/include/llvm/Analysis/DXILMetadataAnalysis.h
+++ b/llvm/include/llvm/Analysis/DXILMetadataAnalysis.h
@@ -21,20 +21,20 @@ class Function;
namespace dxil {
struct EntryProperties {
- const Function *Entry;
+ const Function *Entry{nullptr};
// Specific target shader stage may be specified for entry functions
Triple::EnvironmentType ShaderStage = Triple::UnknownEnvironment;
unsigned NumThreadsX{0}; // X component
unsigned NumThreadsY{0}; // Y component
unsigned NumThreadsZ{0}; // Z component
- EntryProperties(const Function &Fn) : Entry(&Fn) {};
+ EntryProperties(const Function *Fn = nullptr) : Entry(Fn) {};
};
struct ModuleMetadataInfo {
VersionTuple DXILVersion{};
VersionTuple ShaderModelVersion{};
- Triple::EnvironmentType ShaderStage = Triple::UnknownEnvironment;
+ Triple::EnvironmentType ShaderProfile = Triple::UnknownEnvironment;
VersionTuple ValidatorVersion{};
SmallVector<EntryProperties> EntryPropertyVec{};
void print(raw_ostream &OS) const;
diff --git a/llvm/lib/Analysis/DXILMetadataAnalysis.cpp b/llvm/lib/Analysis/DXILMetadataAnalysis.cpp
index cebfe4b84dcdfb..a7f666a3f8b48f 100644
--- a/llvm/lib/Analysis/DXILMetadataAnalysis.cpp
+++ b/llvm/lib/Analysis/DXILMetadataAnalysis.cpp
@@ -27,7 +27,7 @@ static ModuleMetadataInfo collectMetadataInfo(Module &M) {
Triple TT(Triple(M.getTargetTriple()));
MMDAI.DXILVersion = TT.getDXILVersion();
MMDAI.ShaderModelVersion = TT.getOSVersion();
- MMDAI.ShaderStage = TT.getEnvironment();
+ MMDAI.ShaderProfile = TT.getEnvironment();
NamedMDNode *ValidatorVerNode = M.getNamedMetadata("dx.valver");
if (ValidatorVerNode) {
auto *ValVerMD = cast<MDNode>(ValidatorVerNode->getOperand(0));
@@ -42,7 +42,7 @@ static ModuleMetadataInfo collectMetadataInfo(Module &M) {
if (!F.hasFnAttribute("hlsl.shader"))
continue;
- EntryProperties EFP(F);
+ EntryProperties EFP(&F);
// Get "hlsl.shader" attribute
Attribute EntryAttr = F.getFnAttribute("hlsl.shader");
assert(EntryAttr.isValid() &&
@@ -74,8 +74,8 @@ static ModuleMetadataInfo collectMetadataInfo(Module &M) {
void ModuleMetadataInfo::print(raw_ostream &OS) const {
OS << "Shader Model Version : " << ShaderModelVersion.getAsString() << "\n";
OS << "DXIL Version : " << DXILVersion.getAsString() << "\n";
- OS << "Target Shader Stage : " << Triple::getEnvironmentTypeName(ShaderStage)
- << "\n";
+ OS << "Target Shader Stage : "
+ << Triple::getEnvironmentTypeName(ShaderProfile) << "\n";
OS << "Validator Version : " << ValidatorVersion.getAsString() << "\n";
for (const auto &EP : EntryPropertyVec) {
OS << " " << EP.Entry->getName() << "\n";
diff --git a/llvm/lib/Target/DirectX/CMakeLists.txt b/llvm/lib/Target/DirectX/CMakeLists.txt
index f7ae09957996b5..55d32deb49b085 100644
--- a/llvm/lib/Target/DirectX/CMakeLists.txt
+++ b/llvm/lib/Target/DirectX/CMakeLists.txt
@@ -21,7 +21,6 @@ add_llvm_target(DirectXCodeGen
DXContainerGlobals.cpp
DXILFinalizeLinkage.cpp
DXILIntrinsicExpansion.cpp
- DXILMetadata.cpp
DXILOpBuilder.cpp
DXILOpLowering.cpp
DXILPrepare.cpp
diff --git a/llvm/lib/Target/DirectX/DXContainerGlobals.cpp b/llvm/lib/Target/DirectX/DXContainerGlobals.cpp
index aa7769899ff270..0838e2d3faeb79 100644
--- a/llvm/lib/Target/DirectX/DXContainerGlobals.cpp
+++ b/llvm/lib/Target/DirectX/DXContainerGlobals.cpp
@@ -151,16 +151,16 @@ void DXContainerGlobals::addPipelineStateValidationInfo(
dxil::ModuleMetadataInfo &MMI =
getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata();
assert(MMI.EntryPropertyVec.size() == 1 ||
- MMI.ShaderStage == Triple::Library);
+ MMI.ShaderProfile == Triple::Library);
PSV.BaseData.ShaderStage =
- static_cast<uint8_t>(MMI.ShaderStage - Triple::Pixel);
+ static_cast<uint8_t>(MMI.ShaderProfile - Triple::Pixel);
// Hardcoded values here to unblock loading the shader into D3D.
//
// TODO: Lots more stuff to do here!
//
// See issue https://github.com/llvm/llvm-project/issues/96674.
- switch (MMI.ShaderStage) {
+ switch (MMI.ShaderProfile) {
case Triple::Compute:
PSV.BaseData.NumThreadsX = MMI.EntryPropertyVec[0].NumThreadsX;
PSV.BaseData.NumThreadsY = MMI.EntryPropertyVec[0].NumThreadsY;
@@ -170,10 +170,10 @@ void DXContainerGlobals::addPipelineStateValidationInfo(
break;
}
- if (MMI.ShaderStage != Triple::Library)
+ if (MMI.ShaderProfile != Triple::Library)
PSV.EntryName = MMI.EntryPropertyVec[0].Entry->getName();
- PSV.finalize(MMI.ShaderStage);
+ PSV.finalize(MMI.ShaderProfile);
PSV.write(OS);
Constant *Constant =
ConstantDataArray::getString(M.getContext(), Data, /*AddNull*/ false);
diff --git a/llvm/lib/Target/DirectX/DXILMetadata.cpp b/llvm/lib/Target/DirectX/DXILMetadata.cpp
deleted file mode 100644
index 1f5759c3630135..00000000000000
--- a/llvm/lib/Target/DirectX/DXILMetadata.cpp
+++ /dev/null
@@ -1,335 +0,0 @@
-//===- DXILMetadata.cpp - DXIL Metadata helper objects --------------------===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-///
-/// \file This file contains helper objects for working with DXIL metadata.
-///
-//===----------------------------------------------------------------------===//
-
-#include "DXILMetadata.h"
-#include "llvm/IR/Constants.h"
-#include "llvm/IR/IRBuilder.h"
-#include "llvm/IR/Metadata.h"
-#include "llvm/IR/Module.h"
-#include "llvm/Support/VersionTuple.h"
-#include "llvm/TargetParser/Triple.h"
-
-using namespace llvm;
-using namespace llvm::dxil;
-
-ValidatorVersionMD::ValidatorVersionMD(Module &M)
- : Entry(M.getOrInsertNamedMetadata("dx.valver")) {}
-
-void ValidatorVersionMD::update(VersionTuple ValidatorVer) {
- auto &Ctx = Entry->getParent()->getContext();
- IRBuilder<> B(Ctx);
- Metadata *MDVals[2];
- MDVals[0] = ConstantAsMetadata::get(B.getInt32(ValidatorVer.getMajor()));
- MDVals[1] =
- ConstantAsMetadata::get(B.getInt32(ValidatorVer.getMinor().value_or(0)));
-
- if (isEmpty())
- Entry->addOperand(MDNode::get(Ctx, MDVals));
- else
- Entry->setOperand(0, MDNode::get(Ctx, MDVals));
-}
-
-bool ValidatorVersionMD::isEmpty() { return Entry->getNumOperands() == 0; }
-
-VersionTuple ValidatorVersionMD::getAsVersionTuple() {
- if (isEmpty())
- return VersionTuple(1, 0);
- auto *ValVerMD = cast<MDNode>(Entry->getOperand(0));
- auto *MajorMD = mdconst::extract<ConstantInt>(ValVerMD->getOperand(0));
- auto *MinorMD = mdconst::extract<ConstantInt>(ValVerMD->getOperand(1));
- return VersionTuple(MajorMD->getZExtValue(), MinorMD->getZExtValue());
-}
-
-static StringRef getShortShaderStage(Triple::EnvironmentType Env) {
- switch (Env) {
- case Triple::Pixel:
- return "ps";
- case Triple::Vertex:
- return "vs";
- case Triple::Geometry:
- return "gs";
- case Triple::Hull:
- return "hs";
- case Triple::Domain:
- return "ds";
- case Triple::Compute:
- return "cs";
- case Triple::Library:
- return "lib";
- case Triple::Mesh:
- return "ms";
- case Triple::Amplification:
- return "as";
- default:
- break;
- }
- llvm_unreachable("Unsupported environment for DXIL generation.");
- return "";
-}
-
-void dxil::createShaderModelMD(Module &M) {
- NamedMDNode *Entry = M.getOrInsertNamedMetadata("dx.shaderModel");
- Triple TT(M.getTargetTriple());
- VersionTuple Ver = TT.getOSVersion();
- LLVMContext &Ctx = M.getContext();
- IRBuilder<> B(Ctx);
-
- Metadata *Vals[3];
- Vals[0] = MDString::get(Ctx, getShortShaderStage(TT.getEnvironment()));
- Vals[1] = ConstantAsMetadata::get(B.getInt32(Ver.getMajor()));
- Vals[2] = ConstantAsMetadata::get(B.getInt32(Ver.getMinor().value_or(0)));
- Entry->addOperand(MDNode::get(Ctx, Vals));
-}
-
-void dxil::createDXILVersionMD(Module &M) {
- Triple TT(Triple::normalize(M.getTargetTriple()));
- VersionTuple Ver = TT.getDXILVersion();
- LLVMContext &Ctx = M.getContext();
- IRBuilder<> B(Ctx);
- NamedMDNode *Entry = M.getOrInsertNamedMetadata("dx.version");
- Metadata *Vals[2];
- Vals[0] = ConstantAsMetadata::get(B.getInt32(Ver.getMajor()));
- Vals[1] = ConstantAsMetadata::get(B.getInt32(Ver.getMinor().value_or(0)));
- Entry->addOperand(MDNode::get(Ctx, Vals));
-}
-
-static uint32_t getShaderStage(Triple::EnvironmentType Env) {
- return (uint32_t)Env - (uint32_t)llvm::Triple::Pixel;
-}
-
-namespace {
-
-struct EntryProps {
- Triple::EnvironmentType ShaderKind;
- // FIXME: support more shader profiles.
- // See https://github.com/llvm/llvm-project/issues/57927.
- struct {
- unsigned NumThreads[3];
- } CS;
-
- EntryProps(Function &F, Triple::EnvironmentType ModuleShaderKind)
- : ShaderKind(ModuleShaderKind) {
-
- if (ShaderKind == Triple::EnvironmentType::Library) {
- Attribute EntryAttr = F.getFnAttribute("hlsl.shader");
- StringRef EntryProfile = EntryAttr.getValueAsString();
- Triple T("", "", "", EntryProfile);
- ShaderKind = T.getEnvironment();
- }
-
- if (ShaderKind == Triple::EnvironmentType::Compute) {
- auto NumThreadsStr =
- F.getFnAttribute("hlsl.numthreads").getValueAsString();
- SmallVector<StringRef> NumThreads;
- NumThreadsStr.split(NumThreads, ',');
- assert(NumThreads.size() == 3 && "invalid numthreads");
- auto Zip =
- llvm::zip(NumThreads, MutableArrayRef<unsigned>(CS.NumThreads));
- for (auto It : Zip) {
- StringRef Str = std::get<0>(It);
- APInt V;
- [[maybe_unused]] bool Result = Str.getAsInteger(10, V);
- assert(!Result && "Failed to parse numthreads");
-
- unsigned &Num = std::get<1>(It);
- Num = V.getLimitedValue();
- }
- }
- }
-
- MDTuple *emitDXILEntryProps(uint64_t RawShaderFlag, LLVMContext &Ctx,
- bool IsLib) {
- std::vector<Metadata *> MDVals;
-
- if (RawShaderFlag != 0)
- appendShaderFlags(MDVals, RawShaderFlag, Ctx);
-
- // Add shader kind for lib entrys.
- if (IsLib && ShaderKind != Triple::EnvironmentType::Library)
- appendShaderKind(MDVals, Ctx);
-
- if (ShaderKind == Triple::EnvironmentType::Compute)
- appendNumThreads(MDVals, Ctx);
- // FIXME: support more props.
- // See https://github.com/llvm/llvm-project/issues/57948.
- return MDNode::get(Ctx, MDVals);
- }
-
- static MDTuple *emitEntryPropsForEmptyEntry(uint64_t RawShaderFlag,
- LLVMContext &Ctx) {
- if (RawShaderFlag == 0)
- return nullptr;
-
- std::vector<Metadata *> MDVals;
-
- appendShaderFlags(MDVals, RawShaderFlag, Ctx);
- // FIXME: support more props.
- // See https://github.com/llvm/llvm-project/issues/57948.
- return MDNode::get(Ctx, MDVals);
- }
-
-private:
- enum EntryPropsTag {
- ShaderFlagsTag = 0,
- GSStateTag,
- DSStateTag,
- HSStateTag,
- NumThreadsTag,
- AutoBindingSpaceTag,
- RayPayloadSizeTag,
- RayAttribSizeTag,
- ShaderKindTag,
- MSStateTag,
- ASStateTag,
- WaveSizeTag,
- EntryRootSigTag,
- };
-
- void appendNumThreads(std::vector<Metadata *> &MDVals, LLVMContext &Ctx) {
- MDVals.emplace_back(ConstantAsMetadata::get(
- ConstantInt::get(Type::getInt32Ty(Ctx), NumThreadsTag)));
-
- std::vector<Metadata *> NumThreadVals;
- for (auto Num : ArrayRef<unsigned>(CS.NumThreads))
- NumThreadVals.emplace_back(ConstantAsMetadata::get(
- ConstantInt::get(Type::getInt32Ty(Ctx), Num)));
- MDVals.emplace_back(MDNode::get(Ctx, NumThreadVals));
- }
-
- static void appendShaderFlags(std::vector<Metadata *> &MDVals,
- uint64_t RawShaderFlag, LLVMContext &Ctx) {
- MDVals.emplace_back(ConstantAsMetadata::get(
- ConstantInt::get(Type::getInt32Ty(Ctx), ShaderFlagsTag)));
- MDVals.emplace_back(ConstantAsMetadata::get(
- ConstantInt::get(Type::getInt64Ty(Ctx), RawShaderFlag)));
- }
-
- void appendShaderKind(std::vector<Metadata *> &MDVals, LLVMContext &Ctx) {
- MDVals.emplace_back(ConstantAsMetadata::get(
- ConstantInt::get(Type::getInt32Ty(Ctx), ShaderKindTag)));
- MDVals.emplace_back(ConstantAsMetadata::get(
- ConstantInt::get(Type::getInt32Ty(Ctx), getShaderStage(ShaderKind))));
- }
-};
-
-class EntryMD {
- Function &F;
- LLVMContext &Ctx;
- EntryProps Props;
-
-public:
- EntryMD(Function &F, Triple::EnvironmentType ModuleShaderKind)
- : F(F), Ctx(F.getContext()), Props(F, ModuleShaderKind) {}
-
- MDTuple *emitEntryTuple(MDTuple *Resources, uint64_t RawShaderFlag) {
- // FIXME: add signature for profile other than CS.
- // See https://github.com/llvm/llvm-project/issues/57928.
- MDTuple *Signatures = nullptr;
- return emitDXILEntryPointTuple(
- &F, F.getName().str(), Signatures, Resources,
- Props.emitDXILEntryProps(RawShaderFlag, Ctx, /*IsLib*/ false), Ctx);
- }
-
- MDTuple *emitEntryTupleForLib(uint64_t RawShaderFlag) {
- // FIXME: add signature for profile other than CS.
- // See https://github.com/llvm/llvm-project/issues/57928.
- MDTuple *Signatures = nullptr;
- return emitDXILEntryPointTuple(
- &F, F.getName().str(), Signatures,
- /*entry in lib doesn't need resources metadata*/ nullptr,
- Props.emitDXILEntryProps(RawShaderFlag, Ctx, /*IsLib*/ true), Ctx);
- }
-
- // Library will have empty entry metadata which only store the resource table
- // metadata.
- static MDTuple *emitEmptyEntryForLib(MDTuple *Resources,
- uint64_t RawShaderFlag,
- LLVMContext &Ctx) {
- return emitDXILEntryPointTuple(
- nullptr, "", nullptr, Resources,
- EntryProps::emitEntryPropsForEmptyEntry(RawShaderFlag, Ctx), Ctx);
- }
-
-private:
- static MDTuple *emitDXILEntryPointTuple(Function *Fn, const std::string &Name,
- MDTuple *Signatures,
- MDTuple *Resources,
- MDTuple *Properties,
- LLVMContext &Ctx) {
- Metadata *MDVals[5];
- MDVals[0] = Fn ? ValueAsMetadata::get(Fn) : nullptr;
- MDVals[1] = MDString::get(Ctx, Name.c_str());
- MDVals[2] = Signatures;
- MDVals[3] = Resources;
- MDVals[4] = Properties;
- return MDNode::get(Ctx, MDVals);
- }
-};
-} // namespace
-
-void dxil::createEntryMD(Module &M, const uint64_t ShaderFlags) {
- SmallVector<Function *> EntryList;
- for (auto &F : M.functions()) {
- if (!F.hasFnAttribute("hlsl.shader"))
- continue;
- EntryList.emplace_back(&F);
- }
-
- // If there are no entries, do nothing. This is mostly to allow for writing
- // tests with no actual entry functions.
- if (EntryList.empty())
- return;
-
- auto &Ctx = M.getContext();
- // FIXME: generate metadata for resource.
- // See https://github.com/llvm/llvm-project/issues/57926.
- MDTuple *MDResources = nullptr;
- if (auto *NamedResources = M.getNamedMetadata("dx.resources"))
- MDResources = dyn_cast<MDTuple>(NamedResources->getOperand(0));
-
- std::vector<MDNode *> Entries;
- Triple T = Triple(M.getTargetTriple());
- switch (T.getEnvironment()) {
- case Triple::EnvironmentType::Library: {
- // Add empty entry to put resource metadata.
- MDTuple *EmptyEntry =
- EntryMD::emitEmptyEntryForLib(MDResources, ShaderFlags, Ctx);
- Entries.emplace_back(EmptyEntry);
-
- for (Function *Entry : EntryList) {
- EntryMD MD(*Entry, T.getEnvironment());
- Entries.emplace_back(MD.emitEntryTupleForLib(0));
- }
- } break;
- case Triple::EnvironmentType::Compute:
- case Triple::EnvironmentType::Amplification:
- case Triple::EnvironmentType::Mesh:
- case Triple::EnvironmentType::Vertex:
- case Triple::EnvironmentType::Hull:
- case Triple::EnvironmentType::Domain:
- case Triple::EnvironmentType::Geometry:
- case Triple::EnvironmentType::Pixel: {
- assert(EntryList.size() == 1 &&
- "non-lib profiles should only have one entry");
- EntryMD MD(*EntryList.front(), T.getEnvironment());
- Entries.emplace_back(MD.emitEntryTuple(MDResources, ShaderFlags));
- } break;
- default:
- assert(0 && "invalid profile");
- break;
- }
-
- NamedMDNode *EntryPointsNamedMD =
- M.getOrInsertNamedMetadata("dx.entryPoints");
- for (auto *Entry : Entries)
- EntryPointsNamedMD->addOperand(Entry);
-}
diff --git a/llvm/lib/Target/DirectX/DXILMetadata.h b/llvm/lib/Target/DirectX/DXILMetadata.h
deleted file mode 100644
index e05db8d5370dbe..00000000000000
--- a/llvm/lib/Target/DirectX/DXILMetadata.h
+++ /dev/null
@@ -1,43 +0,0 @@
-//===- DXILMetadata.h - DXIL Metadata helper objects ----------------------===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-///
-/// \file This file contains helper objects for working with DXIL metadata.
-///
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_TARGET_DIRECTX_DXILMETADATA_H
-#define LLVM_TARGET_DIRECTX_DXILMETADATA_H
-
-#include <stdint.h>
-
-namespace llvm {
-class Module;
-class NamedMDNode;
-class VersionTuple;
-namespace dxil {
-
-class ValidatorVersionMD {
- NamedMDNode *Entry;
-
-public:
- ValidatorVersionMD(Module &M);
-
- void update(VersionTuple ValidatorVer);
-
- bool isEmpty();
- VersionTuple getAsVersionTuple();
-};
-
-void createShaderModelMD(Module &M);
-void createDXILVersionMD(Module &M);
-void createEntryMD(Module &M, const uint64_t ShaderFlags);
-
-} // namespace dxil
-} // namespace llvm
-
-#endif // LLVM_TARGET_DIRECTX_DXILMETADATA_H
diff --git a/llvm/lib/Target/DirectX/DXILPrepare.cpp b/llvm/lib/Target/DirectX/DXILPrepare.cpp
index f6b7355b936255..eee3aa6ca693a9 100644
--- a/llvm/lib/Target/DirectX/DXILPrepare.cpp
+++ b/llvm/lib/Target/DirectX/DXILPrepare.cpp
@@ -11,7 +11,6 @@
/// Language (DXIL).
//===----------------------------------------------------------------------===//
-#include "DXILMetadata.h"
#include "DXILResourceAnalysis.h"
#include "DXILShaderFlags.h"
#include "DirectX.h"
@@ -173,8 +172,9 @@ class DXILPrepareModule : public ModulePass {
AttrMask.addAttribute(I);
}
- dxil::ValidatorVersionMD ValVerMD(M);
- VersionTuple ValVer = ValVerMD.getAsVersionTuple();
+ const dxil::ModuleMetadataInfo MetadataInfo =
+ getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata();
+ VersionTuple ValVer = MetadataInfo.ValidatorVersion;
bool SkipValidation = ValVer.getMajor() == 0 && ValVer.getMinor() == 0;
for (auto &F : M.functions()) {
@@ -246,9 +246,8 @@ class DXILPrepareModule : public ModulePass {
DXILPrepareModule() : ModulePass(ID) {}
void getAnalysisUsage(AnalysisUsage &AU) const override {
- AU.addPreserved<ShaderFlagsAnalysisWrapper>();
- AU.addPreserved<DXILResourceMDWrapper>();
- AU.addPreserved<DXILMetadataAnalysisWrapperPass>();
+ AU.setPreservesAll();
+ AU.addRequired<DXILMetadataAnalysisWrapperPass>();
}
static char ID; // Pass identification.
};
@@ -258,6 +257,7 @@ char DXILPrepareModule::ID = 0;
INITIALIZE_PASS_BEGIN(DXILPrepareModule, DEBUG_TYPE, "DXIL Prepare Module",
false, false)
+INITIALIZE_PASS_DEPENDENCY(DXILMetadataAnalysisWrapperPass)
INITIALIZE_PASS_END(DXILPrepareModule, DEBUG_TYPE, "DXIL Prepare Module", false,
false)
diff --git a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
index 11cd9df1d1dc42..3c7a9168a59257 100644
--- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
+++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
@@ -7,20 +7,24 @@
//===----------------------------------------------------------------------===//
#include "DXILTranslateMetadata.h"
-#include "DXILMetadata.h"
#include "DXIL...
[truncated]
|
python3kgae
reviewed
Sep 10, 2024
python3kgae
reviewed
Sep 10, 2024
python3kgae
reviewed
Sep 10, 2024
python3kgae
approved these changes
Sep 10, 2024
bogner
reviewed
Sep 10, 2024
python3kgae
reviewed
Sep 13, 2024
python3kgae
approved these changes
Sep 13, 2024
bharadwajy
force-pushed
the
consume-module-metadata-analysis-info
branch
from
6b81f90 to
166a9dd
Compare
damyanp
reviewed
Sep 16, 2024
| AU.addPreserved<ShaderFlagsAnalysisWrapper>(); | ||
| AU.addPreserved<DXILResourceMDWrapper>(); | ||
| AU.addPreserved<DXILMetadataAnalysisWrapperPass>(); | ||
| AU.addRequired<DXILMetadataAnalysisWrapperPass>(); |
There was a problem hiding this comment.
Likely an opportunity for me to learn something:
If we're using setPreservesAll() why do we need to explicitly call addPreserved on any other passes?
There was a problem hiding this comment.
Likely an opportunity for me to learn something:
If we're using
setPreservesAll()why do we need to explicitly calladdPreservedon any other passes?
Correct. Individual addPreserved lines are not needed. Originally deleted but missed in the subsequent rebase.
There was a problem hiding this comment.
Is it correct to say that this preserves all? I'm not sure if any analyses cache information about function attributes, so maybe it's okay there, but we do modify the IR so I doubt it's safe to really say we preserve all analyses...
There was a problem hiding this comment.
Is it correct to say that this preserves all? I'm not sure if any analyses cache information about function attributes, so maybe it's okay there, but we do modify the IR so I doubt it's safe to really say we preserve all analyses...
I share the concern about specifying setPreservedAll() for DXILPrepare pass that actually changes the LLVM IR (as I was discussing offline with @damyanp earlier). As things stand, DXContainerGlobals pass that runs after DXILPrepare pass uses DXILMetadataAnalysis info (i.e., requires the pass) but running it before DXContainerGlobals would produce incomplete info since DXILPrepare deletes function attributes. Hence the unsatisfactory workaround of setPreservedAll() in DXILPrepare.
To address this, I'd like to move attribute removal out of DXILPrepare into DXContainerGlobals pass and (correctly) delete setPreservedAll() from DXILPrepare pass. Is there a known reason for removing function attributes in DXILPrepare pass and not in a later pass?
There was a problem hiding this comment.
I'll plan to move the attribute removal in a follow on PR, if there are no technical reasons to address the issue raised here.
There was a problem hiding this comment.
I still don't understand why we would change this to setPreservesAll. If you just need to preserve DXILMetadataAnalysis then the existing AU.addPreserved<DXILMetadataAnalysisWrapperPass>() call should do that, no?
There was a problem hiding this comment.
I still don't understand why we would change this to
setPreservesAll. If you just need to preserveDXILMetadataAnalysisthen the existingAU.addPreserved<DXILMetadataAnalysisWrapperPass>()call should do that, no?
If DXILMetadataAnalysis pass were not specified as required by any pass running after DXILTranslateMetadata, it would be freed (along with any others that are similarly not required by later passes) after DXILTraslateMetadata. In the current pass order of llc invocation, DXILPrepare runs after DXILTranslateMetadata and does not specify DXILMetadataAnalysis as required, prior to this change. So, the existing AU.addPreserved<DXILMetadataAnalysisWrapperPass>() call in DXILPrepare::getAnalysisUsage() that would only preserve analysis info, if it exists - does not preserve anything. See ouput at the end of this note [1]. Hence, the need to specify DXILMetadataAnalysis as required using AU.addRequired<DXILMetadataAnalysisWrapperPass>().
Regarding the need for calling AU.setPreservesAll(): DXILMetadataAnalysis can be preserved in DXILPrepare pass for use in later passes that require it - such as DXIL Global Emitter - by adding a fine-grained call to AU.addPreserved<DXILMetadataAnalysisWrapperPass>() instead of AU.SetPreservesAll() in DXILPrepare::getAnalysisusage(). However, that would free DXIL Resource analysis and DXIL resource Information pass info needed for later passes. Hence the coarse-grained call AU.setPreservesAll() preserving DXILMetadataAnalysis along with other pass info is used.
A finer-grained control of pass requirements and preservation may be achieved with the following change that avoids the coarse-grained call AU.setPreservesAll(), to both DXILPrepare and DXILMetadataAnalysis passes and may be considered in a follow-on PR, if there is agreement.
diff --git a/llvm/lib/Target/DirectX/DXILPrepare.cpp b/llvm/lib/Target/DirectX/DXILPrepare.cpp
index 1a766c5fb7b4..93d3214bde06 100644
--- a/llvm/lib/Target/DirectX/DXILPrepare.cpp
+++ b/llvm/lib/Target/DirectX/DXILPrepare.cpp
@@ -247,8 +247,10 @@ public:
DXILPrepareModule() : ModulePass(ID) {}
void getAnalysisUsage(AnalysisUsage &AU) const override {
- AU.setPreservesAll();
AU.addRequired<DXILMetadataAnalysisWrapperPass>();
+ AU.addPreserved<DXILResourceMDWrapper>();
+ AU.addPreserved<DXILResourceWrapperPass>();
+ AU.addPreserved<DXILMetadataAnalysisWrapperPass>();
}
static char ID; // Pass identification.
};
diff --git a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
index bea82ccf6006..be370e10df69 100644
--- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
+++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
@@ -379,11 +379,13 @@ public:
StringRef getPassName() const override { return "DXIL Translate Metadata"; }
void getAnalysisUsage(AnalysisUsage &AU) const override {
- AU.setPreservesAll();
AU.addRequired<DXILResourceWrapperPass>();
AU.addRequired<DXILResourceMDWrapper>();
AU.addRequired<ShaderFlagsAnalysisWrapper>();
AU.addRequired<DXILMetadataAnalysisWrapperPass>();
+ AU.addPreserved<DXILResourceWrapperPass>();
+ AU.addPreserved<DXILResourceMDWrapper>();
+ AU.addPreserved<DXILMetadataAnalysisWrapperPass>();
Additional info:
[1] Execution output just with AU.addPreserved<DXILMetadataAnalysisWrapperPass>() in DXILPrepare::getAnalysisUsage()
/Users/bharadwaj/github/llvm-build/hlsl/RelWithDebInfo/bin/llc /Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll --filetype=asm -o /dev/null --debug-pass=Details
-- 'Scalarize vector operations' is not preserving 'DXIL Intrinsic Expansion'
-- 'Scalarize vector operations' is not preserving 'Function Pass Manager'
-- 'DXIL Op Lowering' is not preserving 'DXIL Intrinsic Expansion'
-- 'DXIL Finalize Linkage' is not preserving 'DXIL Op Lowering'
-- 'DXIL Prepare Module' is not preserving 'DXIL Finalize Linkage'
-- 'DXIL Prepare Module' is not preserving 'DXIL Shader Flag Analysis'
-- 'DXIL Prepare Module' is not preserving 'DXIL resource Information'
-- 'DXIL Prepare Module' is not preserving 'DXIL Resource analysis'
-- 'DXIL Prepare Module' is not preserving 'DXIL Translate Metadata'
Pass Arguments: -targetlibinfo -dxil-intrinsic-expansion -domtree -scalarizer -dxil-intrinsic-expansion -dxil-resource -dxil-op-lower -dxil-finalize-linkage -dxil-resource-analysis -dx-shader-flag-analysis -dxil-metadata-analysis -dxil-translate-metadata -dxil-prepare -dxil-resource -dxil-resource-analysis -dxil-pretty-printer -print-module
Target Library Information
ModulePass Manager
DXIL Intrinsic Expansion
-- DXIL Intrinsic Expansion
FunctionPass Manager
Dominator Tree Construction
Scalarize vector operations
-- Dominator Tree Construction
-- Scalarize vector operations
DXIL Intrinsic Expansion
DXIL Resource analysis
DXIL Op Lowering
-- DXIL Intrinsic Expansion
-- DXIL Op Lowering
DXIL Finalize Linkage
-- DXIL Finalize Linkage
DXIL resource Information
DXIL Shader Flag Analysis
DXIL Module Metadata analysis
DXIL Translate Metadata
-- DXIL Resource analysis
-- DXIL resource Information
-- DXIL Shader Flag Analysis
-- DXIL Module Metadata analysis
-- DXIL Translate Metadata
DXIL Prepare Module
-- DXIL Prepare Module
DXIL Resource analysis
DXIL resource Information
DXIL Metadata Pretty Printer
-- DXIL Resource analysis
-- DXIL resource Information
-- DXIL Metadata Pretty Printer
Print Module IR
-- Print Module IR
[2024-09-23 11:58:57.496635000] 0x13dd08950 Executing Pass 'DXIL Intrinsic Expansion' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
[2024-09-23 11:58:57.497185000] 0x13dd08950 Made Modification 'DXIL Intrinsic Expansion' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
0x13dd09cf0 Preserved Analyses: DXIL Resource analysis
-*- 'DXIL Intrinsic Expansion' is the last user of following pass instances. Free these instances
[2024-09-23 11:58:57.497207000] 0x13dd08950 Freeing Pass 'DXIL Intrinsic Expansion' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
[2024-09-23 11:58:57.497218000] 0x13dd08950 Executing Pass 'Function Pass Manager' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
[2024-09-23 11:58:57.497230000] 0x13dd0a7d0 Executing Pass 'Dominator Tree Construction' on Function 'fma'...
[2024-09-23 11:58:57.497244000] 0x13dd0a7d0 Executing Pass 'Scalarize vector operations' on Function 'fma'...
0x13dd09d90 Required Analyses: Dominator Tree Construction
0x13dd09d90 Preserved Analyses: Dominator Tree Construction
-*- 'Scalarize vector operations' is the last user of following pass instances. Free these instances
[2024-09-23 11:58:57.497279000] 0x13dd0a7d0 Freeing Pass 'Dominator Tree Construction' on Function 'fma'...
[2024-09-23 11:58:57.497291000] 0x13dd0a7d0 Freeing Pass 'Scalarize vector operations' on Function 'fma'...
[2024-09-23 11:58:57.497301000] 0x13dd08950 Executing Pass 'DXIL Intrinsic Expansion' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
[2024-09-23 11:58:57.497313000] 0x13dd08950 Made Modification 'DXIL Intrinsic Expansion' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
0x13dd0a3e0 Preserved Analyses: DXIL Resource analysis
-- 'DXIL Intrinsic Expansion' is not preserving 'Function Pass Manager'
[2024-09-23 11:58:57.497333000] 0x13dd08950 Executing Pass 'DXIL Resource analysis' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
[2024-09-23 11:58:57.497345000] 0x13dd08950 Executing Pass 'DXIL Op Lowering' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
0x13dd0a3c0 Required Analyses: DXIL Intrinsic Expansion, DXIL Resource analysis
0x13dd0a3c0 Preserved Analyses: DXIL Resource analysis
-*- 'DXIL Op Lowering' is the last user of following pass instances. Free these instances
[2024-09-23 11:58:57.497378000] 0x13dd08950 Freeing Pass 'DXIL Intrinsic Expansion' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
[2024-09-23 11:58:57.497389000] 0x13dd08950 Freeing Pass 'DXIL Op Lowering' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
[2024-09-23 11:58:57.497399000] 0x13dd08950 Executing Pass 'DXIL Finalize Linkage' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
0x13dd0a9b0 Preserved Analyses: DXIL Resource analysis
-*- 'DXIL Finalize Linkage' is the last user of following pass instances. Free these instances
[2024-09-23 11:58:57.497419000] 0x13dd08950 Freeing Pass 'DXIL Finalize Linkage' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
[2024-09-23 11:58:57.497429000] 0x13dd08950 Executing Pass 'DXIL resource Information' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
[2024-09-23 11:58:57.497441000] 0x13dd08950 Executing Pass 'DXIL Shader Flag Analysis' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
[2024-09-23 11:58:57.497453000] 0x13dd08950 Executing Pass 'DXIL Module Metadata analysis' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
[2024-09-23 11:58:57.497466000] 0x13dd08950 Executing Pass 'DXIL Translate Metadata' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
0x13dd0a9f0 Required Analyses: DXIL Resource analysis, DXIL resource Information, DXIL Shader Flag Analysis, DXIL Module Metadata analysis
[2024-09-23 11:58:57.497524000] 0x13dd08950 Made Modification 'DXIL Translate Metadata' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
-*- 'DXIL Translate Metadata' is the last user of following pass instances. Free these instances
[2024-09-23 11:58:57.497540000] 0x13dd08950 Freeing Pass 'DXIL Resource analysis' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
[2024-09-23 11:58:57.497551000] 0x13dd08950 Freeing Pass 'DXIL resource Information' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
[2024-09-23 11:58:57.497561000] 0x13dd08950 Freeing Pass 'DXIL Shader Flag Analysis' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
[2024-09-23 11:58:57.497572000] 0x13dd08950 Freeing Pass 'DXIL Module Metadata analysis' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
[2024-09-23 11:58:57.497583000] 0x13dd08950 Freeing Pass 'DXIL Translate Metadata' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
[2024-09-23 11:58:57.497594000] 0x13dd08950 Executing Pass 'DXIL Prepare Module' on Module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'...
Assertion failed: (ResultPass && "getAnalysis*() called on an analysis that was not " "'required' by pass!"), function getAnalysisID, file PassAnalysisSupport.h, line 245.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0. Program arguments: /Users/bharadwaj/github/llvm-build/hlsl/RelWithDebInfo/bin/llc /Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll --filetype=asm -o /dev/null --debug-pass=Details
1. Running pass 'DXIL Prepare Module' on module '/Users/bharadwaj/github/llvm-project/llvm/test/CodeGen/DirectX/strip-fn-attrs.ll'.
#0 0x0000000106021ad8 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/Users/bharadwaj/github/llvm-build/hlsl/RelWithDebInfo/bin/llc+0x101481ad8)
<...>
There was a problem hiding this comment.
I'm not arguing that the addRequired<DXILMetadataAnalysisWrapperPass>() isn't necessary, it obviously is. I'm saying we shouldn't change the specific list of preserved passes to setPreservesAll. This pass does not preserve all analyses, since it modifies instructions. The code before your change looks correct to me:
AU.addPreserved<ShaderFlagsAnalysisWrapper>();
AU.addPreserved<DXILResourceMDWrapper>();
AU.addPreserved<DXILMetadataAnalysisWrapperPass>();
AU.addPreserved<DXILResourceWrapperPass>();We can simply add AU.addRequired<DXILMetadataAnalysisWrapperPass>(); after those calls instead of changing those calls to setPreservesAll, since that is incorrect.
damyanp
reviewed
Sep 18, 2024
bogner
reviewed
Sep 18, 2024
| AU.addPreserved<ShaderFlagsAnalysisWrapper>(); | ||
| AU.addPreserved<DXILResourceMDWrapper>(); | ||
| AU.addPreserved<DXILMetadataAnalysisWrapperPass>(); | ||
| AU.addRequired<DXILMetadataAnalysisWrapperPass>(); |
There was a problem hiding this comment.
Is it correct to say that this preserves all? I'm not sure if any analyses cache information about function attributes, so maybe it's okay there, but we do modify the IR so I doubt it's safe to really say we preserve all analyses...
bogner
approved these changes
Sep 23, 2024
and DXILPrepare passes.
a copy-paste. Add necessary function attributes to tests and remove unnecessary check for mumber of entries for non-library shaders in createEntryMD()
shader entries during library profile compilation.
- Delete derived struct ShaderEntryMDInfo and move the functionlaity in its methods as static functions. - Add class DiagnosticInfoModuleFormat derived from DiagnosticInfo for diagnostics reporting in TranslateMetadata pass - Consume Resource metadata information constructed by emitResourceMetadata() - Move generation of metadata for validator version, Shader Model version, DXIL Version into separate functions. - Changes to accept input shader modules with no entry functions - Update/emit named metadata dx.valver only if Metadata Analysis info contains the information; it is not created to with a default value, if it is not.
for non-library target profile compilation. Add a test to verify the check triggers diagnostic report appropriately.
- Rename class DiagnosticInfoModuleFormat as class DiagnosticInfoTranslateMD and enclose it in an anonymous namespace. - Print message directly to DiagnosticPrinter in DiagnosticInfoTranslateMD::print() - Change enum EntryPropsTag to scop[ed enum class - List all ENtryPropsTags in case statement - Moved comment inside constructEntryMetadata() - Changed emitValidatorVersionMD() to return early as appropriate.
Add precise paases preserved in DXILTranslateMetadata instead of setPreservesAll()
bharadwajy
force-pushed
the
consume-module-metadata-analysis-info
branch
from
e20c661 to
dfe7618
Compare
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.
DXILTranslateMetadata::translateMetadata()to consume DXIL Metadata Analysis information. Subsumed intoDXILTranslateMetedata.cppthe functionality inDXILMetadata.*files - that are hence deleted.DXILPreparepass to consume DXIL Metadata Analysis information.ModuleMetadataInfo::ShaderStagetoModuleMetadataInfo::ShaderProfileto better convey what it represents.unknowntarget shader stage specification in triples of a couple of tests.DXILTranslateMetadatapass functionality.