-
Notifications
You must be signed in to change notification settings - Fork 15k
[DirectX] Add DXIL validation of llvm.loop metadata
#164292
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
Conversation
|
@llvm/pr-subscribers-backend-directx Author: Finn Plummer (inbelic) ChangesThis pr adds the equivalent validation of This is done as follows:
Resolves: #137387 Patch is 28.66 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/164292.diff 15 Files Affected:
diff --git a/llvm/docs/DirectX/DXILArchitecture.rst b/llvm/docs/DirectX/DXILArchitecture.rst
index bce7fdaa386ed..de72697fc4505 100644
--- a/llvm/docs/DirectX/DXILArchitecture.rst
+++ b/llvm/docs/DirectX/DXILArchitecture.rst
@@ -113,7 +113,7 @@ are grouped into two flows:
The passes to generate DXIL IR follow the flow:
- DXILOpLowering -> DXILPrepare -> DXILTranslateMetadata
+ DXILOpLowering -> DXILPrepare -> DXILTranslateMetadata -> DXILValidateMetadata
Each of these passes has a defined responsibility:
@@ -122,6 +122,8 @@ Each of these passes has a defined responsibility:
namely removing attributes, and inserting bitcasts to allow typed pointers
to be inserted.
#. DXILTranslateMetadata transforms and emits all recognized DXIL Metadata.
+#. DXILValidateMetadata validates all emitted DXIL metadata structures
+ conform to DXIL validation.
The passes to encode DXIL to binary in the DX Container follow the flow:
diff --git a/llvm/lib/Target/DirectX/CMakeLists.txt b/llvm/lib/Target/DirectX/CMakeLists.txt
index 6c079517e22d6..f9900370660dd 100644
--- a/llvm/lib/Target/DirectX/CMakeLists.txt
+++ b/llvm/lib/Target/DirectX/CMakeLists.txt
@@ -35,6 +35,7 @@ add_llvm_target(DirectXCodeGen
DXILResourceImplicitBinding.cpp
DXILShaderFlags.cpp
DXILTranslateMetadata.cpp
+ DXILValidateMetadata.cpp
DXILRootSignature.cpp
DXILLegalizePass.cpp
diff --git a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
index 77334a47dfdbc..abc61d34facc9 100644
--- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
+++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
@@ -36,26 +36,6 @@ using namespace llvm;
using namespace llvm::dxil;
namespace {
-/// A simple Wrapper DiagnosticInfo that generates Module-level diagnostic
-/// for TranslateMetadata pass
-class DiagnosticInfoTranslateMD : public DiagnosticInfo {
-private:
- const Twine &Msg;
- const Module &Mod;
-
-public:
- /// \p M is the module for which the diagnostic is being emitted. \p Msg is
- /// the message to show. Note that this class does not copy this message, so
- /// this reference must be valid for the whole life time of the diagnostic.
- DiagnosticInfoTranslateMD(const Module &M,
- const Twine &Msg LLVM_LIFETIME_BOUND,
- DiagnosticSeverity Severity = DS_Error)
- : DiagnosticInfo(DK_Unsupported, Severity), Msg(Msg), Mod(M) {}
-
- void print(DiagnosticPrinter &DP) const override {
- DP << Mod.getName() << ": " << Msg << '\n';
- }
-};
enum class EntryPropsTag {
ShaderFlags = 0,
@@ -316,16 +296,17 @@ static void translateBranchMetadata(Module &M, Instruction *BBTerminatorInst) {
BBTerminatorInst->setMetadata("hlsl.controlflow.hint", nullptr);
}
-static std::array<unsigned, 6> getCompatibleInstructionMDs(llvm::Module &M) {
+static std::array<unsigned, 7> getCompatibleInstructionMDs(llvm::Module &M) {
return {
M.getMDKindID("dx.nonuniform"), M.getMDKindID("dx.controlflow.hints"),
M.getMDKindID("dx.precise"), llvm::LLVMContext::MD_range,
- llvm::LLVMContext::MD_alias_scope, llvm::LLVMContext::MD_noalias};
+ llvm::LLVMContext::MD_alias_scope, llvm::LLVMContext::MD_noalias,
+ M.getMDKindID("llvm.loop")};
}
static void translateInstructionMetadata(Module &M) {
// construct allowlist of valid metadata node kinds
- std::array<unsigned, 6> DXILCompatibleMDs = getCompatibleInstructionMDs(M);
+ std::array<unsigned, 7> DXILCompatibleMDs = getCompatibleInstructionMDs(M);
for (Function &F : M) {
for (BasicBlock &BB : F) {
@@ -391,9 +372,6 @@ static void translateGlobalMetadata(Module &M, DXILResourceMap &DRM,
uint64_t CombinedMask = ShaderFlags.getCombinedFlags();
EntryFnMDNodes.emplace_back(
emitTopLevelLibraryNode(M, ResourceMD, CombinedMask));
- } else if (MMDI.EntryPropertyVec.size() > 1) {
- M.getContext().diagnose(DiagnosticInfoTranslateMD(
- M, "Non-library shader: One and only one entry expected"));
}
for (const EntryProperties &EntryProp : MMDI.EntryPropertyVec) {
@@ -402,20 +380,9 @@ static void translateGlobalMetadata(Module &M, DXILResourceMap &DRM,
// If ShaderProfile is Library, mask is already consolidated in the
// top-level library node. Hence it is not emitted.
- uint64_t EntryShaderFlags = 0;
- if (MMDI.ShaderProfile != Triple::EnvironmentType::Library) {
- EntryShaderFlags = EntrySFMask;
- if (EntryProp.ShaderStage != MMDI.ShaderProfile) {
- M.getContext().diagnose(DiagnosticInfoTranslateMD(
- M,
- "Shader stage '" +
- Twine(getShortShaderStage(EntryProp.ShaderStage) +
- "' for entry '" + Twine(EntryProp.Entry->getName()) +
- "' different from specified target profile '" +
- Twine(Triple::getEnvironmentTypeName(MMDI.ShaderProfile) +
- "'"))));
- }
- }
+ uint64_t EntryShaderFlags =
+ MMDI.ShaderProfile == Triple::EnvironmentType::Library ? 0
+ : EntrySFMask;
EntryFnMDNodes.emplace_back(emitEntryMD(EntryProp, Signatures, ResourceMD,
EntryShaderFlags,
MMDI.ShaderProfile));
@@ -448,44 +415,33 @@ PreservedAnalyses DXILTranslateMetadata::run(Module &M,
return PreservedAnalyses::all();
}
-namespace {
-class DXILTranslateMetadataLegacy : public ModulePass {
-public:
- static char ID; // Pass identification, replacement for typeid
- explicit DXILTranslateMetadataLegacy() : ModulePass(ID) {}
-
- StringRef getPassName() const override { return "DXIL Translate Metadata"; }
-
- void getAnalysisUsage(AnalysisUsage &AU) const override {
- AU.addRequired<DXILResourceTypeWrapperPass>();
- AU.addRequired<DXILResourceWrapperPass>();
- AU.addRequired<ShaderFlagsAnalysisWrapper>();
- AU.addRequired<DXILMetadataAnalysisWrapperPass>();
- AU.addRequired<RootSignatureAnalysisWrapper>();
- AU.addPreserved<RootSignatureAnalysisWrapper>();
- AU.addPreserved<DXILResourceWrapperPass>();
- AU.addPreserved<DXILMetadataAnalysisWrapperPass>();
- AU.addPreserved<ShaderFlagsAnalysisWrapper>();
- AU.addPreserved<DXILResourceBindingWrapperPass>();
- }
+void DXILTranslateMetadataLegacy::getAnalysisUsage(AnalysisUsage &AU) const {
+ AU.addRequired<DXILResourceTypeWrapperPass>();
+ AU.addRequired<DXILResourceWrapperPass>();
+ AU.addRequired<ShaderFlagsAnalysisWrapper>();
+ AU.addRequired<DXILMetadataAnalysisWrapperPass>();
+ AU.addRequired<RootSignatureAnalysisWrapper>();
+ AU.addPreserved<RootSignatureAnalysisWrapper>();
+ AU.addPreserved<DXILResourceWrapperPass>();
+ AU.addPreserved<DXILMetadataAnalysisWrapperPass>();
+ AU.addPreserved<ShaderFlagsAnalysisWrapper>();
+ AU.addPreserved<DXILResourceBindingWrapperPass>();
+}
- bool runOnModule(Module &M) override {
- DXILResourceMap &DRM =
- getAnalysis<DXILResourceWrapperPass>().getResourceMap();
- DXILResourceTypeMap &DRTM =
- getAnalysis<DXILResourceTypeWrapperPass>().getResourceTypeMap();
- const ModuleShaderFlags &ShaderFlags =
- getAnalysis<ShaderFlagsAnalysisWrapper>().getShaderFlags();
- dxil::ModuleMetadataInfo MMDI =
- getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata();
-
- translateGlobalMetadata(M, DRM, DRTM, ShaderFlags, MMDI);
- translateInstructionMetadata(M);
- return true;
- }
-};
+bool DXILTranslateMetadataLegacy::runOnModule(Module &M) {
+ DXILResourceMap &DRM =
+ getAnalysis<DXILResourceWrapperPass>().getResourceMap();
+ DXILResourceTypeMap &DRTM =
+ getAnalysis<DXILResourceTypeWrapperPass>().getResourceTypeMap();
+ const ModuleShaderFlags &ShaderFlags =
+ getAnalysis<ShaderFlagsAnalysisWrapper>().getShaderFlags();
+ dxil::ModuleMetadataInfo MMDI =
+ getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata();
-} // namespace
+ translateGlobalMetadata(M, DRM, DRTM, ShaderFlags, MMDI);
+ translateInstructionMetadata(M);
+ return true;
+}
char DXILTranslateMetadataLegacy::ID = 0;
diff --git a/llvm/lib/Target/DirectX/DXILTranslateMetadata.h b/llvm/lib/Target/DirectX/DXILTranslateMetadata.h
index 4c1ffac1781e6..cfb8aaa8f98b5 100644
--- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.h
+++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.h
@@ -10,6 +10,7 @@
#define LLVM_TARGET_DIRECTX_DXILTRANSLATEMETADATA_H
#include "llvm/IR/PassManager.h"
+#include "llvm/Pass.h"
namespace llvm {
@@ -20,6 +21,22 @@ class DXILTranslateMetadata : public PassInfoMixin<DXILTranslateMetadata> {
PreservedAnalyses run(Module &M, ModuleAnalysisManager &);
};
+/// Wrapper pass for the legacy pass manager.
+///
+/// This is required because the passes that will depend on this are codegen
+/// passes which run through the legacy pass manager.
+class DXILTranslateMetadataLegacy : public ModulePass {
+public:
+ static char ID; // Pass identification, replacement for typeid
+ explicit DXILTranslateMetadataLegacy() : ModulePass(ID) {}
+
+ StringRef getPassName() const override { return "DXIL Translate Metadata"; }
+
+ void getAnalysisUsage(AnalysisUsage &AU) const override;
+
+ bool runOnModule(Module &M) override;
+};
+
} // namespace llvm
#endif // LLVM_TARGET_DIRECTX_DXILTRANSLATEMETADATA_H
diff --git a/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp b/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp
new file mode 100644
index 0000000000000..275d3686dd7e9
--- /dev/null
+++ b/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp
@@ -0,0 +1,216 @@
+//===- DXILValidateMetadata.cpp - Pass to validate DXIL metadata ----------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "DXILValidateMetadata.h"
+#include "DXILTranslateMetadata.h"
+#include "DirectX.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/Analysis/DXILMetadataAnalysis.h"
+#include "llvm/IR/BasicBlock.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/DiagnosticInfo.h"
+#include "llvm/IR/DiagnosticPrinter.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Metadata.h"
+#include "llvm/IR/Module.h"
+#include "llvm/InitializePasses.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/ErrorHandling.h"
+
+using namespace llvm;
+
+namespace {
+
+/// A simple Wrapper DiagnosticInfo that generates Module-level diagnostic
+/// for the ValidateMetadata pass
+class DiagnosticInfoValidateMD : public DiagnosticInfo {
+private:
+ const Twine &Msg;
+ const Module &Mod;
+
+public:
+ /// \p M is the module for which the diagnostic is being emitted. \p Msg is
+ /// the message to show. Note that this class does not copy this message, so
+ /// this reference must be valid for the whole life time of the diagnostic.
+ DiagnosticInfoValidateMD(const Module &M,
+ const Twine &Msg LLVM_LIFETIME_BOUND,
+ DiagnosticSeverity Severity = DS_Error)
+ : DiagnosticInfo(DK_Unsupported, Severity), Msg(Msg), Mod(M) {}
+
+ void print(DiagnosticPrinter &DP) const override {
+ DP << Mod.getName() << ": " << Msg << '\n';
+ }
+};
+
+static bool reportError(Module &M, Twine Message,
+ DiagnosticSeverity Severity = DS_Error) {
+ M.getContext().diagnose(DiagnosticInfoValidateMD(M, Message, Severity));
+ return true;
+}
+
+static bool reportLoopError(Module &M, Twine Message,
+ DiagnosticSeverity Severity = DS_Error) {
+ return reportError(M, Twine("Invalid \"llvm.loop\" metadata: ") + Message,
+ Severity);
+}
+
+} // namespace
+
+static void validateLoopMetadata(Module &M, MDNode *LoopMD) {
+ // DXIL only accepts the following loop hints:
+ // llvm.loop.unroll.disable, llvm.loop.unroll.full, llvm.loop.unroll.count
+ std::array<StringLiteral, 3> ValidHintNames = {"llvm.loop.unroll.count",
+ "llvm.loop.unroll.disable",
+ "llvm.loop.unroll.full"};
+
+ // llvm.loop metadata must have it's first operand be a self-reference, so we
+ // require at least 1 operand.
+ //
+ // It only makes sense to specify up to 1 of the hints on a branch, so we can
+ // have at most 2 operands.
+
+ if (LoopMD->getNumOperands() != 1 && LoopMD->getNumOperands() != 2) {
+ reportLoopError(M, "Requires exactly 1 or 2 operands");
+ return;
+ }
+
+ if (LoopMD != LoopMD->getOperand(0)) {
+ reportLoopError(M, "First operand must be a self-reference");
+ return;
+ }
+
+ // A node only containing a self-reference is a valid use to denote a loop
+ if (LoopMD->getNumOperands() == 1)
+ return;
+
+ LoopMD = dyn_cast<MDNode>(LoopMD->getOperand(1));
+ if (!LoopMD) {
+ reportLoopError(M, "Second operand must be a metadata node");
+ return;
+ }
+
+ if (LoopMD->getNumOperands() != 1 && LoopMD->getNumOperands() != 2) {
+ reportLoopError(M, "Requires exactly 1 or 2 operands");
+ return;
+ }
+
+ // It is valid to have a chain of self-referential loop metadata nodes so if
+ // we have another self-reference, recurse.
+ //
+ // Eg:
+ // !0 = !{!0, !1}
+ // !1 = !{!1, !2}
+ // !2 = !{"llvm.loop.unroll.disable"}
+ if (LoopMD == LoopMD->getOperand(0))
+ return validateLoopMetadata(M, LoopMD);
+
+ // Otherwise, we are at our base hint metadata node
+ auto *HintStr = dyn_cast<MDString>(LoopMD->getOperand(0));
+ if (!HintStr || !llvm::is_contained(ValidHintNames, HintStr->getString())) {
+ reportLoopError(M,
+ "First operand must be a valid \"llvm.loop.unroll\" hint");
+ return;
+ }
+
+ // Ensure count node is a constant integer value
+ auto ValidCountNode = [](MDNode *HintMD) -> bool {
+ if (HintMD->getNumOperands() == 2)
+ if (auto *CountMD = dyn_cast<ConstantAsMetadata>(HintMD->getOperand(1)))
+ if (isa<ConstantInt>(CountMD->getValue()))
+ return true;
+ return false;
+ };
+
+ if (HintStr->getString() == "llvm.loop.unroll.count" &&
+ !ValidCountNode(LoopMD)) {
+ reportLoopError(M, "Second operand of \"llvm.loop.unroll.count\" "
+ "must be a constant integer");
+ return;
+ }
+}
+
+static void validateInstructionMetadata(Module &M) {
+ unsigned char MDLoopKind = M.getContext().getMDKindID("llvm.loop");
+
+ for (Function &F : M)
+ for (BasicBlock &BB : F)
+ for (Instruction &I : BB) {
+ if (MDNode *LoopMD = I.getMetadata(MDLoopKind))
+ validateLoopMetadata(M, LoopMD);
+ }
+}
+
+static void validateGlobalMetadata(Module &M,
+ const dxil::ModuleMetadataInfo &MMDI) {
+ if (MMDI.ShaderProfile != Triple::EnvironmentType::Library) {
+ if (1 < MMDI.EntryPropertyVec.size())
+ reportError(M, "Non-library shader: One and only one entry expected");
+
+ for (const dxil::EntryProperties &EntryProp : MMDI.EntryPropertyVec)
+ if (EntryProp.ShaderStage != MMDI.ShaderProfile)
+ reportError(
+ M,
+ "Shader stage '" +
+ Twine(Twine(Triple::getEnvironmentTypeName(
+ EntryProp.ShaderStage)) +
+ "' for entry '" + Twine(EntryProp.Entry->getName()) +
+ "' different from specified target profile '" +
+ Twine(Triple::getEnvironmentTypeName(MMDI.ShaderProfile) +
+ "'")));
+ }
+}
+
+PreservedAnalyses DXILValidateMetadata::run(Module &M,
+ ModuleAnalysisManager &MAM) {
+
+ const dxil::ModuleMetadataInfo MMDI = MAM.getResult<DXILMetadataAnalysis>(M);
+ validateGlobalMetadata(M, MMDI);
+ validateInstructionMetadata(M);
+
+ return PreservedAnalyses::all();
+}
+
+namespace {
+class DXILValidateMetadataLegacy : public ModulePass {
+public:
+ static char ID; // Pass identification, replacement for typeid
+ explicit DXILValidateMetadataLegacy() : ModulePass(ID) {}
+
+ StringRef getPassName() const override { return "DXIL Validate Metadata"; }
+
+ void getAnalysisUsage(AnalysisUsage &AU) const override {
+ AU.addRequired<DXILMetadataAnalysisWrapperPass>();
+ AU.addRequired<DXILTranslateMetadataLegacy>();
+ AU.setPreservesAll();
+ }
+
+ bool runOnModule(Module &M) override {
+ dxil::ModuleMetadataInfo MMDI =
+ getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata();
+ validateGlobalMetadata(M, MMDI);
+ validateInstructionMetadata(M);
+ return true;
+ }
+};
+
+} // namespace
+
+char DXILValidateMetadataLegacy::ID = 0;
+
+ModulePass *llvm::createDXILValidateMetadataLegacyPass() {
+ return new DXILValidateMetadataLegacy();
+}
+
+INITIALIZE_PASS_BEGIN(DXILValidateMetadataLegacy, "dxil-validate-metadata",
+ "DXIL Validate Metadata", false, false)
+INITIALIZE_PASS_DEPENDENCY(DXILMetadataAnalysisWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(DXILTranslateMetadataLegacy)
+INITIALIZE_PASS_END(DXILValidateMetadataLegacy, "dxil-validate-metadata",
+ "DXIL validate Metadata", false, false)
diff --git a/llvm/lib/Target/DirectX/DXILValidateMetadata.h b/llvm/lib/Target/DirectX/DXILValidateMetadata.h
new file mode 100644
index 0000000000000..3188afafc8e22
--- /dev/null
+++ b/llvm/lib/Target/DirectX/DXILValidateMetadata.h
@@ -0,0 +1,24 @@
+//===- DXILValidateMetadata.h - Pass to emit DXIL metadata -----*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TARGET_DIRECTX_DXILVALIDATEMETADATA_H
+#define LLVM_TARGET_DIRECTX_DXILVALIDATEMETADATA_H
+
+#include "llvm/IR/PassManager.h"
+
+namespace llvm {
+
+/// A pass that transforms DXIL Intrinsics that don't have DXIL opCodes
+class DXILValidateMetadata : public PassInfoMixin<DXILValidateMetadata> {
+public:
+ PreservedAnalyses run(Module &M, ModuleAnalysisManager &);
+};
+
+} // namespace llvm
+
+#endif // LLVM_TARGET_DIRECTX_DXILVALIDATEMETADATA_H
diff --git a/llvm/lib/Target/DirectX/DirectX.h b/llvm/lib/Target/DirectX/DirectX.h
index e31c2ffa4f761..90546f133c9d2 100644
--- a/llvm/lib/Target/DirectX/DirectX.h
+++ b/llvm/lib/Target/DirectX/DirectX.h
@@ -90,6 +90,12 @@ void initializeDXILTranslateMetadataLegacyPass(PassRegistry &);
/// Pass to emit metadata for DXIL.
ModulePass *createDXILTranslateMetadataLegacyPass();
+/// Initializer for DXILValidateMetadata.
+void initializeDXILValidateMetadataLegacyPass(PassRegistry &);
+
+/// Pass to validate metadata for DXIL.
+ModulePass *createDXILValidateMetadataLegacyPass();
+
/// Pass to pretty print DXIL metadata.
ModulePass *createDXILPrettyPrinterLegacyPass(raw_ostream &OS);
diff --git a/llvm/lib/Target/DirectX/DirectXPassRegistry.def b/llvm/lib/Target/DirectX/DirectXPassRegistry.def
index b4b48a166800e..a69f9764b932b 100644
--- a/llvm/lib/Target/DirectX/DirectXPassRegistry.def
+++ b/llvm/lib/Target/DirectX/DirectXPassRegistry.def
@@ -31,6 +31,7 @@ MODULE_PASS("dxil-intrinsic-expansion", DXILIntrinsicExpansion())
MODULE_PASS("dxil-op-lower", DXILOpLowering())
MODULE_PASS("dxil-pretty-printer", DXILPrettyPrinterPass(dbgs()))
MODULE_PASS("dxil-translate-metadata", DXILTranslateMetadata())
+MODULE_PASS("dxil-validate-metadata", DXILValidateMetadata())
MODULE_PASS("dxil-resource-implicit-binding", DXILResourceImplicitBinding())
MODULE_PASS("dxil-post-optimization-validation", DXILPostOptimizationValidation())
// TODO: rename to print<foo> after NPM switch
diff --git a/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp b/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp
index bcf84403b2c0d..09cbf031ddefe 100644
--- a/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp
+++ b/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp
@@ -27,6 +27,7 @@
#include "DXILRootSignature.h"
#include "DXILShaderFlags.h"
#include "DXILT...
[truncated]
|
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 good overall, just a few comments and suggestions.
0d99254 to
ec970c2
Compare
786aef0 to
f027a93
Compare
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.
LGTM!
it seemed like we could create this nice level of abstraction, however, this will just cause us to write duplicate logic for iterating through the metadata to first transform and then validate. So we may as well transform in place This problem arises as we want to strip certain llvm.loop metadata and validate on the other types
65f3a56 to
f88477c
Compare
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.
LGTM just a nit
This pr adds the equivalent validation of `llvm.loop` metadata that is [done in DXC](https://github.com/microsoft/DirectXShaderCompiler/blob/8f21027f2ad5dcfa63a275cbd278691f2c8fad33/lib/DxilValidation/DxilValidation.cpp#L3010). This is done as follows: - Add `llvm.loop` to the metadata allow-list in `DXILTranslateMetadata` - Iterate through all `llvm.loop` metadata nodes and strip all incompatible ones - Raise an error for ill-formed nodes that are compatible with DXIL Resolves: llvm#137387
This pr adds the equivalent validation of
llvm.loopmetadata that is done in DXC.This is done as follows:
llvm.loopto the metadata allow-list inDXILTranslateMetadatallvm.loopmetadata nodes and strip all incompatible onesResolves: #137387