Skip to content

[DirectX] Add Resource uses to Resource Handle map in DXILResourceMap #112798

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

Closed

Conversation

lizhengxing
Copy link
Contributor

When lowering some resource use intrisics to DXIL operations, it needs to know the information of the resource that the intrisics are using.

This PR adds Resource uses to Resource Handle map in DXILResourceMap. It helps the resource uses to find the resource information.

This PR is also useful to #106188

@llvmbot llvmbot added backend:DirectX llvm:analysis Includes value tracking, cost tables and constant folding labels Oct 17, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 17, 2024

@llvm/pr-subscribers-llvm-analysis

Author: Zhengxing li (lizhengxing)

Changes

When lowering some resource use intrisics to DXIL operations, it needs to know the information of the resource that the intrisics are using.

This PR adds Resource uses to Resource Handle map in DXILResourceMap. It helps the resource uses to find the resource information.

This PR is also useful to #106188


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

4 Files Affected:

  • (modified) llvm/include/llvm/Analysis/DXILResource.h (+25)
  • (modified) llvm/lib/Analysis/DXILResource.cpp (+31)
  • (modified) llvm/lib/Target/DirectX/DXILOpLowering.cpp (+9)
  • (added) llvm/test/Analysis/DXILResource/dx-resource-map.ll (+48)
