Skip to content

[HLSL] Move DXILResourceImplicitBinding pass closer to DXIL Resource Analysis #140981

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 29, 2025

Conversation

hekota
Copy link
Member

@hekota hekota commented May 22, 2025

Moving DXILResourceImplicitBinding pass and the associated DXILResourceBindingAnalysis lower in the llc pipeline to just before the DXIL Resource Analysis, which is where its results are first needed, and adjusting the set of analyses it preserves.

The reason for this change is that I will soon be adding DXILResourceBindingAnalysis dependency to DXILPostOptimizationValidation pass and bringing this closer to where it is needed avoid unnecessary churn to preserved analysis setting in preceding passes.

…urce Analysis

Moving DXILResourceImplicitBinding pass and the associated DXILResourceBindingAnalysis lower in the llc pipeline to just before DXIL Resource Analysis, which is where its results are first needed.

The reason for this change is that I will soon be adding DXILResourceBindingAnalysis dependency to DXILPostOptimizationValidation pass and bringing this closer to where it is needed avoid unnecessary churn to preserved analysis setting in preceding passes.
@llvmbot
Copy link
Member

llvmbot commented May 22, 2025

@llvm/pr-subscribers-backend-directx

Author: Helena Kotas (hekota)

Changes

Moving DXILResourceImplicitBinding pass and the associated DXILResourceBindingAnalysis lower in the llc pipeline to just before the DXIL Resource Analysis, which is where its results are first needed.

The reason for this change is that I will soon be adding DXILResourceBindingAnalysis dependency to DXILPostOptimizationValidation pass and bringing this closer to where it is needed avoid unnecessary churn to preserved analysis setting in preceding passes.


Full diff: https://github.com/llvm/llvm-project/pull/140981.diff

4 Files Affected:

  • (modified) llvm/lib/Target/DirectX/DXILResourceImplicitBinding.cpp (+10-3)
  • (modified) llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp (+1)
  • (modified) llvm/lib/Target/DirectX/DirectXTargetMachine.cpp (+1-1)
  • (modified) llvm/test/CodeGen/DirectX/llc-pipeline.ll (+2-2)
diff --git a/llvm/lib/Target/DirectX/DXILResourceImplicitBinding.cpp b/llvm/lib/Target/DirectX/DXILResourceImplicitBinding.cpp
index 92454bea51e31..8caa3b37698fc 100644
--- a/llvm/lib/Target/DirectX/DXILResourceImplicitBinding.cpp
+++ b/llvm/lib/Target/DirectX/DXILResourceImplicitBinding.cpp
@@ -135,9 +135,14 @@ PreservedAnalyses DXILResourceImplicitBinding::run(Module &M,
 
   DXILResourceBindingInfo &DRBI = AM.getResult<DXILResourceBindingAnalysis>(M);
   DXILResourceTypeMap &DRTM = AM.getResult<DXILResourceTypeAnalysis>(M);
-  if (DRBI.hasImplicitBinding())
-    if (assignBindings(M, DRBI, DRTM))
-      return PreservedAnalyses::none();
+  if (DRBI.hasImplicitBinding()) {
+    if (assignBindings(M, DRBI, DRTM)) {
+      PreservedAnalyses PA;
+      PA.preserve<DXILResourceBindingAnalysis>();
+      PA.preserve<DXILResourceTypeAnalysis>();
+      return PA;
+    }
+  }
   return PreservedAnalyses::all();
 }
 
@@ -162,6 +167,8 @@ class DXILResourceImplicitBindingLegacy : public ModulePass {
   void getAnalysisUsage(llvm::AnalysisUsage &AU) const override {
     AU.addRequired<DXILResourceTypeWrapperPass>();
     AU.addRequired<DXILResourceBindingWrapperPass>();
+    AU.addPreserved<DXILResourceTypeWrapperPass>();
+    AU.addPreserved<DXILResourceBindingWrapperPass>();
   }
 };
 
