From 1ec064e446413fbd7ac3761dc031a997791b9c2a Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Fri, 17 Oct 2025 10:17:32 -0700 Subject: [PATCH 01/20] [DirectX] add a DXILValidateMetadata pass --- llvm/docs/DirectX/DXILArchitecture.rst | 4 +- llvm/lib/Target/DirectX/CMakeLists.txt | 1 + .../Target/DirectX/DXILTranslateMetadata.cpp | 63 ++++++------- .../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, 172 insertions(+), 38 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 1e4797bbd05aa..5c3635a1a6c68 100644 --- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp +++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp @@ -454,45 +454,34 @@ 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(); - AU.addRequired(); - AU.addRequired(); - AU.addRequired(); - AU.addRequired(); - - AU.addPreserved(); - AU.addPreserved(); - AU.addPreserved(); - AU.addPreserved(); - AU.addPreserved(); - } +void DXILTranslateMetadataLegacy::getAnalysisUsage(AnalysisUsage &AU) const { + AU.addRequired(); + AU.addRequired(); + AU.addRequired(); + AU.addRequired(); + AU.addRequired(); + + AU.addPreserved(); + AU.addPreserved(); + AU.addPreserved(); + AU.addPreserved(); + AU.addPreserved(); +} - bool runOnModule(Module &M) override { - DXILResourceMap &DRM = - getAnalysis().getResourceMap(); - DXILResourceTypeMap &DRTM = - getAnalysis().getResourceTypeMap(); - const ModuleShaderFlags &ShaderFlags = - getAnalysis().getShaderFlags(); - dxil::ModuleMetadataInfo MMDI = - getAnalysis().getModuleMetadata(); - - translateGlobalMetadata(M, DRM, DRTM, ShaderFlags, MMDI); - translateInstructionMetadata(M); - return true; - } -}; +bool DXILTranslateMetadataLegacy::runOnModule(Module &M) { + DXILResourceMap &DRM = + getAnalysis().getResourceMap(); + DXILResourceTypeMap &DRTM = + getAnalysis().getResourceTypeMap(); + const ModuleShaderFlags &ShaderFlags = + getAnalysis().getShaderFlags(); + dxil::ModuleMetadataInfo MMDI = + getAnalysis().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 { 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(); + 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 { +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 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 21f7bce29bf766d1d50a173cf4a536a66eba8b62 Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Mon, 20 Oct 2025 09:33:37 -0700 Subject: [PATCH 02/20] 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 5c3635a1a6c68..1cd4281f9fef7 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, @@ -389,9 +369,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) { @@ -400,20 +377,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(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(); AU.addRequired(); AU.setPreservesAll(); } bool runOnModule(Module &M) override { - validateInstructionMetadata(); + dxil::ModuleMetadataInfo MMDI = + getAnalysis().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 4954331a7adf9a87df4da4a46fe9e4715c031cca Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Mon, 20 Oct 2025 09:48:57 -0700 Subject: [PATCH 03/20] [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 1cd4281f9fef7..ce4a5a39e3e44 100644 --- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp +++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp @@ -294,16 +294,17 @@ static void translateBranchMetadata(Module &M, Instruction *BBTerminatorInst) { BBTerminatorInst->setMetadata("hlsl.controlflow.hint", nullptr); } -static std::array getCompatibleInstructionMDs(llvm::Module &M) { +static std::array 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 DXILCompatibleMDs = getCompatibleInstructionMDs(M); + std::array DXILCompatibleMDs = getCompatibleInstructionMDs(M); for (Function &F : M) { for (BasicBlock &BB : F) { From 32f32d8916de8cef7d2497632a10fd58937012ee Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Mon, 20 Oct 2025 09:49:18 -0700 Subject: [PATCH 04/20] [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 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(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(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(HintMD->getOperand(1))) + if (isa(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().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} From ad3cc23b18bf59ef5ed2d5abed5420ba285cae20 Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Thu, 23 Oct 2025 14:54:23 -0700 Subject: [PATCH 05/20] review: alias std::array --- llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp index ce4a5a39e3e44..d4c52b0c2469d 100644 --- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp +++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp @@ -294,7 +294,9 @@ static void translateBranchMetadata(Module &M, Instruction *BBTerminatorInst) { BBTerminatorInst->setMetadata("hlsl.controlflow.hint", nullptr); } -static std::array getCompatibleInstructionMDs(llvm::Module &M) { +using InstructionMDList = std::array; + +static InstructionMDList getCompatibleInstructionMDs(llvm::Module &M) { return { M.getMDKindID("dx.nonuniform"), M.getMDKindID("dx.controlflow.hints"), M.getMDKindID("dx.precise"), llvm::LLVMContext::MD_range, @@ -304,7 +306,7 @@ static std::array getCompatibleInstructionMDs(llvm::Module &M) { static void translateInstructionMetadata(Module &M) { // construct allowlist of valid metadata node kinds - std::array DXILCompatibleMDs = getCompatibleInstructionMDs(M); + InstructionMDList DXILCompatibleMDs = getCompatibleInstructionMDs(M); for (Function &F : M) { for (BasicBlock &BB : F) { From b0c63d5233b79f4e600a7244c1eaf93430f3b51f Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Thu, 23 Oct 2025 15:00:36 -0700 Subject: [PATCH 06/20] review: fix up comments --- llvm/docs/DirectX/DXILArchitecture.rst | 2 +- llvm/lib/Target/DirectX/DXILValidateMetadata.cpp | 6 +++--- llvm/lib/Target/DirectX/DXILValidateMetadata.h | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/llvm/docs/DirectX/DXILArchitecture.rst b/llvm/docs/DirectX/DXILArchitecture.rst index de72697fc4505..6c1cdabc061b9 100644 --- a/llvm/docs/DirectX/DXILArchitecture.rst +++ b/llvm/docs/DirectX/DXILArchitecture.rst @@ -122,7 +122,7 @@ 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 +#. DXILValidateMetadata validates that 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/DXILValidateMetadata.cpp b/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp index 275d3686dd7e9..c17fcd9a9ad22 100644 --- a/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp +++ b/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp @@ -28,8 +28,8 @@ using namespace llvm; namespace { -/// A simple Wrapper DiagnosticInfo that generates Module-level diagnostic -/// for the ValidateMetadata pass +/// A simple wrapper of DiagnosticInfo that generates module-level diagnostic +/// for the DXILValidateMetadata pass class DiagnosticInfoValidateMD : public DiagnosticInfo { private: const Twine &Msg; @@ -70,7 +70,7 @@ static void validateLoopMetadata(Module &M, MDNode *LoopMD) { "llvm.loop.unroll.disable", "llvm.loop.unroll.full"}; - // llvm.loop metadata must have it's first operand be a self-reference, so we + // llvm.loop metadata must have its 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 diff --git a/llvm/lib/Target/DirectX/DXILValidateMetadata.h b/llvm/lib/Target/DirectX/DXILValidateMetadata.h index 3188afafc8e22..5d13620fbd7e9 100644 --- a/llvm/lib/Target/DirectX/DXILValidateMetadata.h +++ b/llvm/lib/Target/DirectX/DXILValidateMetadata.h @@ -1,4 +1,4 @@ -//===- DXILValidateMetadata.h - Pass to emit DXIL metadata -----*- C++ -*-===// +//===- DXILValidateMetadata.h - Pass to validate 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. @@ -13,7 +13,7 @@ namespace llvm { -/// A pass that transforms DXIL Intrinsics that don't have DXIL opCodes +/// A pass that validates metadata to be DXIL compatible class DXILValidateMetadata : public PassInfoMixin { public: PreservedAnalyses run(Module &M, ModuleAnalysisManager &); From 1580faba55e0f2bea79b73104ac8daf6fc09a5c3 Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Thu, 23 Oct 2025 15:07:11 -0700 Subject: [PATCH 07/20] review: only validate loop metadata on a branch inst --- llvm/lib/Target/DirectX/DXILValidateMetadata.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp b/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp index c17fcd9a9ad22..c59983ce755df 100644 --- a/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp +++ b/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp @@ -17,6 +17,7 @@ #include "llvm/IR/Constants.h" #include "llvm/IR/DiagnosticInfo.h" #include "llvm/IR/DiagnosticPrinter.h" +#include "llvm/IR/Instructions.h" #include "llvm/IR/LLVMContext.h" #include "llvm/IR/Metadata.h" #include "llvm/IR/Module.h" @@ -142,8 +143,9 @@ static void validateInstructionMetadata(Module &M) { for (Function &F : M) for (BasicBlock &BB : F) for (Instruction &I : BB) { - if (MDNode *LoopMD = I.getMetadata(MDLoopKind)) - validateLoopMetadata(M, LoopMD); + if (isa(I)) + if (MDNode *LoopMD = I.getMetadata(MDLoopKind)) + validateLoopMetadata(M, LoopMD); } } From 7235fc06f3c837a016b89213f8a1864ff3d4f2f2 Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Thu, 23 Oct 2025 15:11:05 -0700 Subject: [PATCH 08/20] review: fix up testcase --- llvm/test/CodeGen/DirectX/metadata-stripping.ll | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/llvm/test/CodeGen/DirectX/metadata-stripping.ll b/llvm/test/CodeGen/DirectX/metadata-stripping.ll index f33f432777b61..a79575c8c7637 100644 --- a/llvm/test/CodeGen/DirectX/metadata-stripping.ll +++ b/llvm/test/CodeGen/DirectX/metadata-stripping.ll @@ -25,7 +25,8 @@ _Z4mainDv3_j.exit: ; preds = %for.body.i, %entry ; No more metadata should be necessary, the rest (the current 0 and 1) ; should be removed. ; CHECK-DAG: [[RANGEMD]] = !{i32 1, i32 5} -; CHECK-DAG: !{!"llvm.loop.mustprogress"} +; CHECK-DAG: [[LOOPMD]] = distinct !{[[LOOPMD]], [[PROGRESS:![0-9]+]]} +; CHECK-DAG: {!"llvm.loop.mustprogress"} !0 = distinct !{!0, !1} !1 = !{!"llvm.loop.mustprogress"} !2 = !{i32 1, i32 5} From cefb24d4a907d78e07ce3d8ca7a5dd58918eb710 Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Thu, 23 Oct 2025 15:21:27 -0700 Subject: [PATCH 09/20] review: add additional checks for full/disable --- .../Target/DirectX/DXILValidateMetadata.cpp | 12 ++-- .../CodeGen/DirectX/Metadata/loop-md-errs.ll | 55 +++++++++++++++ .../CodeGen/DirectX/Metadata/loop-md-valid.ll | 67 ++++++++++++++++++- 3 files changed, 129 insertions(+), 5 deletions(-) diff --git a/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp b/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp index c59983ce755df..1db8e9bd9df8d 100644 --- a/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp +++ b/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp @@ -129,10 +129,14 @@ static void validateLoopMetadata(Module &M, MDNode *LoopMD) { 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"); + if (HintStr->getString() == "llvm.loop.unroll.count") { + if (!ValidCountNode(LoopMD)) { + reportLoopError(M, "Second operand of \"llvm.loop.unroll.count\" " + "must be a constant integer"); + return; + } + } else if (LoopMD->getNumOperands() != 1) { + reportLoopError(M, "Can't have a second operand"); return; } } diff --git a/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll b/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll index 1bf16a6559d82..096302d1b52fe 100644 --- a/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll +++ b/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll @@ -4,6 +4,8 @@ ; 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 +; RUN: not opt -S --dxil-validate-metadata %t/invalid-disable.ll 2>&1 | FileCheck %t/invalid-disable.ll +; RUN: not opt -S --dxil-validate-metadata %t/invalid-full.ll 2>&1 | FileCheck %t/invalid-full.ll ; Test that loop metadata is validated as with the DXIL validator @@ -133,3 +135,56 @@ exit: !1 = !{!1, !2} !2 = !{!"llvm.loop.unroll.count", !"not an int"} ; invalid count parameters + +;--- invalid-disable.ll + +; CHECK: Invalid "llvm.loop" metadata: Can't have a second operand + +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.disable", i32 0} ; invalid second operand + + +;--- invalid-full.ll + +; CHECK: Invalid "llvm.loop" metadata: Can't have a second operand + +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.full", i32 0} ; invalid second operand diff --git a/llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll b/llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll index 8dfc0c396aed6..b397bf67d262f 100644 --- a/llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll +++ b/llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll @@ -1,4 +1,9 @@ -; RUN: opt -S --dxil-validate-metadata %s | FileCheck %s +; RUN: split-file %s %t +; RUN: opt -S --dxil-validate-metadata %t/count.ll | FileCheck %t/count.ll +; RUN: opt -S --dxil-validate-metadata %t/disable.ll | FileCheck %t/disable.ll +; RUN: opt -S --dxil-validate-metadata %t/full.ll | FileCheck %t/full.ll + +;--- count.ll ; Test that we allow a self-referential chain and a constant integer in count @@ -29,3 +34,63 @@ exit: !0 = !{!0, !1} !1 = !{!1, !2} !2 = !{!"llvm.loop.unroll.count", i6 4} + +;--- disable.ll + +; Test that we allow a disable 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 + ; 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]], ![[#DISABLE:]]} +; CHECK: ![[#DISABLE]] = !{!"llvm.loop.unroll.disable"} + +!0 = !{!0, !1} +!1 = !{!"llvm.loop.unroll.disable"} + +;--- full.ll + +; Test that we allow a full 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 + ; 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]], ![[#FULL:]]} +; CHECK: ![[#FULL]] = !{!"llvm.loop.unroll.full"} + +!0 = !{!0, !1} +!1 = !{!"llvm.loop.unroll.full"} From 33e9af27b00993c454db0389682af1060e6a2043 Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Fri, 24 Oct 2025 11:08:21 -0700 Subject: [PATCH 10/20] self-review: merge DXILValidate into DXILTranslate 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 --- llvm/lib/Target/DirectX/CMakeLists.txt | 1 - .../Target/DirectX/DXILTranslateMetadata.cpp | 140 ++++++++++- .../Target/DirectX/DXILValidateMetadata.cpp | 222 ------------------ .../lib/Target/DirectX/DXILValidateMetadata.h | 24 -- .../Target/DirectX/DirectXPassRegistry.def | 1 - .../Target/DirectX/DirectXTargetMachine.cpp | 3 - .../CodeGen/DirectX/Metadata/loop-md-errs.ll | 14 +- .../CodeGen/DirectX/Metadata/loop-md-valid.ll | 6 +- .../Metadata/multiple-entries-cs-error.ll | 2 +- .../DirectX/Metadata/target-profile-error.ll | 4 +- llvm/test/CodeGen/DirectX/llc-pipeline.ll | 1 - .../CodeGen/DirectX/metadata-stripping.ll | 3 +- 12 files changed, 145 insertions(+), 276 deletions(-) delete mode 100644 llvm/lib/Target/DirectX/DXILValidateMetadata.cpp delete mode 100644 llvm/lib/Target/DirectX/DXILValidateMetadata.h diff --git a/llvm/lib/Target/DirectX/CMakeLists.txt b/llvm/lib/Target/DirectX/CMakeLists.txt index f9900370660dd..6c079517e22d6 100644 --- a/llvm/lib/Target/DirectX/CMakeLists.txt +++ b/llvm/lib/Target/DirectX/CMakeLists.txt @@ -35,7 +35,6 @@ 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 d4c52b0c2469d..47511671a956d 100644 --- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp +++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp @@ -37,6 +37,39 @@ using namespace llvm::dxil; namespace { +/// A simple wrapper of DiagnosticInfo that generates module-level diagnostic +/// for the DXILValidateMetadata 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); +} + enum class EntryPropsTag { ShaderFlags = 0, GSState, @@ -294,6 +327,83 @@ static void translateBranchMetadata(Module &M, Instruction *BBTerminatorInst) { BBTerminatorInst->setMetadata("hlsl.controlflow.hint", nullptr); } +static void translateLoopMetadata(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 ValidHintNames = {"llvm.loop.unroll.count", + "llvm.loop.unroll.disable", + "llvm.loop.unroll.full"}; + + // llvm.loop metadata must have its 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(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 translateLoopMetadata(M, LoopMD); + + // Otherwise, we are at our base hint metadata node + auto *HintStr = dyn_cast(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(HintMD->getOperand(1))) + if (isa(CountMD->getValue())) + return true; + return false; + }; + + if (HintStr->getString() == "llvm.loop.unroll.count") { + if (!ValidCountNode(LoopMD)) { + reportLoopError(M, "Second operand of \"llvm.loop.unroll.count\" " + "must be a constant integer"); + return; + } + } else if (LoopMD->getNumOperands() != 1) { + reportLoopError(M, "Can't have a second operand"); + return; + } +} + using InstructionMDList = std::array; static InstructionMDList getCompatibleInstructionMDs(llvm::Module &M) { @@ -307,6 +417,7 @@ static InstructionMDList getCompatibleInstructionMDs(llvm::Module &M) { static void translateInstructionMetadata(Module &M) { // construct allowlist of valid metadata node kinds InstructionMDList DXILCompatibleMDs = getCompatibleInstructionMDs(M); + unsigned char MDLoopKind = M.getContext().getMDKindID("llvm.loop"); for (Function &F : M) { for (BasicBlock &BB : F) { @@ -316,6 +427,10 @@ static void translateInstructionMetadata(Module &M) { translateBranchMetadata(M, I); for (auto &I : make_early_inc_range(BB)) { + if (isa(I)) { + if (MDNode *LoopMD = I.getMetadata(MDLoopKind)) + translateLoopMetadata(M, LoopMD); + } I.dropUnknownNonDebugMetadata(DXILCompatibleMDs); } } @@ -372,17 +487,24 @@ static void translateGlobalMetadata(Module &M, DXILResourceMap &DRM, uint64_t CombinedMask = ShaderFlags.getCombinedFlags(); EntryFnMDNodes.emplace_back( emitTopLevelLibraryNode(M, ResourceMD, CombinedMask)); - } + } else if (1 < MMDI.EntryPropertyVec.size()) + reportError(M, "Non-library shader: One and only one entry expected"); for (const EntryProperties &EntryProp : MMDI.EntryPropertyVec) { - const ComputedShaderFlags &EntrySFMask = - ShaderFlags.getFunctionFlags(EntryProp.Entry); - - // If ShaderProfile is Library, mask is already consolidated in the - // top-level library node. Hence it is not emitted. - uint64_t EntryShaderFlags = - MMDI.ShaderProfile == Triple::EnvironmentType::Library ? 0 - : EntrySFMask; + uint64_t EntryShaderFlags = 0; + if (MMDI.ShaderProfile != Triple::EnvironmentType::Library) { + EntryShaderFlags = ShaderFlags.getFunctionFlags(EntryProp.Entry); + if (EntryProp.ShaderStage != MMDI.ShaderProfile) + reportError( + M, + "Shader stage '" + + Twine(Twine(getShortShaderStage(EntryProp.ShaderStage)) + + "' for entry '" + Twine(EntryProp.Entry->getName()) + + "' different from specified target profile '" + + Twine(Triple::getEnvironmentTypeName(MMDI.ShaderProfile) + + "'"))); + } + 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 deleted file mode 100644 index 1db8e9bd9df8d..0000000000000 --- a/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp +++ /dev/null @@ -1,222 +0,0 @@ -//===- 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/Instructions.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 of DiagnosticInfo that generates module-level diagnostic -/// for the DXILValidateMetadata 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 ValidHintNames = {"llvm.loop.unroll.count", - "llvm.loop.unroll.disable", - "llvm.loop.unroll.full"}; - - // llvm.loop metadata must have its 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(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(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(HintMD->getOperand(1))) - if (isa(CountMD->getValue())) - return true; - return false; - }; - - if (HintStr->getString() == "llvm.loop.unroll.count") { - if (!ValidCountNode(LoopMD)) { - reportLoopError(M, "Second operand of \"llvm.loop.unroll.count\" " - "must be a constant integer"); - return; - } - } else if (LoopMD->getNumOperands() != 1) { - reportLoopError(M, "Can't have a second operand"); - 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 (isa(I)) - 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(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(); - AU.addRequired(); - AU.setPreservesAll(); - } - - bool runOnModule(Module &M) override { - dxil::ModuleMetadataInfo MMDI = - getAnalysis().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 deleted file mode 100644 index 5d13620fbd7e9..0000000000000 --- a/llvm/lib/Target/DirectX/DXILValidateMetadata.h +++ /dev/null @@ -1,24 +0,0 @@ -//===- DXILValidateMetadata.h - Pass to validate 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 validates metadata to be DXIL compatible -class DXILValidateMetadata : public PassInfoMixin { -public: - PreservedAnalyses run(Module &M, ModuleAnalysisManager &); -}; - -} // namespace llvm - -#endif // LLVM_TARGET_DIRECTX_DXILVALIDATEMETADATA_H diff --git a/llvm/lib/Target/DirectX/DirectXPassRegistry.def b/llvm/lib/Target/DirectX/DirectXPassRegistry.def index a69f9764b932b..b4b48a166800e 100644 --- a/llvm/lib/Target/DirectX/DirectXPassRegistry.def +++ b/llvm/lib/Target/DirectX/DirectXPassRegistry.def @@ -31,7 +31,6 @@ 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 after NPM switch diff --git a/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp b/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp index 09cbf031ddefe..bcf84403b2c0d 100644 --- a/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp +++ b/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp @@ -27,7 +27,6 @@ #include "DXILRootSignature.h" #include "DXILShaderFlags.h" #include "DXILTranslateMetadata.h" -#include "DXILValidateMetadata.h" #include "DXILWriter/DXILWriterPass.h" #include "DirectX.h" #include "DirectXSubtarget.h" @@ -71,7 +70,6 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeDirectXTarget() { initializeDXILResourceAccessLegacyPass(*PR); initializeDXILResourceImplicitBindingLegacyPass(*PR); initializeDXILTranslateMetadataLegacyPass(*PR); - initializeDXILValidateMetadataLegacyPass(*PR); initializeDXILPostOptimizationValidationLegacyPass(*PR); initializeShaderFlagsAnalysisWrapperPass(*PR); initializeRootSignatureAnalysisWrapperPass(*PR); @@ -124,7 +122,6 @@ 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/Metadata/loop-md-errs.ll b/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll index 096302d1b52fe..487dc53b76d96 100644 --- a/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll +++ b/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll @@ -1,11 +1,11 @@ ; 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 -; RUN: not opt -S --dxil-validate-metadata %t/invalid-disable.ll 2>&1 | FileCheck %t/invalid-disable.ll -; RUN: not opt -S --dxil-validate-metadata %t/invalid-full.ll 2>&1 | FileCheck %t/invalid-full.ll +; RUN: not opt -S --dxil-translate-metadata %t/args.ll 2>&1 | FileCheck %t/args.ll +; RUN: not opt -S --dxil-translate-metadata %t/not-ref.ll 2>&1 | FileCheck %t/not-ref.ll +; RUN: not opt -S --dxil-translate-metadata %t/not-md.ll 2>&1 | FileCheck %t/not-md.ll +; RUN: not opt -S --dxil-translate-metadata %t/not-str.ll 2>&1 | FileCheck %t/not-str.ll +; RUN: not opt -S --dxil-translate-metadata %t/bad-count.ll 2>&1 | FileCheck %t/bad-count.ll +; RUN: not opt -S --dxil-translate-metadata %t/invalid-disable.ll 2>&1 | FileCheck %t/invalid-disable.ll +; RUN: not opt -S --dxil-translate-metadata %t/invalid-full.ll 2>&1 | FileCheck %t/invalid-full.ll ; Test that loop metadata is validated as with the DXIL validator diff --git a/llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll b/llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll index b397bf67d262f..bb55f12b3316e 100644 --- a/llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll +++ b/llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll @@ -1,7 +1,7 @@ ; RUN: split-file %s %t -; RUN: opt -S --dxil-validate-metadata %t/count.ll | FileCheck %t/count.ll -; RUN: opt -S --dxil-validate-metadata %t/disable.ll | FileCheck %t/disable.ll -; RUN: opt -S --dxil-validate-metadata %t/full.ll | FileCheck %t/full.ll +; RUN: opt -S --dxil-translate-metadata %t/count.ll | FileCheck %t/count.ll +; RUN: opt -S --dxil-translate-metadata %t/disable.ll | FileCheck %t/disable.ll +; RUN: opt -S --dxil-translate-metadata %t/full.ll | FileCheck %t/full.ll ;--- count.ll 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 cba10336f7817..5740ee11401f2 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 -dxil-validate-metadata %s 2>&1 | FileCheck %s +; RUN: not opt -S -dxil-translate-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 13e28cfebf9af..671406cb5d364 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-validate-metadata %s 2>&1 | FileCheck %s +; RUN: not opt -S -dxil-translate-metadata %s 2>&1 | FileCheck %s target triple = "dxil-pc-shadermodel6.6-pixel" -; CHECK: Shader stage 'compute' for entry 'entry' different from specified target profile 'pixel' +; CHECK: Shader stage 'cs' for entry 'entry' different from specified target profile 'pixel' define void @entry() #0 { entry: diff --git a/llvm/test/CodeGen/DirectX/llc-pipeline.ll b/llvm/test/CodeGen/DirectX/llc-pipeline.ll index 13b502475b283..d265826cd2469 100644 --- a/llvm/test/CodeGen/DirectX/llc-pipeline.ll +++ b/llvm/test/CodeGen/DirectX/llc-pipeline.ll @@ -42,7 +42,6 @@ ; 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 diff --git a/llvm/test/CodeGen/DirectX/metadata-stripping.ll b/llvm/test/CodeGen/DirectX/metadata-stripping.ll index a79575c8c7637..95a1783eb43db 100644 --- a/llvm/test/CodeGen/DirectX/metadata-stripping.ll +++ b/llvm/test/CodeGen/DirectX/metadata-stripping.ll @@ -25,8 +25,7 @@ _Z4mainDv3_j.exit: ; preds = %for.body.i, %entry ; No more metadata should be necessary, the rest (the current 0 and 1) ; should be removed. ; CHECK-DAG: [[RANGEMD]] = !{i32 1, i32 5} -; CHECK-DAG: [[LOOPMD]] = distinct !{[[LOOPMD]], [[PROGRESS:![0-9]+]]} -; CHECK-DAG: {!"llvm.loop.mustprogress"} +; CHECK-NOT: {!"llvm.loop.mustprogress"} !0 = distinct !{!0, !1} !1 = !{!"llvm.loop.mustprogress"} !2 = !{i32 1, i32 5} From e72428e61dfc982d65151d1023b915aebd6e4e4f Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Fri, 24 Oct 2025 14:04:40 -0700 Subject: [PATCH 11/20] self-review: strip all unrecognized metadata instead of errors --- .../Target/DirectX/DXILTranslateMetadata.cpp | 134 ++++++++++-------- .../CodeGen/DirectX/Metadata/loop-md-errs.ll | 89 +----------- .../DirectX/Metadata/loop-md-stripped.ll | 58 ++++++++ .../CodeGen/DirectX/Metadata/loop-md-valid.ll | 5 +- .../CodeGen/DirectX/metadata-stripping.ll | 3 +- 5 files changed, 139 insertions(+), 150 deletions(-) create mode 100644 llvm/test/CodeGen/DirectX/Metadata/loop-md-stripped.ll diff --git a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp index 47511671a956d..a787c59fcb121 100644 --- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp +++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp @@ -327,81 +327,90 @@ static void translateBranchMetadata(Module &M, Instruction *BBTerminatorInst) { BBTerminatorInst->setMetadata("hlsl.controlflow.hint", nullptr); } -static void translateLoopMetadata(Module &M, MDNode *LoopMD) { +// Determines if the metadata node will be compatible with DXIL's loop metadata +// representation. +// +// Reports an error for compatible metadata that is ill-formed. +static bool isLoopMDCompatible(Module &M, Metadata *MD) { // DXIL only accepts the following loop hints: - // llvm.loop.unroll.disable, llvm.loop.unroll.full, llvm.loop.unroll.count std::array ValidHintNames = {"llvm.loop.unroll.count", "llvm.loop.unroll.disable", "llvm.loop.unroll.full"}; - // llvm.loop metadata must have its 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. + MDNode *HintMD = dyn_cast(MD); + if (!HintMD || HintMD->getNumOperands() == 0) + return false; - if (LoopMD->getNumOperands() != 1 && LoopMD->getNumOperands() != 2) { - reportLoopError(M, "Requires exactly 1 or 2 operands"); - return; - } + auto *HintStr = dyn_cast(HintMD->getOperand(0)); + if (!HintStr) + return false; - if (LoopMD != LoopMD->getOperand(0)) { - reportLoopError(M, "First operand must be a self-reference"); - return; - } + if (!llvm::is_contained(ValidHintNames, HintStr->getString())) + return false; - // A node only containing a self-reference is a valid use to denote a loop - if (LoopMD->getNumOperands() == 1) - return; + auto ValidCountNode = [](MDNode *CountMD) -> bool { + if (CountMD->getNumOperands() == 2) + if (auto *Count = dyn_cast(CountMD->getOperand(1))) + if (isa(Count->getValue())) + return true; + return false; + }; - LoopMD = dyn_cast(LoopMD->getOperand(1)); - if (!LoopMD) { - reportLoopError(M, "Second operand must be a metadata node"); - return; - } + if (HintStr->getString() == "llvm.loop.unroll.count") { + if (!ValidCountNode(HintMD)) + return reportLoopError(M, "Second operand of \"llvm.loop.unroll.count\" " + "must be a constant integer"); + } else if (HintMD->getNumOperands() != 1) + return reportLoopError( + M, "\"llvm.loop.unroll.disable\" and \"llvm.loop.unroll.disable\" " + "must be provided as a single operand"); - if (LoopMD->getNumOperands() != 1 && LoopMD->getNumOperands() != 2) { - reportLoopError(M, "Requires exactly 1 or 2 operands"); - return; - } + return true; +} + +static void translateLoopMetadata(Module &M, Instruction *I, MDNode *BaseMD) { + // A distinct node has the self-referential form: !0 = !{ !0, ... } + auto IsDistinctNode = [](MDNode *Node) -> bool { + return Node && Node->getNumOperands() != 0 && Node == Node->getOperand(0); + }; + + // Strip empty metadata or a non-distinct node + if (BaseMD->getNumOperands() == 0 || !IsDistinctNode(BaseMD)) + return I->setMetadata("llvm.loop", nullptr); - // It is valid to have a chain of self-referential loop metadata nodes so if - // we have another self-reference, recurse. + // It is valid to have a chain of self-refential loop metadata nodes, as + // below. We will collapse these into just one when we reconstruct the + // metadata. // // Eg: // !0 = !{!0, !1} // !1 = !{!1, !2} - // !2 = !{"llvm.loop.unroll.disable"} - if (LoopMD == LoopMD->getOperand(0)) - return translateLoopMetadata(M, LoopMD); - - // Otherwise, we are at our base hint metadata node - auto *HintStr = dyn_cast(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(HintMD->getOperand(1))) - if (isa(CountMD->getValue())) - return true; - return false; - }; - - if (HintStr->getString() == "llvm.loop.unroll.count") { - if (!ValidCountNode(LoopMD)) { - reportLoopError(M, "Second operand of \"llvm.loop.unroll.count\" " - "must be a constant integer"); - return; - } - } else if (LoopMD->getNumOperands() != 1) { - reportLoopError(M, "Can't have a second operand"); - return; - } + // !2 = !{!"llvm.loop.unroll.disable"} + // + // So, traverse down a potential self-referential chain + while (1 < BaseMD->getNumOperands() && + IsDistinctNode(dyn_cast(BaseMD->getOperand(1)))) + BaseMD = dyn_cast(BaseMD->getOperand(1)); + + // To reconstruct a distinct node we create a temporary node that we will + // then update to create a self-reference. + llvm::TempMDTuple TempNode = llvm::MDNode::getTemporary(M.getContext(), {}); + SmallVector CompatibleOperands = {TempNode.get()}; + + // Iterate and reconstruct the metadata nodes that contains any hints, + // stripping any unrecognized metadata. + ArrayRef Operands = BaseMD->operands(); + for (auto &Op : Operands.drop_front()) + if (isLoopMDCompatible(M, Op.get())) + CompatibleOperands.push_back(Op.get()); + + if (2 < CompatibleOperands.size()) + reportLoopError(M, "Provided conflicting hints"); + + MDNode *CompatibleLoopMD = MDNode::get(M.getContext(), CompatibleOperands); + TempNode->replaceAllUsesWith(CompatibleLoopMD); + + I->setMetadata("llvm.loop", CompatibleLoopMD); } using InstructionMDList = std::array; @@ -427,10 +436,9 @@ static void translateInstructionMetadata(Module &M) { translateBranchMetadata(M, I); for (auto &I : make_early_inc_range(BB)) { - if (isa(I)) { + if (isa(I)) if (MDNode *LoopMD = I.getMetadata(MDLoopKind)) - translateLoopMetadata(M, LoopMD); - } + translateLoopMetadata(M, &I, LoopMD); I.dropUnknownNonDebugMetadata(DXILCompatibleMDs); } } diff --git a/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll b/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll index 487dc53b76d96..f67077f381208 100644 --- a/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll +++ b/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll @@ -1,8 +1,5 @@ ; RUN: split-file %s %t ; RUN: not opt -S --dxil-translate-metadata %t/args.ll 2>&1 | FileCheck %t/args.ll -; RUN: not opt -S --dxil-translate-metadata %t/not-ref.ll 2>&1 | FileCheck %t/not-ref.ll -; RUN: not opt -S --dxil-translate-metadata %t/not-md.ll 2>&1 | FileCheck %t/not-md.ll -; RUN: not opt -S --dxil-translate-metadata %t/not-str.ll 2>&1 | FileCheck %t/not-str.ll ; RUN: not opt -S --dxil-translate-metadata %t/bad-count.ll 2>&1 | FileCheck %t/bad-count.ll ; RUN: not opt -S --dxil-translate-metadata %t/invalid-disable.ll 2>&1 | FileCheck %t/invalid-disable.ll ; RUN: not opt -S --dxil-translate-metadata %t/invalid-full.ll 2>&1 | FileCheck %t/invalid-full.ll @@ -11,7 +8,7 @@ ;--- args.ll -; CHECK: Invalid "llvm.loop" metadata: Requires exactly 1 or 2 operands +; CHECK: Invalid "llvm.loop" metadata: Provided conflicting hints target triple = "dxilv1.0-unknown-shadermodel6.0-library" @@ -32,83 +29,9 @@ 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 +!1 = !{!1, !2, !3} ; conflicting args +!2 = !{!"llvm.loop.unroll.full"} +!3 = !{!"llvm.loop.unroll.disable"} ;--- bad-count.ll @@ -138,7 +61,7 @@ exit: ;--- invalid-disable.ll -; CHECK: Invalid "llvm.loop" metadata: Can't have a second operand +; CHECK: Invalid "llvm.loop" metadata: "llvm.loop.unroll.disable" and "llvm.loop.unroll.disable" must be provided as a single operand target triple = "dxilv1.0-unknown-shadermodel6.0-library" @@ -165,7 +88,7 @@ exit: ;--- invalid-full.ll -; CHECK: Invalid "llvm.loop" metadata: Can't have a second operand +; CHECK: Invalid "llvm.loop" metadata: "llvm.loop.unroll.disable" and "llvm.loop.unroll.disable" must be provided as a single operand target triple = "dxilv1.0-unknown-shadermodel6.0-library" diff --git a/llvm/test/CodeGen/DirectX/Metadata/loop-md-stripped.ll b/llvm/test/CodeGen/DirectX/Metadata/loop-md-stripped.ll new file mode 100644 index 0000000000000..e159f2427b192 --- /dev/null +++ b/llvm/test/CodeGen/DirectX/Metadata/loop-md-stripped.ll @@ -0,0 +1,58 @@ +; RUN: split-file %s %t +; RUN: opt -S --dxil-translate-metadata %t/not-distinct.ll 2>&1 | FileCheck %t/not-distinct.ll +; RUN: opt -S --dxil-translate-metadata %t/not-md.ll 2>&1 | FileCheck %t/not-md.ll + +; Test that DXIL incompatible loop metadata is stripped + +;--- not-distinct.ll + +; Ensure it is stripped because it is not provided a distinct loop parent +; CHECK-NOT: {!"llvm.loop.unroll.disable"} + +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 = !{!"llvm.loop.unroll.disable"} ; first node must be a distinct self-reference + + +;--- not-md.ll + +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 !1 + +exit: + ret void +} + +; CHECK: ![[#LOOP_MD:]] = distinct !{![[#LOOP_MD]]} + +!1 = !{!1, i32 0} ; not a metadata node diff --git a/llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll b/llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll index bb55f12b3316e..a189c0e3f8aaa 100644 --- a/llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll +++ b/llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll @@ -5,7 +5,7 @@ ;--- count.ll -; Test that we allow a self-referential chain and a constant integer in count +; Test that we collapse a self-referential chain and allow a unroll.count hint target triple = "dxilv1.0-unknown-shadermodel6.0-library" @@ -27,8 +27,7 @@ exit: ret void } -; CHECK: ![[#LOOP_MD]] = distinct !{![[#LOOP_MD]], ![[#EXTRA_REF:]]} -; CHECK: ![[#EXTRA_REF]] = distinct !{![[#EXTRA_REF]], ![[#COUNT:]]} +; CHECK: ![[#LOOP_MD]] = distinct !{![[#LOOP_MD]], ![[#COUNT:]]} ; CHECK: ![[#COUNT]] = !{!"llvm.loop.unroll.count", i6 4} !0 = !{!0, !1} diff --git a/llvm/test/CodeGen/DirectX/metadata-stripping.ll b/llvm/test/CodeGen/DirectX/metadata-stripping.ll index 95a1783eb43db..b717740f7255b 100644 --- a/llvm/test/CodeGen/DirectX/metadata-stripping.ll +++ b/llvm/test/CodeGen/DirectX/metadata-stripping.ll @@ -25,7 +25,8 @@ _Z4mainDv3_j.exit: ; preds = %for.body.i, %entry ; No more metadata should be necessary, the rest (the current 0 and 1) ; should be removed. ; CHECK-DAG: [[RANGEMD]] = !{i32 1, i32 5} +; CHECK-DAG: [[LOOPMD]] = distinct !{[[LOOPMD]]} ; CHECK-NOT: {!"llvm.loop.mustprogress"} -!0 = distinct !{!0, !1} +!0 = distinct !{!0, !1, !2} !1 = !{!"llvm.loop.mustprogress"} !2 = !{i32 1, i32 5} From e0a36ceaa076996127c2198b987e93c6097d6df0 Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Fri, 24 Oct 2025 16:06:55 -0700 Subject: [PATCH 12/20] self-review: missed clean-up --- llvm/docs/DirectX/DXILArchitecture.rst | 4 +--- llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp | 13 ++++++------- llvm/lib/Target/DirectX/DirectX.h | 6 ------ 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/llvm/docs/DirectX/DXILArchitecture.rst b/llvm/docs/DirectX/DXILArchitecture.rst index 6c1cdabc061b9..bce7fdaa386ed 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 -> DXILValidateMetadata + DXILOpLowering -> DXILPrepare -> DXILTranslateMetadata Each of these passes has a defined responsibility: @@ -122,8 +122,6 @@ 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 that 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/DXILTranslateMetadata.cpp b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp index a787c59fcb121..0650156f3eea0 100644 --- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp +++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp @@ -504,13 +504,12 @@ static void translateGlobalMetadata(Module &M, DXILResourceMap &DRM, EntryShaderFlags = ShaderFlags.getFunctionFlags(EntryProp.Entry); if (EntryProp.ShaderStage != MMDI.ShaderProfile) reportError( - M, - "Shader stage '" + - Twine(Twine(getShortShaderStage(EntryProp.ShaderStage)) + - "' for entry '" + Twine(EntryProp.Entry->getName()) + - "' different from specified target profile '" + - Twine(Triple::getEnvironmentTypeName(MMDI.ShaderProfile) + - "'"))); + M, "Shader stage '" + + Twine(getShortShaderStage(EntryProp.ShaderStage)) + + "' for entry '" + Twine(EntryProp.Entry->getName()) + + "' different from specified target profile '" + + Twine(Triple::getEnvironmentTypeName(MMDI.ShaderProfile) + + "'")); } EntryFnMDNodes.emplace_back(emitEntryMD(EntryProp, Signatures, ResourceMD, diff --git a/llvm/lib/Target/DirectX/DirectX.h b/llvm/lib/Target/DirectX/DirectX.h index 90546f133c9d2..e31c2ffa4f761 100644 --- a/llvm/lib/Target/DirectX/DirectX.h +++ b/llvm/lib/Target/DirectX/DirectX.h @@ -90,12 +90,6 @@ 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); From 5398809b59bd5c9ccf88f2662674bdfa65252778 Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Mon, 27 Oct 2025 09:16:50 -0700 Subject: [PATCH 13/20] review: comment clean up --- llvm/test/CodeGen/DirectX/Metadata/loop-md-stripped.ll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/test/CodeGen/DirectX/Metadata/loop-md-stripped.ll b/llvm/test/CodeGen/DirectX/Metadata/loop-md-stripped.ll index e159f2427b192..09d8aec2ff0e5 100644 --- a/llvm/test/CodeGen/DirectX/Metadata/loop-md-stripped.ll +++ b/llvm/test/CodeGen/DirectX/Metadata/loop-md-stripped.ll @@ -55,4 +55,4 @@ exit: ; CHECK: ![[#LOOP_MD:]] = distinct !{![[#LOOP_MD]]} -!1 = !{!1, i32 0} ; not a metadata node +!1 = !{!1, i32 0} ; second operand is not a metadata node From 9a6a46879f96c92808478ec866a452e35c0ce01d Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Mon, 27 Oct 2025 09:17:09 -0700 Subject: [PATCH 14/20] review: explicitly return validity --- llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp index 0650156f3eea0..e0f46b212a4b2 100644 --- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp +++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp @@ -357,13 +357,17 @@ static bool isLoopMDCompatible(Module &M, Metadata *MD) { }; if (HintStr->getString() == "llvm.loop.unroll.count") { - if (!ValidCountNode(HintMD)) - return reportLoopError(M, "Second operand of \"llvm.loop.unroll.count\" " - "must be a constant integer"); - } else if (HintMD->getNumOperands() != 1) - return reportLoopError( + if (!ValidCountNode(HintMD)) { + reportLoopError(M, "Second operand of \"llvm.loop.unroll.count\" " + "must be a constant integer"); + return false; + } + } else if (HintMD->getNumOperands() != 1) { + reportLoopError( M, "\"llvm.loop.unroll.disable\" and \"llvm.loop.unroll.disable\" " "must be provided as a single operand"); + return false; + } return true; } From ed3d21e39246185a29cee5edbd720418304e3871 Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Mon, 27 Oct 2025 09:19:41 -0700 Subject: [PATCH 15/20] self-review: correct naming --- llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp index e0f46b212a4b2..aadf1dde055c4 100644 --- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp +++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp @@ -435,7 +435,7 @@ static void translateInstructionMetadata(Module &M) { for (Function &F : M) { for (BasicBlock &BB : F) { // This needs to be done first so that "hlsl.controlflow.hints" isn't - // removed in the whitelist below + // removed in the allow-list below if (auto *I = BB.getTerminator()) translateBranchMetadata(M, I); From 7cfd17fc78add00a4dd973ba0c822471beb849b4 Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Tue, 28 Oct 2025 10:32:00 -0700 Subject: [PATCH 16/20] review: improve error messages + handling --- llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp | 14 ++++++-------- llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll | 4 ++-- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp index aadf1dde055c4..66b877951bc2f 100644 --- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp +++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp @@ -58,16 +58,14 @@ class DiagnosticInfoValidateMD : public DiagnosticInfo { } }; -static bool reportError(Module &M, Twine Message, +static void 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, +static void reportLoopError(Module &M, Twine Message, DiagnosticSeverity Severity = DS_Error) { - return reportError(M, Twine("Invalid \"llvm.loop\" metadata: ") + Message, - Severity); + reportError(M, Twine("Invalid \"llvm.loop\" metadata: ") + Message, Severity); } enum class EntryPropsTag { @@ -358,13 +356,13 @@ static bool isLoopMDCompatible(Module &M, Metadata *MD) { if (HintStr->getString() == "llvm.loop.unroll.count") { if (!ValidCountNode(HintMD)) { - reportLoopError(M, "Second operand of \"llvm.loop.unroll.count\" " - "must be a constant integer"); + reportLoopError(M, "\"llvm.loop.unroll.count\" must have 2 operands and " + "the second must be a constant integer"); return false; } } else if (HintMD->getNumOperands() != 1) { reportLoopError( - M, "\"llvm.loop.unroll.disable\" and \"llvm.loop.unroll.disable\" " + M, "\"llvm.loop.unroll.disable\" and \"llvm.loop.unroll.full\" " "must be provided as a single operand"); return false; } diff --git a/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll b/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll index f67077f381208..6a4272a7c858f 100644 --- a/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll +++ b/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll @@ -61,7 +61,7 @@ exit: ;--- invalid-disable.ll -; CHECK: Invalid "llvm.loop" metadata: "llvm.loop.unroll.disable" and "llvm.loop.unroll.disable" must be provided as a single operand +; CHECK: Invalid "llvm.loop" metadata: "llvm.loop.unroll.disable" and "llvm.loop.unroll.full" must be provided as a single operand target triple = "dxilv1.0-unknown-shadermodel6.0-library" @@ -88,7 +88,7 @@ exit: ;--- invalid-full.ll -; CHECK: Invalid "llvm.loop" metadata: "llvm.loop.unroll.disable" and "llvm.loop.unroll.disable" must be provided as a single operand +; CHECK: Invalid "llvm.loop" metadata: "llvm.loop.unroll.disable" and "llvm.loop.unroll.full" must be provided as a single operand target triple = "dxilv1.0-unknown-shadermodel6.0-library" From 4cd0ccfa4d319a7f5f4ebfed20697491ef021ce9 Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Tue, 28 Oct 2025 10:33:38 -0700 Subject: [PATCH 17/20] review: clarify comments --- llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp index 66b877951bc2f..e345bda23b133 100644 --- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp +++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp @@ -376,7 +376,7 @@ static void translateLoopMetadata(Module &M, Instruction *I, MDNode *BaseMD) { return Node && Node->getNumOperands() != 0 && Node == Node->getOperand(0); }; - // Strip empty metadata or a non-distinct node + // Set metadata to null to remove empty/ill-formed metadata from instruction if (BaseMD->getNumOperands() == 0 || !IsDistinctNode(BaseMD)) return I->setMetadata("llvm.loop", nullptr); From f88477cd19a044322ca9960cd2c7ca7bfa8e0d2f Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Tue, 28 Oct 2025 10:38:31 -0700 Subject: [PATCH 18/20] review: correct test-cases --- llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll | 2 +- llvm/test/CodeGen/DirectX/metadata-stripping.ll | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll b/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll index 6a4272a7c858f..fbe4653b45dea 100644 --- a/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll +++ b/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll @@ -35,7 +35,7 @@ exit: ;--- bad-count.ll -; CHECK: Second operand of "llvm.loop.unroll.count" must be a constant integer +; CHECK: "llvm.loop.unroll.count" must have 2 operands and the second must be a constant integer target triple = "dxilv1.0-unknown-shadermodel6.0-library" diff --git a/llvm/test/CodeGen/DirectX/metadata-stripping.ll b/llvm/test/CodeGen/DirectX/metadata-stripping.ll index b717740f7255b..16c72da64203c 100644 --- a/llvm/test/CodeGen/DirectX/metadata-stripping.ll +++ b/llvm/test/CodeGen/DirectX/metadata-stripping.ll @@ -24,9 +24,10 @@ _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-DAG: [[RANGEMD]] = !{i32 1, i32 5} ; CHECK-DAG: [[LOOPMD]] = distinct !{[[LOOPMD]]} ; CHECK-NOT: {!"llvm.loop.mustprogress"} -!0 = distinct !{!0, !1, !2} +!0 = distinct !{!0, !1} !1 = !{!"llvm.loop.mustprogress"} !2 = !{i32 1, i32 5} From f2ffab2ffd897a2a9e12356f2a16a427ef76dbc5 Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Wed, 29 Oct 2025 10:33:07 -0700 Subject: [PATCH 19/20] review: fix formatting --- llvm/test/CodeGen/DirectX/metadata-stripping.ll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/test/CodeGen/DirectX/metadata-stripping.ll b/llvm/test/CodeGen/DirectX/metadata-stripping.ll index 16c72da64203c..338ca5d5d49e2 100644 --- a/llvm/test/CodeGen/DirectX/metadata-stripping.ll +++ b/llvm/test/CodeGen/DirectX/metadata-stripping.ll @@ -24,7 +24,7 @@ _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-NOT: !{!"llvm.loop.mustprogress"} ; CHECK-DAG: [[RANGEMD]] = !{i32 1, i32 5} ; CHECK-DAG: [[LOOPMD]] = distinct !{[[LOOPMD]]} ; CHECK-NOT: {!"llvm.loop.mustprogress"} From 198d809519d4013a00345cec43cb46a4ebb37f26 Mon Sep 17 00:00:00 2001 From: Finn Plummer Date: Wed, 29 Oct 2025 10:33:41 -0700 Subject: [PATCH 20/20] review: fix formatting --- llvm/test/CodeGen/DirectX/metadata-stripping.ll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/test/CodeGen/DirectX/metadata-stripping.ll b/llvm/test/CodeGen/DirectX/metadata-stripping.ll index 338ca5d5d49e2..53716ff29f292 100644 --- a/llvm/test/CodeGen/DirectX/metadata-stripping.ll +++ b/llvm/test/CodeGen/DirectX/metadata-stripping.ll @@ -27,7 +27,7 @@ _Z4mainDv3_j.exit: ; preds = %for.body.i, %entry ; CHECK-NOT: !{!"llvm.loop.mustprogress"} ; CHECK-DAG: [[RANGEMD]] = !{i32 1, i32 5} ; CHECK-DAG: [[LOOPMD]] = distinct !{[[LOOPMD]]} -; CHECK-NOT: {!"llvm.loop.mustprogress"} +; CHECK-NOT: !{!"llvm.loop.mustprogress"} !0 = distinct !{!0, !1} !1 = !{!"llvm.loop.mustprogress"} !2 = !{i32 1, i32 5}