diff --git a/llvm/include/llvm/Analysis/DXILResource.h b/llvm/include/llvm/Analysis/DXILResource.h
index 6b577c02f05450..abd0e8e918aa05 100644
--- a/llvm/include/llvm/Analysis/DXILResource.h
+++ b/llvm/include/llvm/Analysis/DXILResource.h
@@ -264,6 +264,8 @@ class ResourceInfo {
 class DXILResourceMap {
   SmallVector<dxil::ResourceInfo> Resources;
   DenseMap<CallInst *, unsigned> CallMap;
+  // Mapping from Resource use to Resource Handle
+  DenseMap<CallInst *, CallInst *> ResUseToHandleMap;
   unsigned FirstUAV = 0;
   unsigned FirstCBuffer = 0;
   unsigned FirstSampler = 0;
@@ -335,6 +337,29 @@ class DXILResourceMap {
   }
 
   void print(raw_ostream &OS) const;
+
+  void updateResourceMap(CallInst *origCallInst, CallInst *newCallInst);
+
+  void updateResUseMap(CallInst* origResUse, CallInst* newResUse) {
+    assert((origResUse != nullptr) && (newResUse != nullptr) &&
+           (origResUse != newResUse) && "Wrong Inputs");
+
+    updateResUseMapCommon(origResUse, newResUse, /*keepOrigResUseInMap=*/false);
+  }
+
+  CallInst *findResHandleByUse(CallInst *resUse) {
+    auto Pos = ResUseToHandleMap.find(resUse);
+    assert((Pos != ResUseToHandleMap.end()) && "Can't find the resource handle");
+
+    return Pos->second;
+  }
+
+private:
+  void updateResUseMapCommon(CallInst *origResUse, CallInst *newResUse, bool keepOrigResUseInMap) {
+    ResUseToHandleMap.try_emplace(newResUse, findResHandleByUse(origResUse));
+    if (!keepOrigResUseInMap)
+      ResUseToHandleMap.erase(origResUse);
+  }
 };
 
 class DXILResourceAnalysis : public AnalysisInfoMixin<DXILResourceAnalysis> {
diff --git a/llvm/lib/Analysis/DXILResource.cpp b/llvm/lib/Analysis/DXILResource.cpp
index 2802480481690d..801cb0aa9876d5 100644
--- a/llvm/lib/Analysis/DXILResource.cpp
+++ b/llvm/lib/Analysis/DXILResource.cpp
@@ -719,6 +719,12 @@ DXILResourceMap::DXILResourceMap(
     if (Resources.empty() || RI != Resources.back())
       Resources.push_back(RI);
     CallMap[CI] = Resources.size() - 1;
+
+    // Build ResUseToHandleMap
+    for (auto it = CI->users().begin(); it != CI->users().end(); ++it) {
+      CallInst *CI_Use = dyn_cast<CallInst>(*it);
+      ResUseToHandleMap[CI_Use] = CI;
+    }
   }
 
   unsigned Size = Resources.size();
@@ -744,6 +750,23 @@ DXILResourceMap::DXILResourceMap(
   }
 }
 
+void DXILResourceMap::updateResourceMap(CallInst *origCallInst,
+                                        CallInst *newCallInst) {
+  assert((origCallInst != nullptr) && (newCallInst != nullptr) &&
+         (origCallInst != newCallInst));
+
+  CallMap.try_emplace(newCallInst, CallMap[origCallInst]);
+  CallMap.erase(origCallInst);
+
+  // Update ResUseToHandleMap since Resource Handle changed
+  for (auto it = origCallInst->users().begin();
+       it != origCallInst->users().end();
+       ++it) {
+    CallInst *CI_Use = dyn_cast<CallInst>(*it);
+    ResUseToHandleMap[CI_Use] = newCallInst;
+  }
+}
+
 void DXILResourceMap::print(raw_ostream &OS) const {
   for (unsigned I = 0, E = Resources.size(); I != E; ++I) {
     OS << "Binding " << I << ":\n";
@@ -756,6 +779,14 @@ void DXILResourceMap::print(raw_ostream &OS) const {
     CI->print(OS);
     OS << "\n";
   }
+
+  for (const auto &[ResUse, ResHandle] : ResUseToHandleMap) {
+    OS << "\n";
+    OS << "Resource " << CallMap.find(ResHandle)->second;
+    OS << " is used by ";
+    ResUse->print(OS);
+    OS << "\n";
+  }
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/llvm/lib/Target/DirectX/DXILOpLowering.cpp b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
index 99df4850872078..8e800da9968303 100644
--- a/llvm/lib/Target/DirectX/DXILOpLowering.cpp
+++ b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
@@ -204,6 +204,8 @@ class OpLowerer {
 
       Value *Cast = createTmpHandleCast(*OpCall, CI->getType());
 
+      DRM.updateResourceMap(CI, *OpCall);
+
       CI->replaceAllUsesWith(Cast);
       CI->eraseFromParent();
       return Error::success();
@@ -248,6 +250,8 @@ class OpLowerer {
 
       Value *Cast = createTmpHandleCast(*OpAnnotate, CI->getType());
 
+      DRM.updateResourceMap(CI, *OpBind);
+
       CI->replaceAllUsesWith(Cast);
       CI->eraseFromParent();
 
@@ -412,6 +416,9 @@ class OpLowerer {
           OpCode::BufferLoad, Args, CI->getName(), NewRetTy);
       if (Error E = OpCall.takeError())
         return E;
+
+      DRM.updateResUseMap(CI, *OpCall);
+
       if (Error E = replaceResRetUses(CI, *OpCall, HasCheckBit))
         return E;
 
@@ -456,6 +463,8 @@ class OpLowerer {
       if (Error E = OpCall.takeError())
         return E;
 
+      DRM.updateResUseMap(CI, *OpCall);
+
       CI->eraseFromParent();
       return Error::success();
     });
diff --git a/llvm/test/Analysis/DXILResource/dx-resource-map.ll b/llvm/test/Analysis/DXILResource/dx-resource-map.ll
new file mode 100644
index 00000000000000..5f196e3ba7e825
--- /dev/null
+++ b/llvm/test/Analysis/DXILResource/dx-resource-map.ll
@@ -0,0 +1,48 @@
+; RUN: opt -S -disable-output -disable-output -passes="print<dxil-resource>,dxil-op-lower,print<dxil-resource>" -mtriple=dxil-pc-shadermodel6.6-compute < %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK_SM66
+; RUN: opt -S -disable-output -disable-output -passes="print<dxil-resource>,dxil-op-lower,print<dxil-resource>" -mtriple=dxil-pc-shadermodel6.2-compute < %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK_SM62
+
+define void @test_typedbuffer() {
+  ; RWBuffer<float4> Buf : register(u5, space3)
+  %uav1 = call target("dx.TypedBuffer", <4 x float>, 1, 0, 0)
+              @llvm.dx.handle.fromBinding.tdx.TypedBuffer_f32_1_0(
+                  i32 3, i32 5, i32 1, i32 0, i1 false)
+  ; CHECK: Binding [[UAV1:[0-9]+]]:
+  ; CHECK:   Symbol: ptr undef
+  ; CHECK:   Name: ""
+  ; CHECK:   Binding:
+  ; CHECK:     Record ID: 0
+  ; CHECK:     Space: 3
+  ; CHECK:     Lower Bound: 5
+  ; CHECK:     Size: 1
+  ; CHECK:   Class: UAV
+  ; CHECK:   Kind: TypedBuffer
+  ; CHECK:   Globally Coherent: 0
+  ; CHECK:   HasCounter: 0
+  ; CHECK:   IsROV: 0
+  ; CHECK:   Element Type: f32
+  ; CHECK:   Element Count: 4
+
+  ; CHECK:     Call bound to [[UAV1]]:  %uav1 = call target("dx.TypedBuffer", <4 x float>, 1, 0, 0) @llvm.dx.handle.fromBinding.tdx.TypedBuffer_v4f32_1_0_0t(i32 3, i32 5, i32 1, i32 0, i1 false)
+  ; CHECK-DAG: Resource [[UAV1]] is used by   %data0 = call <4 x float> @llvm.dx.typedBufferLoad.v4f32.tdx.TypedBuffer_v4f32_1_0_0t(target("dx.TypedBuffer", <4 x float>, 1, 0, 0) %uav1, i32 0)
+  ; CHECK-DAG: Resource [[UAV1]] is used by   call void @llvm.dx.typedBufferStore.tdx.TypedBuffer_v4f32_1_0_0t.v4f32(target("dx.TypedBuffer", <4 x float>, 1, 0, 0) %uav1, i32 2, <4 x float> %data0)
+
+  %data0 = call <4 x float> @llvm.dx.typedBufferLoad(
+      target("dx.TypedBuffer", <4 x float>, 1, 0, 0) %uav1, i32 0)
+  call void @llvm.dx.typedBufferStore(
+      target("dx.TypedBuffer", <4 x float>, 1, 0, 0) %uav1,
+      i32 2, <4 x float> %data0)
+
+  ;
+  ;;; After dxil-op-lower, the DXILResourceMap info should be updated.
+  ;
+  ; CHECK_SM66:     Call bound to [[UAV1]]:  %uav11 = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 218, %dx.types.ResBind { i32 5, i32 5, i32 3, i8 1 }, i32 0, i1 false)
+  ; CHECK_SM66-DAG: Resource [[UAV1]] is used by   %data02 = call %dx.types.ResRet.f32 @dx.op.bufferLoad.f32(i32 68, %dx.types.Handle %uav1_annot, i32 0, i32 undef)
+  ; CHECK_SM66-DAG: Resource [[UAV1]] is used by   call void @dx.op.bufferStore.f32(i32 69, %dx.types.Handle %uav1_annot, i32 2, i32 undef, float %9, float %10, float %11, float %12, i8 15)
+  ;
+  ; CHECK_SM62:     Call bound to [[UAV1]]:  %uav11 = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 0, i32 0, i1 false)
+  ; CHECK_SM62-DAG: Resource [[UAV1]] is used by   %data02 = call %dx.types.ResRet.f32 @dx.op.bufferLoad.f32(i32 68, %dx.types.Handle %uav11, i32 0, i32 undef)
+  ; CHECK_SM62-DAG: Resource [[UAV1]] is used by   call void @dx.op.bufferStore.f32(i32 69, %dx.types.Handle %uav11, i32 2, i32 undef, float %9, float %10, float %11, float %12, i8 15)
+
+  ret void
+}
+

@llvmbot
Copy link
Member

llvmbot commented Oct 17, 2024

@llvm/pr-subscribers-backend-directx

Author: Zhengxing li (lizhengxing)

Changes

When lowering some resource use intrisics to DXIL operations, it needs to know the information of the resource that the intrisics are using.

This PR adds Resource uses to Resource Handle map in DXILResourceMap. It helps the resource uses to find the resource information.

This PR is also useful to #106188


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

4 Files Affected:

  • (modified) llvm/include/llvm/Analysis/DXILResource.h (+25)
  • (modified) llvm/lib/Analysis/DXILResource.cpp (+31)
  • (modified) llvm/lib/Target/DirectX/DXILOpLowering.cpp (+9)
  • (added) llvm/test/Analysis/DXILResource/dx-resource-map.ll (+48)
diff --git a/llvm/include/llvm/Analysis/DXILResource.h b/llvm/include/llvm/Analysis/DXILResource.h
index 6b577c02f05450..abd0e8e918aa05 100644
--- a/llvm/include/llvm/Analysis/DXILResource.h
+++ b/llvm/include/llvm/Analysis/DXILResource.h
@@ -264,6 +264,8 @@ class ResourceInfo {
 class DXILResourceMap {
   SmallVector<dxil::ResourceInfo> Resources;
   DenseMap<CallInst *, unsigned> CallMap;
+  // Mapping from Resource use to Resource Handle
+  DenseMap<CallInst *, CallInst *> ResUseToHandleMap;
   unsigned FirstUAV = 0;
   unsigned FirstCBuffer = 0;
   unsigned FirstSampler = 0;
@@ -335,6 +337,29 @@ class DXILResourceMap {
   }
 
   void print(raw_ostream &OS) const;
+
+  void updateResourceMap(CallInst *origCallInst, CallInst *newCallInst);
+
+  void updateResUseMap(CallInst* origResUse, CallInst* newResUse) {
+    assert((origResUse != nullptr) && (newResUse != nullptr) &&
+           (origResUse != newResUse) && "Wrong Inputs");
+
+    updateResUseMapCommon(origResUse, newResUse, /*keepOrigResUseInMap=*/false);
+  }
+
+  CallInst *findResHandleByUse(CallInst *resUse) {
+    auto Pos = ResUseToHandleMap.find(resUse);
+    assert((Pos != ResUseToHandleMap.end()) && "Can't find the resource handle");
+
+    return Pos->second;
+  }
+
+private:
+  void updateResUseMapCommon(CallInst *origResUse, CallInst *newResUse, bool keepOrigResUseInMap) {
+    ResUseToHandleMap.try_emplace(newResUse, findResHandleByUse(origResUse));
+    if (!keepOrigResUseInMap)
+      ResUseToHandleMap.erase(origResUse);
+  }
 };
 
 class DXILResourceAnalysis : public AnalysisInfoMixin<DXILResourceAnalysis> {
diff --git a/llvm/lib/Analysis/DXILResource.cpp b/llvm/lib/Analysis/DXILResource.cpp
index 2802480481690d..801cb0aa9876d5 100644
--- a/llvm/lib/Analysis/DXILResource.cpp
+++ b/llvm/lib/Analysis/DXILResource.cpp
@@ -719,6 +719,12 @@ DXILResourceMap::DXILResourceMap(
     if (Resources.empty() || RI != Resources.back())
       Resources.push_back(RI);
     CallMap[CI] = Resources.size() - 1;
+
+    // Build ResUseToHandleMap
+    for (auto it = CI->users().begin(); it != CI->users().end(); ++it) {
+      CallInst *CI_Use = dyn_cast<CallInst>(*it);
+      ResUseToHandleMap[CI_Use] = CI;
+    }
   }
 
   unsigned Size = Resources.size();
@@ -744,6 +750,23 @@ DXILResourceMap::DXILResourceMap(
   }
 }
 
+void DXILResourceMap::updateResourceMap(CallInst *origCallInst,
+                                        CallInst *newCallInst) {
+  assert((origCallInst != nullptr) && (newCallInst != nullptr) &&
+         (origCallInst != newCallInst));
+
+  CallMap.try_emplace(newCallInst, CallMap[origCallInst]);
+  CallMap.erase(origCallInst);
+
+  // Update ResUseToHandleMap since Resource Handle changed
+  for (auto it = origCallInst->users().begin();
+       it != origCallInst->users().end();
+       ++it) {
+    CallInst *CI_Use = dyn_cast<CallInst>(*it);
+    ResUseToHandleMap[CI_Use] = newCallInst;
+  }
+}
+
 void DXILResourceMap::print(raw_ostream &OS) const {
   for (unsigned I = 0, E = Resources.size(); I != E; ++I) {
     OS << "Binding " << I << ":\n";
@@ -756,6 +779,14 @@ void DXILResourceMap::print(raw_ostream &OS) const {
     CI->print(OS);
     OS << "\n";
   }
+
+  for (const auto &[ResUse, ResHandle] : ResUseToHandleMap) {
+    OS << "\n";
+    OS << "Resource " << CallMap.find(ResHandle)->second;
+    OS << " is used by ";
+    ResUse->print(OS);
+    OS << "\n";
+  }
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/llvm/lib/Target/DirectX/DXILOpLowering.cpp b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
index 99df4850872078..8e800da9968303 100644
--- a/llvm/lib/Target/DirectX/DXILOpLowering.cpp
+++ b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
@@ -204,6 +204,8 @@ class OpLowerer {
 
       Value *Cast = createTmpHandleCast(*OpCall, CI->getType());
 
+      DRM.updateResourceMap(CI, *OpCall);
+
       CI->replaceAllUsesWith(Cast);
       CI->eraseFromParent();
       return Error::success();
@@ -248,6 +250,8 @@ class OpLowerer {
 
       Value *Cast = createTmpHandleCast(*OpAnnotate, CI->getType());
 
+      DRM.updateResourceMap(CI, *OpBind);
+
       CI->replaceAllUsesWith(Cast);
       CI->eraseFromParent();
 
@@ -412,6 +416,9 @@ class OpLowerer {
           OpCode::BufferLoad, Args, CI->getName(), NewRetTy);
       if (Error E = OpCall.takeError())
         return E;
+
+      DRM.updateResUseMap(CI, *OpCall);
+
       if (Error E = replaceResRetUses(CI, *OpCall, HasCheckBit))
         return E;
 
@@ -456,6 +463,8 @@ class OpLowerer {
       if (Error E = OpCall.takeError())
         return E;
 
+      DRM.updateResUseMap(CI, *OpCall);
+
       CI->eraseFromParent();
       return Error::success();
     });
diff --git a/llvm/test/Analysis/DXILResource/dx-resource-map.ll b/llvm/test/Analysis/DXILResource/dx-resource-map.ll
new file mode 100644
index 00000000000000..5f196e3ba7e825
--- /dev/null
+++ b/llvm/test/Analysis/DXILResource/dx-resource-map.ll
@@ -0,0 +1,48 @@
+; RUN: opt -S -disable-output -disable-output -passes="print<dxil-resource>,dxil-op-lower,print<dxil-resource>" -mtriple=dxil-pc-shadermodel6.6-compute < %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK_SM66
+; RUN: opt -S -disable-output -disable-output -passes="print<dxil-resource>,dxil-op-lower,print<dxil-resource>" -mtriple=dxil-pc-shadermodel6.2-compute < %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK_SM62
+
+define void @test_typedbuffer() {
+  ; RWBuffer<float4> Buf : register(u5, space3)
+  %uav1 = call target("dx.TypedBuffer", <4 x float>, 1, 0, 0)
+              @llvm.dx.handle.fromBinding.tdx.TypedBuffer_f32_1_0(
+                  i32 3, i32 5, i32 1, i32 0, i1 false)
+  ; CHECK: Binding [[UAV1:[0-9]+]]:
+  ; CHECK:   Symbol: ptr undef
+  ; CHECK:   Name: ""
+  ; CHECK:   Binding:
+  ; CHECK:     Record ID: 0
+  ; CHECK:     Space: 3
+  ; CHECK:     Lower Bound: 5
+  ; CHECK:     Size: 1
+  ; CHECK:   Class: UAV
+  ; CHECK:   Kind: TypedBuffer
+  ; CHECK:   Globally Coherent: 0
+  ; CHECK:   HasCounter: 0
+  ; CHECK:   IsROV: 0
+  ; CHECK:   Element Type: f32
+  ; CHECK:   Element Count: 4
+
+  ; CHECK:     Call bound to [[UAV1]]:  %uav1 = call target("dx.TypedBuffer", <4 x float>, 1, 0, 0) @llvm.dx.handle.fromBinding.tdx.TypedBuffer_v4f32_1_0_0t(i32 3, i32 5, i32 1, i32 0, i1 false)
+  ; CHECK-DAG: Resource [[UAV1]] is used by   %data0 = call <4 x float> @llvm.dx.typedBufferLoad.v4f32.tdx.TypedBuffer_v4f32_1_0_0t(target("dx.TypedBuffer", <4 x float>, 1, 0, 0) %uav1, i32 0)
+  ; CHECK-DAG: Resource [[UAV1]] is used by   call void @llvm.dx.typedBufferStore.tdx.TypedBuffer_v4f32_1_0_0t.v4f32(target("dx.TypedBuffer", <4 x float>, 1, 0, 0) %uav1, i32 2, <4 x float> %data0)
+
+  %data0 = call <4 x float> @llvm.dx.typedBufferLoad(
+      target("dx.TypedBuffer", <4 x float>, 1, 0, 0) %uav1, i32 0)
+  call void @llvm.dx.typedBufferStore(
+      target("dx.TypedBuffer", <4 x float>, 1, 0, 0) %uav1,
+      i32 2, <4 x float> %data0)
+
+  ;
+  ;;; After dxil-op-lower, the DXILResourceMap info should be updated.
+  ;
+  ; CHECK_SM66:     Call bound to [[UAV1]]:  %uav11 = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 218, %dx.types.ResBind { i32 5, i32 5, i32 3, i8 1 }, i32 0, i1 false)
+  ; CHECK_SM66-DAG: Resource [[UAV1]] is used by   %data02 = call %dx.types.ResRet.f32 @dx.op.bufferLoad.f32(i32 68, %dx.types.Handle %uav1_annot, i32 0, i32 undef)
+  ; CHECK_SM66-DAG: Resource [[UAV1]] is used by   call void @dx.op.bufferStore.f32(i32 69, %dx.types.Handle %uav1_annot, i32 2, i32 undef, float %9, float %10, float %11, float %12, i8 15)
+  ;
+  ; CHECK_SM62:     Call bound to [[UAV1]]:  %uav11 = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 0, i32 0, i1 false)
+  ; CHECK_SM62-DAG: Resource [[UAV1]] is used by   %data02 = call %dx.types.ResRet.f32 @dx.op.bufferLoad.f32(i32 68, %dx.types.Handle %uav11, i32 0, i32 undef)
+  ; CHECK_SM62-DAG: Resource [[UAV1]] is used by   call void @dx.op.bufferStore.f32(i32 69, %dx.types.Handle %uav11, i32 2, i32 undef, float %9, float %10, float %11, float %12, i8 15)
+
+  ret void
+}
+

Copy link

github-actions bot commented Oct 17, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@lizhengxing lizhengxing force-pushed the directx-add-ResUes-to-Handle-map branch 2 times, most recently from e3152c2 to 5b3e612 Compare October 21, 2024 17:10
Copy link
Member

@hekota hekota left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment update, otherwise LGTM!

When lowering some resource use intrisics to DXIL operations, it needs to know the information of the resource that the intrisics are using.

This PR adds Resource uses to Resource Handle map in DXILResourceMap. It helps the resource uses to find the resource information.

This PR is also useful to llvm#106188
@lizhengxing lizhengxing force-pushed the directx-add-ResUes-to-Handle-map branch from 5b3e612 to 392daa1 Compare October 21, 2024 18:43
@bogner
Copy link
Contributor

bogner commented Oct 22, 2024

Do you have a draft PR of #106188 or some other example of how this will be used? It isn't entirely clear to me how a map of the uses makes the problem of finding the mapping easier.

@lizhengxing lizhengxing marked this pull request as draft November 19, 2024 01:09
@lizhengxing lizhengxing marked this pull request as ready for review November 19, 2024 02:43
@bogner
Copy link
Contributor

bogner commented Nov 29, 2024

This approach feels fragile to me. There are a couple of things that make it awkward:

  • It seems more generic than it is - we aren't actually interested in the uses of the handle creation call in general, we just want to look through dx.op.annotateHandle type calls
  • Needing to keep the list of uses up to date makes it too easy to accidentally invalidate the analysis

I think if we need a mapping from an arbitrary instance of a handle to the resource info, we can probably do this more directly by having a helper that walks through defs and specifically recognizes the annotateHandle op as a special case. However, this has a few of its own problems (knowing about dx.op here is a break in abstraction, we currently can have intermediate loads and stores, etc).

Stepping back from this for a second though, for the purpose of issue #106188 (and the PR in #116845) I guess we don't actually really need access to the binding information, we're mostly just interested in the resource type. This should be accessible from the handle itself regardless of source, so I think what we actually want to do here is break out some of the logic in ResourceMapper::mapBufferType so we can access information about a resource type from a TargetExtType directly, and then use that rather than the DXILResourceMap when we don't actually care about the binding info.

@@ -0,0 +1,48 @@
; RUN: opt -S -disable-output -disable-output -passes="print<dxil-resource>,dxil-op-lower,print<dxil-resource>" -mtriple=dxil-pc-shadermodel6.6-compute < %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK_SM66
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like you have -disable-output twice for no reason ?
This also is the case in the other .ll file.

@bogner
Copy link
Contributor

bogner commented Dec 19, 2024

I don't believe this is needed any more after #119773

@lizhengxing
Copy link
Contributor Author

As @bogner mentioned in the latest comment, this PR isn't needed anymore after #119773.

@lizhengxing lizhengxing closed this Jan 3, 2025
@lizhengxing lizhengxing deleted the directx-add-ResUes-to-Handle-map branch January 3, 2025 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:DirectX llvm:analysis Includes value tracking, cost tables and constant folding
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants