From 50a114fa0caa162bb79815e17b1b2b451c698ea5 Mon Sep 17 00:00:00 2001 From: Ashley Coleman Date: Mon, 28 Apr 2025 13:28:46 -0600 Subject: [PATCH 1/2] [HLSL] Raise Diag for Invalid CounterDirection --- llvm/include/llvm/Analysis/DXILResource.h | 19 ++- llvm/lib/Target/DirectX/CMakeLists.txt | 1 + .../DXILPostOptimizationValidation.cpp | 119 ++++++++++++++++++ .../DirectX/DXILPostOptimizationValidation.h | 28 +++++ 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 + .../CodeGen/DirectX/resource_counter_error.ll | 10 ++ 9 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp create mode 100644 llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h create mode 100644 llvm/test/CodeGen/DirectX/resource_counter_error.ll diff --git a/llvm/include/llvm/Analysis/DXILResource.h b/llvm/include/llvm/Analysis/DXILResource.h index 2631c3cb94c8ac..102b184af00ba9 100644 --- a/llvm/include/llvm/Analysis/DXILResource.h +++ b/llvm/include/llvm/Analysis/DXILResource.h @@ -451,8 +451,10 @@ ModulePass *createDXILResourceTypeWrapperPassPass(); //===----------------------------------------------------------------------===// class DXILResourceMap { + using CallMapTy = DenseMap; + SmallVector Infos; - DenseMap CallMap; + CallMapTy CallMap; unsigned FirstUAV = 0; unsigned FirstCBuffer = 0; unsigned FirstSampler = 0; @@ -532,6 +534,21 @@ class DXILResourceMap { return make_range(sampler_begin(), sampler_end()); } + struct call_iterator + : iterator_adaptor_base { + call_iterator() = default; + call_iterator(CallMapTy::iterator Iter) + : call_iterator::iterator_adaptor_base(std::move(Iter)) {} + + CallInst *operator*() const { return I->first; } + }; + + call_iterator call_begin() { return call_iterator(CallMap.begin()); } + call_iterator call_end() { return call_iterator(CallMap.end()); } + iterator_range calls() { + return make_range(call_begin(), call_end()); + } + void print(raw_ostream &OS, DXILResourceTypeMap &DRTM, const DataLayout &DL) const; diff --git a/llvm/lib/Target/DirectX/CMakeLists.txt b/llvm/lib/Target/DirectX/CMakeLists.txt index 65105d3a5f4c38..01e0ef7e9bbc9d 100644 --- a/llvm/lib/Target/DirectX/CMakeLists.txt +++ b/llvm/lib/Target/DirectX/CMakeLists.txt @@ -28,6 +28,7 @@ add_llvm_target(DirectXCodeGen DXILIntrinsicExpansion.cpp DXILOpBuilder.cpp DXILOpLowering.cpp + DXILPostOptimizationValidation.cpp DXILPrepare.cpp DXILPrettyPrinter.cpp DXILResourceAccess.cpp diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp new file mode 100644 index 00000000000000..19345f1a469eeb --- /dev/null +++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp @@ -0,0 +1,119 @@ +//===- DXILPostOptimizationValidation.cpp - Opt DXIL validation ----------===// +// +// 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 "DXILPostOptimizationValidation.h" +#include "DXILConstants.h" +#include "DXILIntrinsicExpansion.h" +#include "DXILOpBuilder.h" +#include "DXILShaderFlags.h" +#include "DirectX.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/Analysis/DXILMetadataAnalysis.h" +#include "llvm/Analysis/DXILResource.h" +#include "llvm/CodeGen/Passes.h" +#include "llvm/IR/DiagnosticInfo.h" +#include "llvm/IR/Function.h" +#include "llvm/IR/IRBuilder.h" +#include "llvm/IR/Instruction.h" +#include "llvm/IR/Instructions.h" +#include "llvm/IR/Intrinsics.h" +#include "llvm/IR/IntrinsicsDirectX.h" +#include "llvm/IR/Module.h" +#include "llvm/IR/PassManager.h" +#include "llvm/InitializePasses.h" +#include "llvm/Pass.h" +#include "llvm/Support/ErrorHandling.h" +#include + +#define DEBUG_TYPE "dxil-post-optimization-validation" + +using namespace llvm; +using namespace llvm::dxil; + +namespace { +class DXILValidator { + Module &M; + DXILResourceMap &DRM; + +public: + DXILValidator(Module &M, DXILResourceMap &DRM) : M(M), DRM(DRM) {} + + void validate() { + for (const auto &UAV : DRM.uavs()) { + if (UAV.CounterDirection != ResourceCounterDirection::Invalid) + continue; + + CallInst *ResourceHandle = nullptr; + for (CallInst *MaybeHandle : DRM.calls()) { + if (*DRM.find(MaybeHandle) == UAV) { + ResourceHandle = MaybeHandle; + break; + } + } + + StringRef Message = + "RWStructuredBuffers may increment or decrement their " + "counters, but not both."; + for (const auto &U : ResourceHandle->users()) { + const CallInst *CI = dyn_cast(U); + if (!CI && CI->getIntrinsicID() != Intrinsic::dx_resource_updatecounter) + continue; + + M.getContext().diagnose(DiagnosticInfoGenericWithLoc( + Message, *CI->getFunction(), CI->getDebugLoc())); + } + } + } +}; +} // namespace + +PreservedAnalyses +DXILPostOptimizationValidation::run(Module &M, ModuleAnalysisManager &MAM) { + DXILResourceMap &DRM = MAM.getResult(M); + + DXILValidator(M, DRM).validate(); + return PreservedAnalyses::all(); +} + +namespace { +class DXILPostOptimizationValidationLegacy : public ModulePass { +public: + bool runOnModule(Module &M) override { + DXILResourceMap &DRM = + getAnalysis().getResourceMap(); + + DXILValidator(M, DRM).validate(); + + return false; + } + StringRef getPassName() const override { + return "DXIL Post Optimization Validation"; + } + DXILPostOptimizationValidationLegacy() : ModulePass(ID) {} + + static char ID; // Pass identification. + void getAnalysisUsage(llvm::AnalysisUsage &AU) const override { + AU.addRequired(); + AU.addPreserved(); + AU.addPreserved(); + AU.addPreserved(); + } +}; +char DXILPostOptimizationValidationLegacy::ID = 0; +} // end anonymous namespace + +INITIALIZE_PASS_BEGIN(DXILPostOptimizationValidationLegacy, DEBUG_TYPE, + "DXIL Post Optimization Validation", false, false) +INITIALIZE_PASS_DEPENDENCY(DXILResourceTypeWrapperPass) +INITIALIZE_PASS_DEPENDENCY(DXILResourceWrapperPass) +INITIALIZE_PASS_END(DXILPostOptimizationValidationLegacy, DEBUG_TYPE, + "DXIL Post Optimization Validation", false, false) + +ModulePass *llvm::createDXILPostOptimizationValidationLegacyPass() { + return new DXILPostOptimizationValidationLegacy(); +} diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h new file mode 100644 index 00000000000000..5a7f7454c8390b --- /dev/null +++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h @@ -0,0 +1,28 @@ +//===- DXILPostOptimizationValidation.h - Opt DXIL Validations -*- 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 +// +//===----------------------------------------------------------------------===// +// +// \file Pass for validating DXIL after lowering and optimizations are applied. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_LIB_TARGET_DIRECTX_DXILPOSTOPTIMIZATIONVALIDATION_H +#define LLVM_LIB_TARGET_DIRECTX_DXILPOSTOPTIMIZATIONVALIDATION_H + +#include "llvm/IR/PassManager.h" + +namespace llvm { + +class DXILPostOptimizationValidation + : public PassInfoMixin { +public: + PreservedAnalyses run(Module &M, ModuleAnalysisManager &MAM); +}; + +} // namespace llvm + +#endif // LLVM_LIB_TARGET_DIRECTX_DXILPOSTOPTIMIZATIONVALIDATION_H diff --git a/llvm/lib/Target/DirectX/DirectX.h b/llvm/lib/Target/DirectX/DirectX.h index f64aaaf65d937a..f52c581e8f308f 100644 --- a/llvm/lib/Target/DirectX/DirectX.h +++ b/llvm/lib/Target/DirectX/DirectX.h @@ -90,6 +90,12 @@ ModulePass *createDXILPrettyPrinterLegacyPass(raw_ostream &OS); /// Initializer for DXILPrettyPrinter. void initializeDXILPrettyPrinterLegacyPass(PassRegistry &); +/// Initializer for DXILPostOptimizationValidation. +void initializeDXILPostOptimizationValidationLegacyPass(PassRegistry &); + +/// Pass to lowering LLVM intrinsic call to DXIL op function call. +ModulePass *createDXILPostOptimizationValidationLegacyPass(); + /// Initializer for dxil::ShaderFlagsAnalysisWrapper pass. void initializeShaderFlagsAnalysisWrapperPass(PassRegistry &); diff --git a/llvm/lib/Target/DirectX/DirectXPassRegistry.def b/llvm/lib/Target/DirectX/DirectXPassRegistry.def index da239402d01eb1..2d57483d7e8e36 100644 --- a/llvm/lib/Target/DirectX/DirectXPassRegistry.def +++ b/llvm/lib/Target/DirectX/DirectXPassRegistry.def @@ -30,6 +30,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-post-optimization-validation", DXILPostOptimizationValidation()) // TODO: rename to print after NPM switch MODULE_PASS("print-dx-shader-flags", dxil::ShaderFlagsAnalysisPrinter(dbgs())) MODULE_PASS("print", dxil::RootSignatureAnalysisPrinter(dbgs())) diff --git a/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp b/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp index 398abd66dda161..19cfa89bb75eee 100644 --- a/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp +++ b/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp @@ -19,6 +19,7 @@ #include "DXILIntrinsicExpansion.h" #include "DXILLegalizePass.h" #include "DXILOpLowering.h" +#include "DXILPostOptimizationValidation.h" #include "DXILPrettyPrinter.h" #include "DXILResourceAccess.h" #include "DXILRootSignature.h" @@ -63,6 +64,7 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeDirectXTarget() { initializeDXILOpLoweringLegacyPass(*PR); initializeDXILResourceAccessLegacyPass(*PR); initializeDXILTranslateMetadataLegacyPass(*PR); + initializeDXILPostOptimizationValidationLegacyPass(*PR); initializeShaderFlagsAnalysisWrapperPass(*PR); initializeRootSignatureAnalysisWrapperPass(*PR); initializeDXILFinalizeLinkageLegacyPass(*PR); @@ -110,6 +112,7 @@ class DirectXPassConfig : public TargetPassConfig { addPass(createDXILForwardHandleAccessesLegacyPass()); addPass(createDXILLegalizeLegacyPass()); addPass(createDXILTranslateMetadataLegacyPass()); + 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 a2412b6324a055..705e05ced9aae1 100644 --- a/llvm/test/CodeGen/DirectX/llc-pipeline.ll +++ b/llvm/test/CodeGen/DirectX/llc-pipeline.ll @@ -28,6 +28,7 @@ ; CHECK-NEXT: DXIL Module Metadata analysis ; CHECK-NEXT: DXIL Shader Flag Analysis ; CHECK-NEXT: DXIL Translate Metadata +; CHECK-NEXT: DXIL Post Optimization Validation ; CHECK-NEXT: DXIL Op Lowering ; CHECK-NEXT: DXIL Prepare Module diff --git a/llvm/test/CodeGen/DirectX/resource_counter_error.ll b/llvm/test/CodeGen/DirectX/resource_counter_error.ll new file mode 100644 index 00000000000000..1fc0332c605526 --- /dev/null +++ b/llvm/test/CodeGen/DirectX/resource_counter_error.ll @@ -0,0 +1,10 @@ +; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.3-library %s 2>&1 | FileCheck %s +; CHECK: RWStructuredBuffers may increment or decrement their counters, but not both. + +define void @inc_and_dec() { +entry: + %handle = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.resource.handlefrombinding(i32 1, i32 2, i32 3, i32 4, i1 false) + call i32 @llvm.dx.resource.updatecounter(target("dx.RawBuffer", float, 1, 0) %handle, i8 -1) + call i32 @llvm.dx.resource.updatecounter(target("dx.RawBuffer", float, 1, 0) %handle, i8 1) + ret void +} From 4dfa76dbedd61fd1f82b81f6c4a921c5f7a00fb1 Mon Sep 17 00:00:00 2001 From: Ashley Coleman Date: Wed, 30 Apr 2025 13:52:46 -0600 Subject: [PATCH 2/2] Address Comments --- llvm/include/llvm/Analysis/DXILResource.h | 3 + llvm/lib/Analysis/DXILResource.cpp | 4 +- .../DXILPostOptimizationValidation.cpp | 67 +++++++------------ .../DirectX/DXILPostOptimizationValidation.h | 3 +- 4 files changed, 33 insertions(+), 44 deletions(-) diff --git a/llvm/include/llvm/Analysis/DXILResource.h b/llvm/include/llvm/Analysis/DXILResource.h index 102b184af00ba9..b02529c88b775d 100644 --- a/llvm/include/llvm/Analysis/DXILResource.h +++ b/llvm/include/llvm/Analysis/DXILResource.h @@ -458,6 +458,7 @@ class DXILResourceMap { unsigned FirstUAV = 0; unsigned FirstCBuffer = 0; unsigned FirstSampler = 0; + bool HasInvalidDirection = false; /// Populate all the resource instance data. void populate(Module &M, DXILResourceTypeMap &DRTM); @@ -549,6 +550,8 @@ class DXILResourceMap { return make_range(call_begin(), call_end()); } + bool hasInvalidCounterDirection() const { return HasInvalidDirection; } + void print(raw_ostream &OS, DXILResourceTypeMap &DRTM, const DataLayout &DL) const; diff --git a/llvm/lib/Analysis/DXILResource.cpp b/llvm/lib/Analysis/DXILResource.cpp index 96e1b44a17f305..fc74175070e349 100644 --- a/llvm/lib/Analysis/DXILResource.cpp +++ b/llvm/lib/Analysis/DXILResource.cpp @@ -811,8 +811,10 @@ void DXILResourceMap::populateCounterDirections(Module &M) { for (ResourceInfo *RBInfo : RBInfos) { if (RBInfo->CounterDirection == ResourceCounterDirection::Unknown) RBInfo->CounterDirection = Direction; - else if (RBInfo->CounterDirection != Direction) + else if (RBInfo->CounterDirection != Direction) { RBInfo->CounterDirection = ResourceCounterDirection::Invalid; + HasInvalidDirection = true; + } } } } diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp index 19345f1a469eeb..1dc0c2fb13c11e 100644 --- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp +++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp @@ -7,28 +7,15 @@ //===----------------------------------------------------------------------===// #include "DXILPostOptimizationValidation.h" -#include "DXILConstants.h" -#include "DXILIntrinsicExpansion.h" -#include "DXILOpBuilder.h" #include "DXILShaderFlags.h" #include "DirectX.h" -#include "llvm/ADT/SmallVector.h" #include "llvm/Analysis/DXILMetadataAnalysis.h" #include "llvm/Analysis/DXILResource.h" -#include "llvm/CodeGen/Passes.h" #include "llvm/IR/DiagnosticInfo.h" -#include "llvm/IR/Function.h" -#include "llvm/IR/IRBuilder.h" -#include "llvm/IR/Instruction.h" #include "llvm/IR/Instructions.h" -#include "llvm/IR/Intrinsics.h" #include "llvm/IR/IntrinsicsDirectX.h" #include "llvm/IR/Module.h" -#include "llvm/IR/PassManager.h" #include "llvm/InitializePasses.h" -#include "llvm/Pass.h" -#include "llvm/Support/ErrorHandling.h" -#include #define DEBUG_TYPE "dxil-post-optimization-validation" @@ -36,47 +23,42 @@ using namespace llvm; using namespace llvm::dxil; namespace { -class DXILValidator { - Module &M; - DXILResourceMap &DRM; -public: - DXILValidator(Module &M, DXILResourceMap &DRM) : M(M), DRM(DRM) {} - - void validate() { - for (const auto &UAV : DRM.uavs()) { - if (UAV.CounterDirection != ResourceCounterDirection::Invalid) - continue; +static void reportInvalidDirection(Module &M, DXILResourceMap &DRM) { + for (const auto &UAV : DRM.uavs()) { + if (UAV.CounterDirection != ResourceCounterDirection::Invalid) + continue; - CallInst *ResourceHandle = nullptr; - for (CallInst *MaybeHandle : DRM.calls()) { - if (*DRM.find(MaybeHandle) == UAV) { - ResourceHandle = MaybeHandle; - break; - } + CallInst *ResourceHandle = nullptr; + for (CallInst *MaybeHandle : DRM.calls()) { + if (*DRM.find(MaybeHandle) == UAV) { + ResourceHandle = MaybeHandle; + break; } + } - StringRef Message = - "RWStructuredBuffers may increment or decrement their " - "counters, but not both."; - for (const auto &U : ResourceHandle->users()) { - const CallInst *CI = dyn_cast(U); - if (!CI && CI->getIntrinsicID() != Intrinsic::dx_resource_updatecounter) - continue; + StringRef Message = "RWStructuredBuffers may increment or decrement their " + "counters, but not both."; + for (const auto &U : ResourceHandle->users()) { + const CallInst *CI = dyn_cast(U); + if (!CI && CI->getIntrinsicID() != Intrinsic::dx_resource_updatecounter) + continue; - M.getContext().diagnose(DiagnosticInfoGenericWithLoc( - Message, *CI->getFunction(), CI->getDebugLoc())); - } + M.getContext().diagnose(DiagnosticInfoGenericWithLoc( + Message, *CI->getFunction(), CI->getDebugLoc())); } } -}; +} + } // namespace PreservedAnalyses DXILPostOptimizationValidation::run(Module &M, ModuleAnalysisManager &MAM) { DXILResourceMap &DRM = MAM.getResult(M); - DXILValidator(M, DRM).validate(); + if (DRM.hasInvalidCounterDirection()) + reportInvalidDirection(M, DRM); + return PreservedAnalyses::all(); } @@ -87,7 +69,8 @@ class DXILPostOptimizationValidationLegacy : public ModulePass { DXILResourceMap &DRM = getAnalysis().getResourceMap(); - DXILValidator(M, DRM).validate(); + if (DRM.hasInvalidCounterDirection()) + reportInvalidDirection(M, DRM); return false; } diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h index 5a7f7454c8390b..cb5e6245142723 100644 --- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h +++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h @@ -6,7 +6,8 @@ // //===----------------------------------------------------------------------===// // -// \file Pass for validating DXIL after lowering and optimizations are applied. +// \file Pass for validating IR after optimizations are applied and before +// lowering to DXIL. // //===----------------------------------------------------------------------===//