https://github.com/inbelic updated https://github.com/llvm/llvm-project/pull/164292
>From 868c42bbd41aae5c43c89dd653d949418ec914f1 Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Fri, 17 Oct 2025 10:17:32 -0700 Subject: [PATCH 01/18] [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<DXILResourceTypeWrapperPass>(); - AU.addRequired<DXILResourceWrapperPass>(); - AU.addRequired<ShaderFlagsAnalysisWrapper>(); - AU.addRequired<DXILMetadataAnalysisWrapperPass>(); - AU.addRequired<RootSignatureAnalysisWrapper>(); - - AU.addPreserved<DXILMetadataAnalysisWrapperPass>(); - AU.addPreserved<DXILResourceBindingWrapperPass>(); - AU.addPreserved<DXILResourceWrapperPass>(); - AU.addPreserved<RootSignatureAnalysisWrapper>(); - AU.addPreserved<ShaderFlagsAnalysisWrapper>(); - } +void DXILTranslateMetadataLegacy::getAnalysisUsage(AnalysisUsage &AU) const { + AU.addRequired<DXILResourceTypeWrapperPass>(); + AU.addRequired<DXILResourceWrapperPass>(); + AU.addRequired<ShaderFlagsAnalysisWrapper>(); + AU.addRequired<DXILMetadataAnalysisWrapperPass>(); + AU.addRequired<RootSignatureAnalysisWrapper>(); + + AU.addPreserved<DXILMetadataAnalysisWrapperPass>(); + AU.addPreserved<DXILResourceBindingWrapperPass>(); + AU.addPreserved<DXILResourceWrapperPass>(); + AU.addPreserved<RootSignatureAnalysisWrapper>(); + AU.addPreserved<ShaderFlagsAnalysisWrapper>(); +} - bool runOnModule(Module &M) override { - DXILResourceMap &DRM = - getAnalysis<DXILResourceWrapperPass>().getResourceMap(); - DXILResourceTypeMap &DRTM = - getAnalysis<DXILResourceTypeWrapperPass>().getResourceTypeMap(); - const ModuleShaderFlags &ShaderFlags = - getAnalysis<ShaderFlagsAnalysisWrapper>().getShaderFlags(); - dxil::ModuleMetadataInfo MMDI = - getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata(); - - translateGlobalMetadata(M, DRM, DRTM, ShaderFlags, MMDI); - translateInstructionMetadata(M); - return true; - } -}; +bool DXILTranslateMetadataLegacy::runOnModule(Module &M) { + DXILResourceMap &DRM = + getAnalysis<DXILResourceWrapperPass>().getResourceMap(); + DXILResourceTypeMap &DRTM = + getAnalysis<DXILResourceTypeWrapperPass>().getResourceTypeMap(); + const ModuleShaderFlags &ShaderFlags = + getAnalysis<ShaderFlagsAnalysisWrapper>().getShaderFlags(); + dxil::ModuleMetadataInfo MMDI = + getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata(); -} // namespace + translateGlobalMetadata(M, DRM, DRTM, ShaderFlags, MMDI); + translateInstructionMetadata(M); + return true; +} char DXILTranslateMetadataLegacy::ID = 0; diff --git a/llvm/lib/Target/DirectX/DXILTranslateMetadata.h b/llvm/lib/Target/DirectX/DXILTranslateMetadata.h index 4c1ffac1781e6..cfb8aaa8f98b5 100644 --- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.h +++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.h @@ -10,6 +10,7 @@ #define LLVM_TARGET_DIRECTX_DXILTRANSLATEMETADATA_H #include "llvm/IR/PassManager.h" +#include "llvm/Pass.h" namespace llvm { @@ -20,6 +21,22 @@ class DXILTranslateMetadata : public PassInfoMixin<DXILTranslateMetadata> { PreservedAnalyses run(Module &M, ModuleAnalysisManager &); }; +/// Wrapper pass for the legacy pass manager. +/// +/// This is required because the passes that will depend on this are codegen +/// passes which run through the legacy pass manager. +class DXILTranslateMetadataLegacy : public ModulePass { +public: + static char ID; // Pass identification, replacement for typeid + explicit DXILTranslateMetadataLegacy() : ModulePass(ID) {} + + StringRef getPassName() const override { return "DXIL Translate Metadata"; } + + void getAnalysisUsage(AnalysisUsage &AU) const override; + + bool runOnModule(Module &M) override; +}; + } // namespace llvm #endif // LLVM_TARGET_DIRECTX_DXILTRANSLATEMETADATA_H diff --git a/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp b/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp new file mode 100644 index 0000000000000..199a34b8c2616 --- /dev/null +++ b/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp @@ -0,0 +1,90 @@ +//===- DXILValidateMetadata.cpp - Pass to validate DXIL metadata ----------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "DXILValidateMetadata.h" +#include "DXILTranslateMetadata.h" +#include "DirectX.h" +#include "llvm/ADT/Twine.h" +#include "llvm/IR/BasicBlock.h" +#include "llvm/IR/DiagnosticInfo.h" +#include "llvm/IR/DiagnosticPrinter.h" +#include "llvm/IR/Metadata.h" +#include "llvm/IR/Module.h" +#include "llvm/InitializePasses.h" +#include "llvm/Support/ErrorHandling.h" + +using namespace llvm; + +namespace { + +/// A simple Wrapper DiagnosticInfo that generates Module-level diagnostic +/// for the ValidateMetadata pass +class DiagnosticInfoValidateMD : public DiagnosticInfo { +private: + const Twine &Msg; + const Module &Mod; + +public: + /// \p M is the module for which the diagnostic is being emitted. \p Msg is + /// the message to show. Note that this class does not copy this message, so + /// this reference must be valid for the whole life time of the diagnostic. + DiagnosticInfoValidateMD(const Module &M, + const Twine &Msg LLVM_LIFETIME_BOUND, + DiagnosticSeverity Severity = DS_Error) + : DiagnosticInfo(DK_Unsupported, Severity), Msg(Msg), Mod(M) {} + + void print(DiagnosticPrinter &DP) const override { + DP << Mod.getName() << ": " << Msg << '\n'; + } +}; + +} // namespace + +static void validateInstructionMetadata(Module &M) { + llvm::errs() << "hello from new pass!\n"; +} + +PreservedAnalyses DXILValidateMetadata::run(Module &M, + ModuleAnalysisManager &MAM) { + validateInstructionMetadata(M); + + return PreservedAnalyses::all(); +} + +namespace { +class DXILValidateMetadataLegacy : public ModulePass { +public: + static char ID; // Pass identification, replacement for typeid + explicit DXILValidateMetadataLegacy() : ModulePass(ID) {} + + StringRef getPassName() const override { return "DXIL Validate Metadata"; } + + void getAnalysisUsage(AnalysisUsage &AU) const override { + AU.addRequired<DXILTranslateMetadataLegacy>(); + AU.setPreservesAll(); + } + + bool runOnModule(Module &M) override { + validateInstructionMetadata(); + return true; + } +}; + +} // namespace + +char DXILValidateMetadataLegacy::ID = 0; + +ModulePass *llvm::createDXILValidateMetadataLegacyPass() { + return new DXILValidateMetadataLegacy(); +} + +INITIALIZE_PASS_BEGIN(DXILValidateMetadataLegacy, "dxil-validate-metadata", + "DXIL Validate Metadata", false, false) +INITIALIZE_PASS_DEPENDENCY(DXILTranslateMetadataLegacy) +INITIALIZE_PASS_END(DXILValidateMetadataLegacy, "dxil-validate-metadata", + "DXIL validate Metadata", false, false) diff --git a/llvm/lib/Target/DirectX/DXILValidateMetadata.h b/llvm/lib/Target/DirectX/DXILValidateMetadata.h new file mode 100644 index 0000000000000..3188afafc8e22 --- /dev/null +++ b/llvm/lib/Target/DirectX/DXILValidateMetadata.h @@ -0,0 +1,24 @@ +//===- DXILValidateMetadata.h - Pass to emit DXIL metadata -----*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_TARGET_DIRECTX_DXILVALIDATEMETADATA_H +#define LLVM_TARGET_DIRECTX_DXILVALIDATEMETADATA_H + +#include "llvm/IR/PassManager.h" + +namespace llvm { + +/// A pass that transforms DXIL Intrinsics that don't have DXIL opCodes +class DXILValidateMetadata : public PassInfoMixin<DXILValidateMetadata> { +public: + PreservedAnalyses run(Module &M, ModuleAnalysisManager &); +}; + +} // namespace llvm + +#endif // LLVM_TARGET_DIRECTX_DXILVALIDATEMETADATA_H diff --git a/llvm/lib/Target/DirectX/DirectX.h b/llvm/lib/Target/DirectX/DirectX.h index e31c2ffa4f761..90546f133c9d2 100644 --- a/llvm/lib/Target/DirectX/DirectX.h +++ b/llvm/lib/Target/DirectX/DirectX.h @@ -90,6 +90,12 @@ void initializeDXILTranslateMetadataLegacyPass(PassRegistry &); /// Pass to emit metadata for DXIL. ModulePass *createDXILTranslateMetadataLegacyPass(); +/// Initializer for DXILValidateMetadata. +void initializeDXILValidateMetadataLegacyPass(PassRegistry &); + +/// Pass to validate metadata for DXIL. +ModulePass *createDXILValidateMetadataLegacyPass(); + /// Pass to pretty print DXIL metadata. ModulePass *createDXILPrettyPrinterLegacyPass(raw_ostream &OS); diff --git a/llvm/lib/Target/DirectX/DirectXPassRegistry.def b/llvm/lib/Target/DirectX/DirectXPassRegistry.def index b4b48a166800e..a69f9764b932b 100644 --- a/llvm/lib/Target/DirectX/DirectXPassRegistry.def +++ b/llvm/lib/Target/DirectX/DirectXPassRegistry.def @@ -31,6 +31,7 @@ MODULE_PASS("dxil-intrinsic-expansion", DXILIntrinsicExpansion()) MODULE_PASS("dxil-op-lower", DXILOpLowering()) MODULE_PASS("dxil-pretty-printer", DXILPrettyPrinterPass(dbgs())) MODULE_PASS("dxil-translate-metadata", DXILTranslateMetadata()) +MODULE_PASS("dxil-validate-metadata", DXILValidateMetadata()) MODULE_PASS("dxil-resource-implicit-binding", DXILResourceImplicitBinding()) MODULE_PASS("dxil-post-optimization-validation", DXILPostOptimizationValidation()) // TODO: rename to print<foo> after NPM switch diff --git a/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp b/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp index bcf84403b2c0d..09cbf031ddefe 100644 --- a/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp +++ b/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp @@ -27,6 +27,7 @@ #include "DXILRootSignature.h" #include "DXILShaderFlags.h" #include "DXILTranslateMetadata.h" +#include "DXILValidateMetadata.h" #include "DXILWriter/DXILWriterPass.h" #include "DirectX.h" #include "DirectXSubtarget.h" @@ -70,6 +71,7 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeDirectXTarget() { initializeDXILResourceAccessLegacyPass(*PR); initializeDXILResourceImplicitBindingLegacyPass(*PR); initializeDXILTranslateMetadataLegacyPass(*PR); + initializeDXILValidateMetadataLegacyPass(*PR); initializeDXILPostOptimizationValidationLegacyPass(*PR); initializeShaderFlagsAnalysisWrapperPass(*PR); initializeRootSignatureAnalysisWrapperPass(*PR); @@ -122,6 +124,7 @@ class DirectXPassConfig : public TargetPassConfig { addPass(createDXILLegalizeLegacyPass()); addPass(createDXILResourceImplicitBindingLegacyPass()); addPass(createDXILTranslateMetadataLegacyPass()); + addPass(createDXILValidateMetadataLegacyPass()); addPass(createDXILPostOptimizationValidationLegacyPass()); addPass(createDXILOpLoweringLegacyPass()); addPass(createDXILPrepareModulePass()); diff --git a/llvm/test/CodeGen/DirectX/llc-pipeline.ll b/llvm/test/CodeGen/DirectX/llc-pipeline.ll index d265826cd2469..13b502475b283 100644 --- a/llvm/test/CodeGen/DirectX/llc-pipeline.ll +++ b/llvm/test/CodeGen/DirectX/llc-pipeline.ll @@ -42,6 +42,7 @@ ; CHECK-NEXT: DXIL Shader Flag Analysis ; CHECK-NEXT: DXIL Root Signature Analysis ; CHECK-NEXT: DXIL Translate Metadata +; CHECK-NEXT: DXIL Validate Metadata ; CHECK-NEXT: DXIL Post Optimization Validation ; CHECK-NEXT: DXIL Op Lowering ; CHECK-NEXT: DXIL Prepare Module >From 7c8aa55f1168a22c1b14f667f75f9aec9e17a77c Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Mon, 20 Oct 2025 09:33:37 -0700 Subject: [PATCH 02/18] 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<DXILMetadataAnalysis>(M); + validateGlobalMetadata(M, MMDI); validateInstructionMetadata(M); return PreservedAnalyses::all(); @@ -65,12 +95,15 @@ class DXILValidateMetadataLegacy : public ModulePass { StringRef getPassName() const override { return "DXIL Validate Metadata"; } void getAnalysisUsage(AnalysisUsage &AU) const override { + AU.addRequired<DXILMetadataAnalysisWrapperPass>(); AU.addRequired<DXILTranslateMetadataLegacy>(); AU.setPreservesAll(); } bool runOnModule(Module &M) override { - validateInstructionMetadata(); + dxil::ModuleMetadataInfo MMDI = + getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata(); + validateGlobalMetadata(M, MMDI); return true; } }; @@ -85,6 +118,7 @@ ModulePass *llvm::createDXILValidateMetadataLegacyPass() { INITIALIZE_PASS_BEGIN(DXILValidateMetadataLegacy, "dxil-validate-metadata", "DXIL Validate Metadata", false, false) +INITIALIZE_PASS_DEPENDENCY(DXILMetadataAnalysisWrapperPass) INITIALIZE_PASS_DEPENDENCY(DXILTranslateMetadataLegacy) INITIALIZE_PASS_END(DXILValidateMetadataLegacy, "dxil-validate-metadata", "DXIL validate Metadata", false, false) diff --git a/llvm/test/CodeGen/DirectX/Metadata/multiple-entries-cs-error.ll b/llvm/test/CodeGen/DirectX/Metadata/multiple-entries-cs-error.ll index 9697d4389a888..cba10336f7817 100644 --- a/llvm/test/CodeGen/DirectX/Metadata/multiple-entries-cs-error.ll +++ b/llvm/test/CodeGen/DirectX/Metadata/multiple-entries-cs-error.ll @@ -1,4 +1,4 @@ -; RUN: not opt -S -S -dxil-translate-metadata %s 2>&1 | FileCheck %s +; RUN: not opt -S -dxil-validate-metadata %s 2>&1 | FileCheck %s target triple = "dxil-pc-shadermodel6.8-compute" ; CHECK: Non-library shader: One and only one entry expected diff --git a/llvm/test/CodeGen/DirectX/Metadata/target-profile-error.ll b/llvm/test/CodeGen/DirectX/Metadata/target-profile-error.ll index 671406cb5d364..13e28cfebf9af 100644 --- a/llvm/test/CodeGen/DirectX/Metadata/target-profile-error.ll +++ b/llvm/test/CodeGen/DirectX/Metadata/target-profile-error.ll @@ -1,8 +1,8 @@ -; RUN: not opt -S -dxil-translate-metadata %s 2>&1 | FileCheck %s +; RUN: not opt -S -dxil-validate-metadata %s 2>&1 | FileCheck %s target triple = "dxil-pc-shadermodel6.6-pixel" -; CHECK: Shader stage 'cs' for entry 'entry' different from specified target profile 'pixel' +; CHECK: Shader stage 'compute' for entry 'entry' different from specified target profile 'pixel' define void @entry() #0 { entry: >From 9541b049810894b50d7aad952e3e6acb689485c7 Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Mon, 20 Oct 2025 09:48:57 -0700 Subject: [PATCH 03/18] [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<unsigned, 6> getCompatibleInstructionMDs(llvm::Module &M) { +static std::array<unsigned, 7> getCompatibleInstructionMDs(llvm::Module &M) { return { M.getMDKindID("dx.nonuniform"), M.getMDKindID("dx.controlflow.hints"), M.getMDKindID("dx.precise"), llvm::LLVMContext::MD_range, - llvm::LLVMContext::MD_alias_scope, llvm::LLVMContext::MD_noalias}; + llvm::LLVMContext::MD_alias_scope, llvm::LLVMContext::MD_noalias, + M.getMDKindID("llvm.loop")}; } static void translateInstructionMetadata(Module &M) { // construct allowlist of valid metadata node kinds - std::array<unsigned, 6> DXILCompatibleMDs = getCompatibleInstructionMDs(M); + std::array<unsigned, 7> DXILCompatibleMDs = getCompatibleInstructionMDs(M); for (Function &F : M) { for (BasicBlock &BB : F) { >From fd8b27c8869c3e0240a2d050e9f1604d4197da29 Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Mon, 20 Oct 2025 09:49:18 -0700 Subject: [PATCH 04/18] [DirectX] add DXIL validation of "llvm.loop" metadata --- .../Target/DirectX/DXILValidateMetadata.cpp | 94 +++++++++++- .../CodeGen/DirectX/Metadata/loop-md-errs.ll | 135 ++++++++++++++++++ .../CodeGen/DirectX/Metadata/loop-md-valid.ll | 31 ++++ .../CodeGen/DirectX/metadata-stripping.ll | 7 +- 4 files changed, 262 insertions(+), 5 deletions(-) create mode 100644 llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll create mode 100644 llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll diff --git a/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp b/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp index 84f17770166be..275d3686dd7e9 100644 --- a/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp +++ b/llvm/lib/Target/DirectX/DXILValidateMetadata.cpp @@ -9,14 +9,19 @@ #include "DXILValidateMetadata.h" #include "DXILTranslateMetadata.h" #include "DirectX.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/StringRef.h" #include "llvm/ADT/Twine.h" #include "llvm/Analysis/DXILMetadataAnalysis.h" #include "llvm/IR/BasicBlock.h" +#include "llvm/IR/Constants.h" #include "llvm/IR/DiagnosticInfo.h" #include "llvm/IR/DiagnosticPrinter.h" +#include "llvm/IR/LLVMContext.h" #include "llvm/IR/Metadata.h" #include "llvm/IR/Module.h" #include "llvm/InitializePasses.h" +#include "llvm/Support/Casting.h" #include "llvm/Support/ErrorHandling.h" using namespace llvm; @@ -50,10 +55,96 @@ static bool reportError(Module &M, Twine Message, return true; } +static bool reportLoopError(Module &M, Twine Message, + DiagnosticSeverity Severity = DS_Error) { + return reportError(M, Twine("Invalid \"llvm.loop\" metadata: ") + Message, + Severity); +} + } // namespace +static void validateLoopMetadata(Module &M, MDNode *LoopMD) { + // DXIL only accepts the following loop hints: + // llvm.loop.unroll.disable, llvm.loop.unroll.full, llvm.loop.unroll.count + std::array<StringLiteral, 3> ValidHintNames = {"llvm.loop.unroll.count", + "llvm.loop.unroll.disable", + "llvm.loop.unroll.full"}; + + // llvm.loop metadata must have it's first operand be a self-reference, so we + // require at least 1 operand. + // + // It only makes sense to specify up to 1 of the hints on a branch, so we can + // have at most 2 operands. + + if (LoopMD->getNumOperands() != 1 && LoopMD->getNumOperands() != 2) { + reportLoopError(M, "Requires exactly 1 or 2 operands"); + return; + } + + if (LoopMD != LoopMD->getOperand(0)) { + reportLoopError(M, "First operand must be a self-reference"); + return; + } + + // A node only containing a self-reference is a valid use to denote a loop + if (LoopMD->getNumOperands() == 1) + return; + + LoopMD = dyn_cast<MDNode>(LoopMD->getOperand(1)); + if (!LoopMD) { + reportLoopError(M, "Second operand must be a metadata node"); + return; + } + + if (LoopMD->getNumOperands() != 1 && LoopMD->getNumOperands() != 2) { + reportLoopError(M, "Requires exactly 1 or 2 operands"); + return; + } + + // It is valid to have a chain of self-referential loop metadata nodes so if + // we have another self-reference, recurse. + // + // Eg: + // !0 = !{!0, !1} + // !1 = !{!1, !2} + // !2 = !{"llvm.loop.unroll.disable"} + if (LoopMD == LoopMD->getOperand(0)) + return validateLoopMetadata(M, LoopMD); + + // Otherwise, we are at our base hint metadata node + auto *HintStr = dyn_cast<MDString>(LoopMD->getOperand(0)); + if (!HintStr || !llvm::is_contained(ValidHintNames, HintStr->getString())) { + reportLoopError(M, + "First operand must be a valid \"llvm.loop.unroll\" hint"); + return; + } + + // Ensure count node is a constant integer value + auto ValidCountNode = [](MDNode *HintMD) -> bool { + if (HintMD->getNumOperands() == 2) + if (auto *CountMD = dyn_cast<ConstantAsMetadata>(HintMD->getOperand(1))) + if (isa<ConstantInt>(CountMD->getValue())) + return true; + return false; + }; + + if (HintStr->getString() == "llvm.loop.unroll.count" && + !ValidCountNode(LoopMD)) { + reportLoopError(M, "Second operand of \"llvm.loop.unroll.count\" " + "must be a constant integer"); + return; + } +} + static void validateInstructionMetadata(Module &M) { - llvm::errs() << "hello from new pass!\n"; + unsigned char MDLoopKind = M.getContext().getMDKindID("llvm.loop"); + + for (Function &F : M) + for (BasicBlock &BB : F) + for (Instruction &I : BB) { + if (MDNode *LoopMD = I.getMetadata(MDLoopKind)) + validateLoopMetadata(M, LoopMD); + } } static void validateGlobalMetadata(Module &M, @@ -104,6 +195,7 @@ class DXILValidateMetadataLegacy : public ModulePass { dxil::ModuleMetadataInfo MMDI = getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata(); validateGlobalMetadata(M, MMDI); + validateInstructionMetadata(M); return true; } }; diff --git a/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll b/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll new file mode 100644 index 0000000000000..1bf16a6559d82 --- /dev/null +++ b/llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll @@ -0,0 +1,135 @@ +; RUN: split-file %s %t +; RUN: not opt -S --dxil-validate-metadata %t/args.ll 2>&1 | FileCheck %t/args.ll +; RUN: not opt -S --dxil-validate-metadata %t/not-ref.ll 2>&1 | FileCheck %t/not-ref.ll +; RUN: not opt -S --dxil-validate-metadata %t/not-md.ll 2>&1 | FileCheck %t/not-md.ll +; RUN: not opt -S --dxil-validate-metadata %t/not-str.ll 2>&1 | FileCheck %t/not-str.ll +; RUN: not opt -S --dxil-validate-metadata %t/bad-count.ll 2>&1 | FileCheck %t/bad-count.ll + +; Test that loop metadata is validated as with the DXIL validator + +;--- args.ll + +; CHECK: Invalid "llvm.loop" metadata: Requires exactly 1 or 2 operands + +target triple = "dxilv1.0-unknown-shadermodel6.0-library" + +define void @example_loop(i32 %n) { +entry: + br label %loop.header + +loop.header: + %i = phi i32 [ 0, %entry ], [ %i.next, %loop.body ] + %cmp = icmp slt i32 %i, %n + br i1 %cmp, label %loop.body, label %exit + +loop.body: + %i.next = add nsw i32 %i, 1 + br label %loop.header, !llvm.loop !1 + +exit: + ret void +} + +!1 = !{!1, !1, !1} ; too many args + +;--- not-ref.ll + +; CHECK: Invalid "llvm.loop" metadata: First operand must be a self-reference + +target triple = "dxilv1.0-unknown-shadermodel6.0-library" + +define void @example_loop(i32 %n) { +entry: + br label %loop.header + +loop.header: + %i = phi i32 [ 0, %entry ], [ %i.next, %loop.body ] + %cmp = icmp slt i32 %i, %n + br i1 %cmp, label %loop.body, label %exit + +loop.body: + %i.next = add nsw i32 %i, 1 + br label %loop.header, !llvm.loop !1 + +exit: + ret void +} + +!1 = !{i32 0} ; not a self-reference + +;--- not-md.ll + +; CHECK: Invalid "llvm.loop" metadata: Second operand must be a metadata node + +target triple = "dxilv1.0-unknown-shadermodel6.0-library" + +define void @example_loop(i32 %n) { +entry: + br label %loop.header + +loop.header: + %i = phi i32 [ 0, %entry ], [ %i.next, %loop.body ] + %cmp = icmp slt i32 %i, %n + br i1 %cmp, label %loop.body, label %exit + +loop.body: + %i.next = add nsw i32 %i, 1 + br label %loop.header, !llvm.loop !1 + +exit: + ret void +} + +!1 = !{!1, i32 0} ; not a metadata node + +;--- not-str.ll + +; CHECK: Invalid "llvm.loop" metadata: First operand must be a valid "llvm.loop.unroll" hint + +target triple = "dxilv1.0-unknown-shadermodel6.0-library" + +define void @example_loop(i32 %n) { +entry: + br label %loop.header + +loop.header: + %i = phi i32 [ 0, %entry ], [ %i.next, %loop.body ] + %cmp = icmp slt i32 %i, %n + br i1 %cmp, label %loop.body, label %exit + +loop.body: + %i.next = add nsw i32 %i, 1 + br label %loop.header, !llvm.loop !1 + +exit: + ret void +} + +!1 = !{!1, !2} +!2 = !{i32 0} ; not a hint name string + +;--- bad-count.ll + +; CHECK: Second operand of "llvm.loop.unroll.count" must be a constant integer + +target triple = "dxilv1.0-unknown-shadermodel6.0-library" + +define void @example_loop(i32 %n) { +entry: + br label %loop.header + +loop.header: + %i = phi i32 [ 0, %entry ], [ %i.next, %loop.body ] + %cmp = icmp slt i32 %i, %n + br i1 %cmp, label %loop.body, label %exit + +loop.body: + %i.next = add nsw i32 %i, 1 + br label %loop.header, !llvm.loop !1 + +exit: + ret void +} + +!1 = !{!1, !2} +!2 = !{!"llvm.loop.unroll.count", !"not an int"} ; invalid count parameters diff --git a/llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll b/llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll new file mode 100644 index 0000000000000..8dfc0c396aed6 --- /dev/null +++ b/llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll @@ -0,0 +1,31 @@ +; RUN: opt -S --dxil-validate-metadata %s | FileCheck %s + +; Test that we allow a self-referential chain and a constant integer in count + +target triple = "dxilv1.0-unknown-shadermodel6.0-library" + +define void @example_loop(i32 %n) { +entry: + br label %loop.header + +loop.header: + %i = phi i32 [ 0, %entry ], [ %i.next, %loop.body ] + %cmp = icmp slt i32 %i, %n + br i1 %cmp, label %loop.body, label %exit + +loop.body: + %i.next = add nsw i32 %i, 1 + ; CHECK: br label %loop.header, !llvm.loop ![[#LOOP_MD:]] + br label %loop.header, !llvm.loop !0 + +exit: + ret void +} + +; CHECK: ![[#LOOP_MD]] = distinct !{![[#LOOP_MD]], ![[#EXTRA_REF:]]} +; CHECK: ![[#EXTRA_REF]] = distinct !{![[#EXTRA_REF]], ![[#COUNT:]]} +; CHECK: ![[#COUNT]] = !{!"llvm.loop.unroll.count", i6 4} + +!0 = !{!0, !1} +!1 = !{!1, !2} +!2 = !{!"llvm.loop.unroll.count", i6 4} diff --git a/llvm/test/CodeGen/DirectX/metadata-stripping.ll b/llvm/test/CodeGen/DirectX/metadata-stripping.ll index 531ab6c334d24..f33f432777b61 100644 --- a/llvm/test/CodeGen/DirectX/metadata-stripping.ll +++ b/llvm/test/CodeGen/DirectX/metadata-stripping.ll @@ -14,7 +14,7 @@ entry: %cmp.i = icmp ult i32 1, 2 ; Ensure that the !llvm.loop metadata node gets dropped. - ; CHECK: br i1 %cmp.i, label %_Z4mainDv3_j.exit, label %_Z4mainDv3_j.exit{{$}} + ; CHECK: br i1 %cmp.i, label %_Z4mainDv3_j.exit, label %_Z4mainDv3_j.exit, !llvm.loop [[LOOPMD:![0-9]+]] br i1 %cmp.i, label %_Z4mainDv3_j.exit, label %_Z4mainDv3_j.exit, !llvm.loop !0 _Z4mainDv3_j.exit: ; preds = %for.body.i, %entry @@ -24,9 +24,8 @@ _Z4mainDv3_j.exit: ; preds = %for.body.i, %entry ; These next check lines check that only the range metadata remains ; No more metadata should be necessary, the rest (the current 0 and 1) ; should be removed. -; CHECK-NOT: !{!"llvm.loop.mustprogress"} -; CHECK: [[RANGEMD]] = !{i32 1, i32 5} -; CHECK-NOT: !{!"llvm.loop.mustprogress"} +; CHECK-DAG: [[RANGEMD]] = !{i32 1, i32 5} +; CHECK-DAG: !{!"llvm.loop.mustprogress"} !0 = distinct !{!0, !1} !1 = !{!"llvm.loop.mustprogress"} !2 = !{i32 1, i32 5} >From 8cec5aab67e475690d385ec98b6db32388e4f20e Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Thu, 23 Oct 2025 14:54:23 -0700 Subject: [PATCH 05/18] 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<unsigned, 7> getCompatibleInstructionMDs(llvm::Module &M) { +using InstructionMDList = std::array<unsigned, 7>; + +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<unsigned, 7> getCompatibleInstructionMDs(llvm::Module &M) { static void translateInstructionMetadata(Module &M) { // construct allowlist of valid metadata node kinds - std::array<unsigned, 7> DXILCompatibleMDs = getCompatibleInstructionMDs(M); + InstructionMDList DXILCompatibleMDs = getCompatibleInstructionMDs(M); for (Function &F : M) { for (BasicBlock &BB : F) { >From 45abd1970c142664b8366c4e316447c311616698 Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Thu, 23 Oct 2025 15:00:36 -0700 Subject: [PATCH 06/18] 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<DXILValidateMetadata> { public: PreservedAnalyses run(Module &M, ModuleAnalysisManager &); >From f3fa7aa274cff753a6e8ac75d3076ce5d6dd99e6 Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Thu, 23 Oct 2025 15:07:11 -0700 Subject: [PATCH 07/18] 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<BranchInst>(I)) + if (MDNode *LoopMD = I.getMetadata(MDLoopKind)) + validateLoopMetadata(M, LoopMD); } } >From b3bf5cb75d2e406ca409f1980aa5e27b7bf15215 Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Thu, 23 Oct 2025 15:11:05 -0700 Subject: [PATCH 08/18] 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 f027a93a74bb465c24476513647afe9851c5bbb0 Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Thu, 23 Oct 2025 15:21:27 -0700 Subject: [PATCH 09/18] 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 24909ab182da3eac57a1d5250ca59ee5c66adf06 Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Fri, 24 Oct 2025 11:08:21 -0700 Subject: [PATCH 10/18] 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<StringLiteral, 3> 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<MDNode>(LoopMD->getOperand(1)); + if (!LoopMD) { + reportLoopError(M, "Second operand must be a metadata node"); + return; + } + + if (LoopMD->getNumOperands() != 1 && LoopMD->getNumOperands() != 2) { + reportLoopError(M, "Requires exactly 1 or 2 operands"); + return; + } + + // It is valid to have a chain of self-referential loop metadata nodes so if + // we have another self-reference, recurse. + // + // Eg: + // !0 = !{!0, !1} + // !1 = !{!1, !2} + // !2 = !{"llvm.loop.unroll.disable"} + if (LoopMD == LoopMD->getOperand(0)) + return translateLoopMetadata(M, LoopMD); + + // Otherwise, we are at our base hint metadata node + auto *HintStr = dyn_cast<MDString>(LoopMD->getOperand(0)); + if (!HintStr || !llvm::is_contained(ValidHintNames, HintStr->getString())) { + reportLoopError(M, + "First operand must be a valid \"llvm.loop.unroll\" hint"); + return; + } + + // Ensure count node is a constant integer value + auto ValidCountNode = [](MDNode *HintMD) -> bool { + if (HintMD->getNumOperands() == 2) + if (auto *CountMD = dyn_cast<ConstantAsMetadata>(HintMD->getOperand(1))) + if (isa<ConstantInt>(CountMD->getValue())) + return true; + return false; + }; + + if (HintStr->getString() == "llvm.loop.unroll.count") { + 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<unsigned, 7>; 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<BranchInst>(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<StringLiteral, 3> 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<MDNode>(LoopMD->getOperand(1)); - if (!LoopMD) { - reportLoopError(M, "Second operand must be a metadata node"); - return; - } - - if (LoopMD->getNumOperands() != 1 && LoopMD->getNumOperands() != 2) { - reportLoopError(M, "Requires exactly 1 or 2 operands"); - return; - } - - // It is valid to have a chain of self-referential loop metadata nodes so if - // we have another self-reference, recurse. - // - // Eg: - // !0 = !{!0, !1} - // !1 = !{!1, !2} - // !2 = !{"llvm.loop.unroll.disable"} - if (LoopMD == LoopMD->getOperand(0)) - return validateLoopMetadata(M, LoopMD); - - // Otherwise, we are at our base hint metadata node - auto *HintStr = dyn_cast<MDString>(LoopMD->getOperand(0)); - if (!HintStr || !llvm::is_contained(ValidHintNames, HintStr->getString())) { - reportLoopError(M, - "First operand must be a valid \"llvm.loop.unroll\" hint"); - return; - } - - // Ensure count node is a constant integer value - auto ValidCountNode = [](MDNode *HintMD) -> bool { - if (HintMD->getNumOperands() == 2) - if (auto *CountMD = dyn_cast<ConstantAsMetadata>(HintMD->getOperand(1))) - if (isa<ConstantInt>(CountMD->getValue())) - return true; - return false; - }; - - if (HintStr->getString() == "llvm.loop.unroll.count") { - 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<BranchInst>(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<DXILMetadataAnalysis>(M); - validateGlobalMetadata(M, MMDI); - validateInstructionMetadata(M); - - return PreservedAnalyses::all(); -} - -namespace { -class DXILValidateMetadataLegacy : public ModulePass { -public: - static char ID; // Pass identification, replacement for typeid - explicit DXILValidateMetadataLegacy() : ModulePass(ID) {} - - StringRef getPassName() const override { return "DXIL Validate Metadata"; } - - void getAnalysisUsage(AnalysisUsage &AU) const override { - AU.addRequired<DXILMetadataAnalysisWrapperPass>(); - AU.addRequired<DXILTranslateMetadataLegacy>(); - AU.setPreservesAll(); - } - - bool runOnModule(Module &M) override { - dxil::ModuleMetadataInfo MMDI = - getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata(); - validateGlobalMetadata(M, MMDI); - validateInstructionMetadata(M); - return true; - } -}; - -} // namespace - -char DXILValidateMetadataLegacy::ID = 0; - -ModulePass *llvm::createDXILValidateMetadataLegacyPass() { - return new DXILValidateMetadataLegacy(); -} - -INITIALIZE_PASS_BEGIN(DXILValidateMetadataLegacy, "dxil-validate-metadata", - "DXIL Validate Metadata", false, false) -INITIALIZE_PASS_DEPENDENCY(DXILMetadataAnalysisWrapperPass) -INITIALIZE_PASS_DEPENDENCY(DXILTranslateMetadataLegacy) -INITIALIZE_PASS_END(DXILValidateMetadataLegacy, "dxil-validate-metadata", - "DXIL validate Metadata", false, false) diff --git a/llvm/lib/Target/DirectX/DXILValidateMetadata.h b/llvm/lib/Target/DirectX/DXILValidateMetadata.h 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<DXILValidateMetadata> { -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<foo> 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 062c7b6f995fb53e89e89a9ce81a442a68cea004 Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Fri, 24 Oct 2025 14:04:40 -0700 Subject: [PATCH 11/18] 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<StringLiteral, 3> 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<MDNode>(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<MDString>(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<ConstantAsMetadata>(CountMD->getOperand(1))) + if (isa<ConstantInt>(Count->getValue())) + return true; + return false; + }; - LoopMD = dyn_cast<MDNode>(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<MDString>(LoopMD->getOperand(0)); - if (!HintStr || !llvm::is_contained(ValidHintNames, HintStr->getString())) { - reportLoopError(M, - "First operand must be a valid \"llvm.loop.unroll\" hint"); - return; - } - - // Ensure count node is a constant integer value - auto ValidCountNode = [](MDNode *HintMD) -> bool { - if (HintMD->getNumOperands() == 2) - if (auto *CountMD = dyn_cast<ConstantAsMetadata>(HintMD->getOperand(1))) - if (isa<ConstantInt>(CountMD->getValue())) - return true; - return false; - }; - - if (HintStr->getString() == "llvm.loop.unroll.count") { - 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<MDNode>(BaseMD->getOperand(1)))) + BaseMD = dyn_cast<MDNode>(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<Metadata *> CompatibleOperands = {TempNode.get()}; + + // Iterate and reconstruct the metadata nodes that contains any hints, + // stripping any unrecognized metadata. + ArrayRef<MDOperand> 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<unsigned, 7>; @@ -427,10 +436,9 @@ static void translateInstructionMetadata(Module &M) { translateBranchMetadata(M, I); for (auto &I : make_early_inc_range(BB)) { - if (isa<BranchInst>(I)) { + if (isa<BranchInst>(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 10ab3acb4e894047d0aa70c80d858d8807187ff9 Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Fri, 24 Oct 2025 16:06:55 -0700 Subject: [PATCH 12/18] 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 27f1264ccf9be954c1a4276d145db0060d417be9 Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Mon, 27 Oct 2025 09:16:50 -0700 Subject: [PATCH 13/18] 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 b1c15aba814bd4c29ddfa172e3ffae66304fc4a8 Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Mon, 27 Oct 2025 09:17:09 -0700 Subject: [PATCH 14/18] 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 269449547e410055252e58720ccc3518316dfac4 Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Mon, 27 Oct 2025 09:19:41 -0700 Subject: [PATCH 15/18] 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 e702f0173e2fb14f54a79cff1810f830297936fb Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Tue, 28 Oct 2025 10:32:00 -0700 Subject: [PATCH 16/18] 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 468b3a3718852baf724dda8f64d4d594e635c146 Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Tue, 28 Oct 2025 10:33:38 -0700 Subject: [PATCH 17/18] 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 65f3a562599aef14eafefe48547538ccda69473c Mon Sep 17 00:00:00 2001 From: Finn Plummer <[email protected]> Date: Tue, 28 Oct 2025 10:38:31 -0700 Subject: [PATCH 18/18] 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} _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
