https://github.com/inbelic created https://github.com/llvm/llvm-project/pull/164292
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: - Introduce a `DXILValidateMetadata` pass into the DirectX backend - Move any validation done in `DXILTranslateMetadata` into this new pass - Add `llvm.loop` to the metadata whitelist in `DXILTranslateMetadata` - Add the equivalent checks done for DXC into the new pass Resolves: https://github.com/llvm/llvm-project/issues/137387 >From a63874016767597c3d4bc4152524e130ab51693a Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Fri, 17 Oct 2025 10:17:32 -0700 Subject: [PATCH 1/4] [DirectX] add a DXILValidateMetadata pass --- llvm/docs/DirectX/DXILArchitecture.rst | 4 +- llvm/lib/Target/DirectX/CMakeLists.txt | 1 + .../Target/DirectX/DXILTranslateMetadata.cpp | 61 ++++++------- .../Target/DirectX/DXILTranslateMetadata.h | 17 ++++ .../Target/DirectX/DXILValidateMetadata.cpp | 90 +++++++++++++++++++ .../lib/Target/DirectX/DXILValidateMetadata.h | 24 +++++ llvm/lib/Target/DirectX/DirectX.h | 6 ++ .../Target/DirectX/DirectXPassRegistry.def | 1 + .../Target/DirectX/DirectXTargetMachine.cpp | 3 + llvm/test/CodeGen/DirectX/llc-pipeline.ll | 1 + 10 files changed, 171 insertions(+), 37 deletions(-) create mode 100644 llvm/lib/Target/DirectX/DXILValidateMetadata.cpp create mode 100644 llvm/lib/Target/DirectX/DXILValidateMetadata.h 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..98cda2e1223f6 100644 --- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp +++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp @@ -448,44 +448,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..199a34b8c2616 --- /dev/null +++ b/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp @@ -0,0 +1,90 @@ +//===- 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/Twine.h" +#include "llvm/IR/BasicBlock.h" +#include "llvm/IR/DiagnosticInfo.h" +#include "llvm/IR/DiagnosticPrinter.h" +#include "llvm/IR/Metadata.h" +#include "llvm/IR/Module.h" +#include "llvm/InitializePasses.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'; + } +}; + +} // namespace + +static void validateInstructionMetadata(Module &M) { + llvm::errs() << "hello from new pass!\n"; +} + +PreservedAnalyses DXILValidateMetadata::run(Module &M, + ModuleAnalysisManager &MAM) { + 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<DXILTranslateMetadataLegacy>(); + AU.setPreservesAll(); + } + + bool runOnModule(Module &M) override { + validateInstructionMetadata(); + 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(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 "DXILTranslateMetadata.h" +#include "DXILValidateMetadata.h" #include "DXILWriter/DXILWriterPass.h" #include "DirectX.h" #include "DirectXSubtarget.h" @@ -70,6 +71,7 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeDirectXTarget() { initializeDXILResourceAccessLegacyPass(*PR); initializeDXILResourceImplicitBindingLegacyPass(*PR); initializeDXILTranslateMetadataLegacyPass(*PR); + initializeDXILValidateMetadataLegacyPass(*PR); initializeDXILPostOptimizationValidationLegacyPass(*PR); initializeShaderFlagsAnalysisWrapperPass(*PR); initializeRootSignatureAnalysisWrapperPass(*PR); @@ -122,6 +124,7 @@ class DirectXPassConfig : public TargetPassConfig { addPass(createDXILLegalizeLegacyPass()); addPass(createDXILResourceImplicitBindingLegacyPass()); addPass(createDXILTranslateMetadataLegacyPass()); + addPass(createDXILValidateMetadataLegacyPass()); addPass(createDXILPostOptimizationValidationLegacyPass()); addPass(createDXILOpLoweringLegacyPass()); addPass(createDXILPrepareModulePass()); diff --git a/llvm/test/CodeGen/DirectX/llc-pipeline.ll b/llvm/test/CodeGen/DirectX/llc-pipeline.ll index d265826cd2469..13b502475b283 100644 --- a/llvm/test/CodeGen/DirectX/llc-pipeline.ll +++ b/llvm/test/CodeGen/DirectX/llc-pipeline.ll @@ -42,6 +42,7 @@ ; CHECK-NEXT: DXIL Shader Flag Analysis ; CHECK-NEXT: DXIL Root Signature Analysis ; CHECK-NEXT: DXIL Translate Metadata +; CHECK-NEXT: DXIL Validate Metadata ; CHECK-NEXT: DXIL Post Optimization Validation ; CHECK-NEXT: DXIL Op Lowering ; CHECK-NEXT: DXIL Prepare Module >From 63111804194f7e6134dcad0f59b97369286862fb Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Mon, 20 Oct 2025 09:33:37 -0700 Subject: [PATCH 2/4] nfc: factor out validations in translate metadata --- .../Target/DirectX/DXILTranslateMetadata.cpp | 40 ++----------------- .../Target/DirectX/DXILValidateMetadata.cpp | 36 ++++++++++++++++- .../Metadata/multiple-entries-cs-error.ll | 2 +- .../DirectX/Metadata/target-profile-error.ll | 4 +- 4 files changed, 41 insertions(+), 41 deletions(-) diff --git a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp index 98cda2e1223f6..4ef025d5fd09a 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, @@ -391,9 +371,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 +379,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)); diff --git a/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp b/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp index 199a34b8c2616..84f17770166be 100644 --- a/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp +++ b/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp @@ -10,6 +10,7 @@ #include "DXILTranslateMetadata.h" #include "DirectX.h" #include "llvm/ADT/Twine.h" +#include "llvm/Analysis/DXILMetadataAnalysis.h" #include "llvm/IR/BasicBlock.h" #include "llvm/IR/DiagnosticInfo.h" #include "llvm/IR/DiagnosticPrinter.h" @@ -43,14 +44,43 @@ class DiagnosticInfoValidateMD : public DiagnosticInfo { } }; +static bool reportError(Module &M, Twine Message, + DiagnosticSeverity Severity = DS_Error) { + M.getContext().diagnose(DiagnosticInfoValidateMD(M, Message, Severity)); + return true; +} + } // namespace static void validateInstructionMetadata(Module &M) { llvm::errs() << "hello from new pass!\n"; } +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(); @@ -65,12 +95,15 @@ class DXILValidateMetadataLegacy : public ModulePass { 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 { - validateInstructionMetadata(); + dxil::ModuleMetadataInfo MMDI = + getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata(); + validateGlobalMetadata(M, MMDI); return true; } }; @@ -85,6 +118,7 @@ ModulePass *llvm::createDXILValidateMetadataLegacyPass() { 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/test/CodeGen/DirectX/Metadata/multiple-entries-cs-error.ll b/llvm/test/CodeGen/DirectX/Metadata/multiple-entries-cs-error.ll index 9697d4389a888..cba10336f7817 100644 --- a/llvm/test/CodeGen/DirectX/Metadata/multiple-entries-cs-error.ll +++ b/llvm/test/CodeGen/DirectX/Metadata/multiple-entries-cs-error.ll @@ -1,4 +1,4 @@ -; RUN: not opt -S -S -dxil-translate-metadata %s 2>&1 | FileCheck %s +; RUN: not opt -S -dxil-validate-metadata %s 2>&1 | FileCheck %s target triple = "dxil-pc-shadermodel6.8-compute" ; CHECK: Non-library shader: One and only one entry expected diff --git a/llvm/test/CodeGen/DirectX/Metadata/target-profile-error.ll b/llvm/test/CodeGen/DirectX/Metadata/target-profile-error.ll index 671406cb5d364..13e28cfebf9af 100644 --- a/llvm/test/CodeGen/DirectX/Metadata/target-profile-error.ll +++ b/llvm/test/CodeGen/DirectX/Metadata/target-profile-error.ll @@ -1,8 +1,8 @@ -; RUN: not opt -S -dxil-translate-metadata %s 2>&1 | FileCheck %s +; RUN: not opt -S -dxil-validate-metadata %s 2>&1 | FileCheck %s target triple = "dxil-pc-shadermodel6.6-pixel" -; CHECK: Shader stage 'cs' for entry 'entry' different from specified target profile 'pixel' +; CHECK: Shader stage 'compute' for entry 'entry' different from specified target profile 'pixel' define void @entry() #0 { entry: >From dfa87b60ce97ca952f25cde2ee9ab7e51d65f878 Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Mon, 20 Oct 2025 09:48:57 -0700 Subject: [PATCH 3/4] [DirectX] add "llvm.loop" to metadata whitelist --- llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp index 4ef025d5fd09a..abc61d34facc9 100644 --- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp +++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp @@ -296,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) { >From 786aef0b7df2508d53aba0f24e5100e60224240f Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Mon, 20 Oct 2025 09:49:18 -0700 Subject: [PATCH 4/4] [DirectX] add DXIL validation of "llvm.loop" metadata --- .../Target/DirectX/DXILValidateMetadata.cpp | 94 +++++++++++- .../CodeGen/DirectX/Metadata/loop-md-errs.ll | 135 ++++++++++++++++++ .../CodeGen/DirectX/Metadata/loop-md-valid.ll | 31 ++++ .../CodeGen/DirectX/metadata-stripping.ll | 7 +- 4 files changed, 262 insertions(+), 5 deletions(-) create mode 100644 llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll create mode 100644 llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll diff --git a/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp b/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp index 84f17770166be..275d3686dd7e9 100644 --- a/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp +++ b/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp @@ -9,14 +9,19 @@ #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; @@ -50,10 +55,96 @@ static bool reportError(Module &M, Twine Message, 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) { - llvm::errs() << "hello from new pass!\n"; + 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, @@ -104,6 +195,7 @@ class DXILValidateMetadataLegacy : public ModulePass { dxil::ModuleMetadataInfo MMDI = getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata(); validateGlobalMetadata(M, MMDI); + validateInstructionMetadata(M); return true; } }; diff --git a/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll b/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll new file mode 100644 index 0000000000000..1bf16a6559d82 --- /dev/null +++ b/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll @@ -0,0 +1,135 @@ +; RUN: split-file %s %t +; RUN: not opt -S --dxil-validate-metadata %t/args.ll 2>&1 | FileCheck %t/args.ll +; RUN: not opt -S --dxil-validate-metadata %t/not-ref.ll 2>&1 | FileCheck %t/not-ref.ll +; RUN: not opt -S --dxil-validate-metadata %t/not-md.ll 2>&1 | FileCheck %t/not-md.ll +; RUN: not opt -S --dxil-validate-metadata %t/not-str.ll 2>&1 | FileCheck %t/not-str.ll +; RUN: not opt -S --dxil-validate-metadata %t/bad-count.ll 2>&1 | FileCheck %t/bad-count.ll + +; Test that loop metadata is validated as with the DXIL validator + +;--- args.ll + +; CHECK: Invalid "llvm.loop" metadata: Requires exactly 1 or 2 operands + +target triple = "dxilv1.0-unknown-shadermodel6.0-library" + +define void @example_loop(i32 %n) { +entry: + br label %loop.header + +loop.header: + %i = phi i32 [ 0, %entry ], [ %i.next, %loop.body ] + %cmp = icmp slt i32 %i, %n + br i1 %cmp, label %loop.body, label %exit + +loop.body: + %i.next = add nsw i32 %i, 1 + br label %loop.header, !llvm.loop !1 + +exit: + ret void +} + +!1 = !{!1, !1, !1} ; too many args + +;--- not-ref.ll + +; CHECK: Invalid "llvm.loop" metadata: First operand must be a self-reference + +target triple = "dxilv1.0-unknown-shadermodel6.0-library" + +define void @example_loop(i32 %n) { +entry: + br label %loop.header + +loop.header: + %i = phi i32 [ 0, %entry ], [ %i.next, %loop.body ] + %cmp = icmp slt i32 %i, %n + br i1 %cmp, label %loop.body, label %exit + +loop.body: + %i.next = add nsw i32 %i, 1 + br label %loop.header, !llvm.loop !1 + +exit: + ret void +} + +!1 = !{i32 0} ; not a self-reference + +;--- not-md.ll + +; CHECK: Invalid "llvm.loop" metadata: Second operand must be a metadata node + +target triple = "dxilv1.0-unknown-shadermodel6.0-library" + +define void @example_loop(i32 %n) { +entry: + br label %loop.header + +loop.header: + %i = phi i32 [ 0, %entry ], [ %i.next, %loop.body ] + %cmp = icmp slt i32 %i, %n + br i1 %cmp, label %loop.body, label %exit + +loop.body: + %i.next = add nsw i32 %i, 1 + br label %loop.header, !llvm.loop !1 + +exit: + ret void +} + +!1 = !{!1, i32 0} ; not a metadata node + +;--- not-str.ll + +; CHECK: Invalid "llvm.loop" metadata: First operand must be a valid "llvm.loop.unroll" hint + +target triple = "dxilv1.0-unknown-shadermodel6.0-library" + +define void @example_loop(i32 %n) { +entry: + br label %loop.header + +loop.header: + %i = phi i32 [ 0, %entry ], [ %i.next, %loop.body ] + %cmp = icmp slt i32 %i, %n + br i1 %cmp, label %loop.body, label %exit + +loop.body: + %i.next = add nsw i32 %i, 1 + br label %loop.header, !llvm.loop !1 + +exit: + ret void +} + +!1 = !{!1, !2} +!2 = !{i32 0} ; not a hint name string + +;--- bad-count.ll + +; CHECK: Second operand of "llvm.loop.unroll.count" must be a constant integer + +target triple = "dxilv1.0-unknown-shadermodel6.0-library" + +define void @example_loop(i32 %n) { +entry: + br label %loop.header + +loop.header: + %i = phi i32 [ 0, %entry ], [ %i.next, %loop.body ] + %cmp = icmp slt i32 %i, %n + br i1 %cmp, label %loop.body, label %exit + +loop.body: + %i.next = add nsw i32 %i, 1 + br label %loop.header, !llvm.loop !1 + +exit: + ret void +} + +!1 = !{!1, !2} +!2 = !{!"llvm.loop.unroll.count", !"not an int"} ; invalid count parameters diff --git a/llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll b/llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll new file mode 100644 index 0000000000000..8dfc0c396aed6 --- /dev/null +++ b/llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll @@ -0,0 +1,31 @@ +; RUN: opt -S --dxil-validate-metadata %s | FileCheck %s + +; Test that we allow a self-referential chain and a constant integer in count + +target triple = "dxilv1.0-unknown-shadermodel6.0-library" + +define void @example_loop(i32 %n) { +entry: + br label %loop.header + +loop.header: + %i = phi i32 [ 0, %entry ], [ %i.next, %loop.body ] + %cmp = icmp slt i32 %i, %n + br i1 %cmp, label %loop.body, label %exit + +loop.body: + %i.next = add nsw i32 %i, 1 + ; CHECK: br label %loop.header, !llvm.loop ![[#LOOP_MD:]] + br label %loop.header, !llvm.loop !0 + +exit: + ret void +} + +; CHECK: ![[#LOOP_MD]] = distinct !{![[#LOOP_MD]], ![[#EXTRA_REF:]]} +; CHECK: ![[#EXTRA_REF]] = distinct !{![[#EXTRA_REF]], ![[#COUNT:]]} +; CHECK: ![[#COUNT]] = !{!"llvm.loop.unroll.count", i6 4} + +!0 = !{!0, !1} +!1 = !{!1, !2} +!2 = !{!"llvm.loop.unroll.count", i6 4} diff --git a/llvm/test/CodeGen/DirectX/metadata-stripping.ll b/llvm/test/CodeGen/DirectX/metadata-stripping.ll index 531ab6c334d24..f33f432777b61 100644 --- a/llvm/test/CodeGen/DirectX/metadata-stripping.ll +++ b/llvm/test/CodeGen/DirectX/metadata-stripping.ll @@ -14,7 +14,7 @@ entry: %cmp.i = icmp ult i32 1, 2 ; Ensure that the !llvm.loop metadata node gets dropped. - ; CHECK: br i1 %cmp.i, label %_Z4mainDv3_j.exit, label %_Z4mainDv3_j.exit{{$}} + ; CHECK: br i1 %cmp.i, label %_Z4mainDv3_j.exit, label %_Z4mainDv3_j.exit, !llvm.loop [[LOOPMD:![0-9]+]] br i1 %cmp.i, label %_Z4mainDv3_j.exit, label %_Z4mainDv3_j.exit, !llvm.loop !0 _Z4mainDv3_j.exit: ; preds = %for.body.i, %entry @@ -24,9 +24,8 @@ _Z4mainDv3_j.exit: ; preds = %for.body.i, %entry ; These next check lines check that only the range metadata remains ; No more metadata should be necessary, the rest (the current 0 and 1) ; should be removed. -; CHECK-NOT: !{!"llvm.loop.mustprogress"} -; CHECK: [[RANGEMD]] = !{i32 1, i32 5} -; CHECK-NOT: !{!"llvm.loop.mustprogress"} +; CHECK-DAG: [[RANGEMD]] = !{i32 1, i32 5} +; CHECK-DAG: !{!"llvm.loop.mustprogress"} !0 = distinct !{!0, !1} !1 = !{!"llvm.loop.mustprogress"} !2 = !{i32 1, i32 5} _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