diff --git a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
index e0dce3d750ed1..1d496bf0a5656 100644
--- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
+++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
@@ -408,6 +408,7 @@ class DXILTranslateMetadataLegacy : public ModulePass {
     AU.addPreserved<DXILResourceWrapperPass>();
     AU.addPreserved<DXILMetadataAnalysisWrapperPass>();
     AU.addPreserved<ShaderFlagsAnalysisWrapper>();
+    AU.addPreserved<DXILResourceBindingWrapperPass>();
   }
 
   bool runOnModule(Module &M) override {
diff --git a/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp b/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp
index 22142484cef3c..40fe6c6e639e4 100644
--- a/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp
+++ b/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp
@@ -103,7 +103,6 @@ class DirectXPassConfig : public TargetPassConfig {
   FunctionPass *createTargetRegisterAllocator(bool) override { return nullptr; }
   void addCodeGenPrepare() override {
     addPass(createDXILFinalizeLinkageLegacyPass());
-    addPass(createDXILResourceImplicitBindingLegacyPass());
     addPass(createDXILResourceAccessLegacyPass());
     addPass(createDXILIntrinsicExpansionLegacyPass());
     addPass(createDXILCBufferAccessLegacyPass());
@@ -114,6 +113,7 @@ class DirectXPassConfig : public TargetPassConfig {
     addPass(createScalarizerPass(DxilScalarOptions));
     addPass(createDXILForwardHandleAccessesLegacyPass());
     addPass(createDXILLegalizeLegacyPass());
+    addPass(createDXILResourceImplicitBindingLegacyPass());
     addPass(createDXILTranslateMetadataLegacyPass());
     addPass(createDXILPostOptimizationValidationLegacyPass());
     addPass(createDXILOpLoweringLegacyPass());
diff --git a/llvm/test/CodeGen/DirectX/llc-pipeline.ll b/llvm/test/CodeGen/DirectX/llc-pipeline.ll
index 088040a491bdc..2b29fd30a7a56 100644
--- a/llvm/test/CodeGen/DirectX/llc-pipeline.ll
+++ b/llvm/test/CodeGen/DirectX/llc-pipeline.ll
@@ -14,8 +14,6 @@
 
 ; CHECK-NEXT: ModulePass Manager
 ; CHECK-NEXT:   DXIL Finalize Linkage
-; CHECK-NEXT:   DXIL Resource Binding Analysis
-; CHECK-NEXT:   DXIL Resource Implicit Binding
 ; CHECK-NEXT:   FunctionPass Manager
 ; CHECK-NEXT:     DXIL Resource Access
 ; CHECK-NEXT:   DXIL Intrinsic Expansion
@@ -27,6 +25,8 @@
 ; CHECK-NEXT:     Scalarize vector operations
 ; CHECK-NEXT:     DXIL Forward Handle Accesses
 ; CHECK-NEXT:     DXIL Legalizer
+; CHECK-NEXT:   DXIL Resource Binding Analysis
+; CHECK-NEXT:   DXIL Resource Implicit Binding
 ; CHECK-NEXT:   DXIL Resources Analysis
 ; CHECK-NEXT:   DXIL Module Metadata analysis
 ; CHECK-NEXT:   DXIL Shader Flag Analysis

@hekota hekota changed the title [HLSL] Moving DXILResourceImplicitBinding pass closer to DXIL Resource Analysis [HLSL] Move DXILResourceImplicitBinding pass closer to DXIL Resource Analysis May 22, 2025
@hekota hekota linked an issue May 27, 2025 that may be closed by this pull request
@hekota hekota merged commit 74ad4ba into llvm:main May 29, 2025
12 checks passed
svkeerthy pushed a commit that referenced this pull request May 29, 2025
…Analysis (#140981)

Moving `DXILResourceImplicitBinding` pass and the associated `DXILResourceBindingAnalysis` lower in the llc pipeline to just before the DXIL Resource Analysis, which is where its results are first needed, and adjusting the set of analyses it preserves.

The reason for this change is that I will soon be adding `DXILResourceBindingAnalysis` dependency to `DXILPostOptimizationValidation` pass and bringing this closer to where it is needed avoid unnecessary churn to preserved analysis setting in preceding passes.
google-yfyang pushed a commit to google-yfyang/llvm-project that referenced this pull request May 29, 2025
…Analysis (llvm#140981)

Moving `DXILResourceImplicitBinding` pass and the associated `DXILResourceBindingAnalysis` lower in the llc pipeline to just before the DXIL Resource Analysis, which is where its results are first needed, and adjusting the set of analyses it preserves.

The reason for this change is that I will soon be adding `DXILResourceBindingAnalysis` dependency to `DXILPostOptimizationValidation` pass and bringing this closer to where it is needed avoid unnecessary churn to preserved analysis setting in preceding passes.
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
…Analysis (llvm#140981)

Moving `DXILResourceImplicitBinding` pass and the associated `DXILResourceBindingAnalysis` lower in the llc pipeline to just before the DXIL Resource Analysis, which is where its results are first needed, and adjusting the set of analyses it preserves.

The reason for this change is that I will soon be adding `DXILResourceBindingAnalysis` dependency to `DXILPostOptimizationValidation` pass and bringing this closer to where it is needed avoid unnecessary churn to preserved analysis setting in preceding passes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[HLSL] Diagnose overlapping resource bindings
4 participants