Skip to content

[DirectX] Implement the ForwardHandleAccesses pass #135378

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 1 commit into from
Apr 23, 2025

Conversation

bogner
Copy link
Contributor

@bogner bogner commented Apr 11, 2025

This pass attempts to forward resource handle creation to accesses of the handle global. This avoids dependence on optimizations like CSE and GlobalOpt for correctness of DXIL.

Fixes #134574.

@llvmbot
Copy link
Member

llvmbot commented Apr 11, 2025

@llvm/pr-subscribers-backend-directx

@llvm/pr-subscribers-llvm-analysis

Author: Justin Bogner (bogner)

Changes

This pass attempts to forward resource handle creation to accesses of the handle global. This avoids dependence on optimizations like CSE and GlobalOpt for correctness of DXIL.

Fixes #134574.


Patch is 20.80 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/135378.diff

13 Files Affected:

  • (modified) llvm/include/llvm/Analysis/DXILResource.h (+18)
  • (modified) llvm/lib/Target/DirectX/CMakeLists.txt (+1)
  • (added) llvm/lib/Target/DirectX/DXILForwardHandleAccesses.cpp (+165)
  • (added) llvm/lib/Target/DirectX/DXILForwardHandleAccesses.h (+28)
  • (modified) llvm/lib/Target/DirectX/DirectX.h (+6)
  • (modified) llvm/lib/Target/DirectX/DirectXPassRegistry.def (+1)
  • (modified) llvm/lib/Target/DirectX/DirectXTargetMachine.cpp (+3)
  • (added) llvm/test/CodeGen/DirectX/ForwardHandleAccesses/alloca.ll (+20)
  • (added) llvm/test/CodeGen/DirectX/ForwardHandleAccesses/ambiguous.ll (+21)
  • (added) llvm/test/CodeGen/DirectX/ForwardHandleAccesses/buffer-O0.ll (+44)
  • (added) llvm/test/CodeGen/DirectX/ForwardHandleAccesses/cbuffer-access.ll (+23)
  • (added) llvm/test/CodeGen/DirectX/ForwardHandleAccesses/undominated.ll (+16)
  • (modified) llvm/test/CodeGen/DirectX/llc-pipeline.ll (+2-1)
diff --git a/llvm/include/llvm/Analysis/DXILResource.h b/llvm/include/llvm/Analysis/DXILResource.h
index ff7961c9ad51c..7d37eb70e1ca9 100644
--- a/llvm/include/llvm/Analysis/DXILResource.h
+++ b/llvm/include/llvm/Analysis/DXILResource.h
@@ -196,6 +196,24 @@ class SamplerExtType : public TargetExtType {
   }
 };
 
+class AnyResourceExtType : public TargetExtType {
+public:
+  AnyResourceExtType() = delete;
+  AnyResourceExtType(const AnyResourceExtType &) = delete;
+  AnyResourceExtType &operator=(const AnyResourceExtType &) = delete;
+
+  static bool classof(const TargetExtType *T) {
+    return isa<RawBufferExtType>(T) || isa<TypedBufferExtType>(T) ||
+           isa<TextureExtType>(T) || isa<MSTextureExtType>(T) ||
+           isa<FeedbackTextureExtType>(T) || isa<CBufferExtType>(T) ||
+           isa<SamplerExtType>(T);
+  }
+
+  static bool classof(const Type *T) {
+    return isa<TargetExtType>(T) && classof(cast<TargetExtType>(T));
+  }
+};
+
 /// The dx.Layout target extension type
 ///
 /// `target("dx.Layout", <Type>, <size>, [offsets...])`
diff --git a/llvm/lib/Target/DirectX/CMakeLists.txt b/llvm/lib/Target/DirectX/CMakeLists.txt
index 13f8adbe4f132..823419fb830ae 100644
--- a/llvm/lib/Target/DirectX/CMakeLists.txt
+++ b/llvm/lib/Target/DirectX/CMakeLists.txt
@@ -22,6 +22,7 @@ add_llvm_target(DirectXCodeGen
   DXContainerGlobals.cpp
   DXILDataScalarization.cpp
   DXILFinalizeLinkage.cpp
+  DXILForwardHandleAccesses.cpp
   DXILFlattenArrays.cpp
   DXILIntrinsicExpansion.cpp
   DXILOpBuilder.cpp
diff --git a/llvm/lib/Target/DirectX/DXILForwardHandleAccesses.cpp b/llvm/lib/Target/DirectX/DXILForwardHandleAccesses.cpp
new file mode 100644
index 0000000000000..888ba6b00d9e8
--- /dev/null
+++ b/llvm/lib/Target/DirectX/DXILForwardHandleAccesses.cpp
@@ -0,0 +1,165 @@
+//===- DXILForwardHandleAccesses.cpp - Cleanup Handles --------------------===//
+//
+// 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 "DXILForwardHandleAccesses.h"
+#include "DXILShaderFlags.h"
+#include "DirectX.h"
+#include "llvm/Analysis/DXILResource.h"
+#include "llvm/Analysis/Loads.h"
+#include "llvm/IR/DiagnosticInfo.h"
+#include "llvm/IR/Dominators.h"
+#include "llvm/IR/IntrinsicInst.h"
+#include "llvm/IR/Intrinsics.h"
+#include "llvm/IR/IntrinsicsDirectX.h"
+#include "llvm/IR/Module.h"
+#include "llvm/InitializePasses.h"
+#include "llvm/Pass.h"
+#include "llvm/Transforms/Utils/Local.h"
+
+#define DEBUG_TYPE "dxil-forward-handle-accesses"
+
+using namespace llvm;
+
+static void diagnoseAmbiguousHandle(IntrinsicInst *NewII,
+                                    IntrinsicInst *PrevII) {
+  Function *F = NewII->getFunction();
+  LLVMContext &Context = F->getParent()->getContext();
+  Context.diagnose(DiagnosticInfoGeneric(
+      Twine("Handle at \"") + NewII->getName() + "\" overwrites handle at \"" +
+      PrevII->getName() + "\""));
+}
+
+static void diagnoseHandleNotFound(LoadInst *LI) {
+  Function *F = LI->getFunction();
+  LLVMContext &Context = F->getParent()->getContext();
+  Context.diagnose(DiagnosticInfoGeneric(
+      LI, Twine("Load of \"") + LI->getPointerOperand()->getName() +
+              "\" is not a global resource handle"));
+}
+
+static void diagnoseUndominatedLoad(LoadInst *LI, IntrinsicInst *Handle) {
+  Function *F = LI->getFunction();
+  LLVMContext &Context = F->getParent()->getContext();
+  Context.diagnose(DiagnosticInfoGeneric(
+      LI, Twine("Load at \"") + LI->getName() +
+              "\" is not dominated by handle creation at \"" +
+              Handle->getName() + "\""));
+}
+
+static void
+processHandle(IntrinsicInst *II,
+              DenseMap<GlobalVariable *, IntrinsicInst *> &HandleMap) {
+  for (User *U : II->users())
+    if (auto *SI = dyn_cast<StoreInst>(U))
+      if (auto *GV = dyn_cast<GlobalVariable>(SI->getPointerOperand())) {
+        auto Entry = HandleMap.try_emplace(GV, II);
+        if (Entry.second)
+          LLVM_DEBUG(dbgs() << "Added " << GV->getName() << " to handle map\n");
+        else
+          diagnoseAmbiguousHandle(II, Entry.first->second);
+      }
+}
+
+static bool forwardHandleAccesses(Function &F, DominatorTree &DT) {
+  bool Changed = false;
+
+  DenseMap<GlobalVariable *, IntrinsicInst *> HandleMap;
+  SmallVector<LoadInst *> LoadsToProcess;
+  for (BasicBlock &BB : F)
+    for (Instruction &Inst : BB)
+      if (auto *II = dyn_cast<IntrinsicInst>(&Inst)) {
+        switch (II->getIntrinsicID()) {
+        case Intrinsic::dx_resource_handlefrombinding:
+          processHandle(II, HandleMap);
+          break;
+        default:
+          continue;
+        }
+      } else if (auto *LI = dyn_cast<LoadInst>(&Inst))
+        if (isa<dxil::AnyResourceExtType>(LI->getType()))
+          LoadsToProcess.push_back(LI);
+
+  for (LoadInst *LI : LoadsToProcess) {
+    Value *V = LI->getPointerOperand();
+    auto *GV = dyn_cast<GlobalVariable>(LI->getPointerOperand());
+
+    // If we didn't find the global, we may need to walk through a level of
+    // indirection. This generally happens at -O0.
+    if (!GV)
+      if (auto *NestedLI = dyn_cast<LoadInst>(V)) {
+        BasicBlock::iterator BBI(NestedLI);
+        Value *Loaded = FindAvailableLoadedValue(
+            NestedLI, NestedLI->getParent(), BBI, 0, nullptr, nullptr);
+        GV = dyn_cast_or_null<GlobalVariable>(Loaded);
+      }
+
+    auto It = HandleMap.find(GV);
+    if (It == HandleMap.end()) {
+      diagnoseHandleNotFound(LI);
+      continue;
+    }
+    Changed = true;
+
+    if (!DT.dominates(It->second, LI)) {
+      diagnoseUndominatedLoad(LI, It->second);
+      continue;
+    }
+
+    LLVM_DEBUG(dbgs() << "Replacing uses of " << GV->getName() << " at "
+                      << LI->getName() << " with " << It->second->getName()
+                      << "\n");
+    LI->replaceAllUsesWith(It->second);
+    LI->eraseFromParent();
+  }
+
+  return Changed;
+}
+
+PreservedAnalyses DXILForwardHandleAccesses::run(Function &F,
+                                                 FunctionAnalysisManager &AM) {
+  PreservedAnalyses PA;
+
+  DominatorTree *DT = &AM.getResult<DominatorTreeAnalysis>(F);
+  bool Changed = forwardHandleAccesses(F, *DT);
+
+  if (!Changed)
+    return PreservedAnalyses::all();
+  return PA;
+}
+
+namespace {
+class DXILForwardHandleAccessesLegacy : public FunctionPass {
+public:
+  bool runOnFunction(Function &F) override {
+    DominatorTree *DT = &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
+    return forwardHandleAccesses(F, *DT);
+  }
+  StringRef getPassName() const override {
+    return "DXIL Forward Handle Accesses";
+  }
+
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.addRequired<DominatorTreeWrapperPass>();
+  }
+
+  DXILForwardHandleAccessesLegacy() : FunctionPass(ID) {}
+
+  static char ID; // Pass identification.
+};
+char DXILForwardHandleAccessesLegacy::ID = 0;
+} // end anonymous namespace
+
+INITIALIZE_PASS_BEGIN(DXILForwardHandleAccessesLegacy, DEBUG_TYPE,
+                      "DXIL Forward Handle Accesses", false, false)
+INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
+INITIALIZE_PASS_END(DXILForwardHandleAccessesLegacy, DEBUG_TYPE,
+                    "DXIL Forward Handle Accesses", false, false)
+
+FunctionPass *llvm::createDXILForwardHandleAccessesLegacyPass() {
+  return new DXILForwardHandleAccessesLegacy();
+}
diff --git a/llvm/lib/Target/DirectX/DXILForwardHandleAccesses.h b/llvm/lib/Target/DirectX/DXILForwardHandleAccesses.h
new file mode 100644
index 0000000000000..76940287a50ad
--- /dev/null
+++ b/llvm/lib/Target/DirectX/DXILForwardHandleAccesses.h
@@ -0,0 +1,28 @@
+//===- DXILForwardHandleAccesses.h - Cleanup Handles ------------*- 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 Eliminate redundant stores and loads from handle globals.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIB_TARGET_DIRECTX_DXILFORWARDHANDLEACCESS_H
+#define LLVM_LIB_TARGET_DIRECTX_DXILFORWARDHANDLEACCESS_H
+
+#include "llvm/IR/PassManager.h"
+
+namespace llvm {
+
+class DXILForwardHandleAccesses
+    : public PassInfoMixin<DXILForwardHandleAccesses> {
+public:
+  PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
+};
+
+} // namespace llvm
+
+#endif // LLVM_LIB_TARGET_DIRECTX_DXILFORWARDHANDLEACCESS_H
diff --git a/llvm/lib/Target/DirectX/DirectX.h b/llvm/lib/Target/DirectX/DirectX.h
index 96a8a08c875f8..6e11eea11fed6 100644
--- a/llvm/lib/Target/DirectX/DirectX.h
+++ b/llvm/lib/Target/DirectX/DirectX.h
@@ -47,6 +47,12 @@ void initializeDXILFlattenArraysLegacyPass(PassRegistry &);
 /// Pass to flatten arrays into a one dimensional DXIL legal form
 ModulePass *createDXILFlattenArraysLegacyPass();
 
+/// Initializer for DXIL Forward Handle Accesses Pass
+void initializeDXILForwardHandleAccessesLegacyPass(PassRegistry &);
+
+/// Pass to eliminate redundant stores and loads from handle globals.
+FunctionPass *createDXILForwardHandleAccessesLegacyPass();
+
 /// Initializer DXIL legalizationPass
 void initializeDXILLegalizeLegacyPass(PassRegistry &);
 
diff --git a/llvm/lib/Target/DirectX/DirectXPassRegistry.def b/llvm/lib/Target/DirectX/DirectXPassRegistry.def
index 87d91ead1896f..afea7c24a833a 100644
--- a/llvm/lib/Target/DirectX/DirectXPassRegistry.def
+++ b/llvm/lib/Target/DirectX/DirectXPassRegistry.def
@@ -37,6 +37,7 @@ MODULE_PASS("print<dxil-root-signature>", dxil::RootSignatureAnalysisPrinter(dbg
 #ifndef FUNCTION_PASS
 #define FUNCTION_PASS(NAME, CREATE_PASS)
 #endif
+FUNCTION_PASS("dxil-forward-handle-accesses", DXILForwardHandleAccesses())
 FUNCTION_PASS("dxil-resource-access", DXILResourceAccess())
 FUNCTION_PASS("dxil-legalize", DXILLegalizePass())
 #undef FUNCTION_PASS
diff --git a/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp b/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp
index 747e4b3eb9411..5aaa400589456 100644
--- a/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp
+++ b/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp
@@ -14,6 +14,7 @@
 #include "DirectXTargetMachine.h"
 #include "DXILDataScalarization.h"
 #include "DXILFlattenArrays.h"
+#include "DXILForwardHandleAccesses.h"
 #include "DXILIntrinsicExpansion.h"
 #include "DXILLegalizePass.h"
 #include "DXILOpLowering.h"
@@ -65,6 +66,7 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeDirectXTarget() {
   initializeRootSignatureAnalysisWrapperPass(*PR);
   initializeDXILFinalizeLinkageLegacyPass(*PR);
   initializeDXILPrettyPrinterLegacyPass(*PR);
+  initializeDXILForwardHandleAccessesLegacyPass(*PR);
 }
 
 class DXILTargetObjectFile : public TargetLoweringObjectFile {
@@ -102,6 +104,7 @@ class DirectXPassConfig : public TargetPassConfig {
     ScalarizerPassOptions DxilScalarOptions;
     DxilScalarOptions.ScalarizeLoadStore = true;
     addPass(createScalarizerPass(DxilScalarOptions));
+    addPass(createDXILForwardHandleAccessesLegacyPass());
     addPass(createDXILLegalizeLegacyPass());
     addPass(createDXILTranslateMetadataLegacyPass());
     addPass(createDXILOpLoweringLegacyPass());
diff --git a/llvm/test/CodeGen/DirectX/ForwardHandleAccesses/alloca.ll b/llvm/test/CodeGen/DirectX/ForwardHandleAccesses/alloca.ll
new file mode 100644
index 0000000000000..f9abfbddeae57
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ForwardHandleAccesses/alloca.ll
@@ -0,0 +1,20 @@
+; RUN: not opt -S -dxil-forward-handle-accesses -mtriple=dxil--shadermodel6.3-library %s 2>&1 | FileCheck %s
+
+; CHECK: error: Load of "buf" is not a global resource handle
+
+%"class.hlsl::RWStructuredBuffer" = type { target("dx.RawBuffer", <4 x float>, 1, 0) }
+@Buf = internal global %"class.hlsl::RWStructuredBuffer" poison, align 4
+
+define float @f() {
+entry:
+  %buf = alloca target("dx.RawBuffer", <4 x float>, 1, 0), align 4
+  %h = call target("dx.RawBuffer", <4 x float>, 1, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 0, i32 1, i32 0, i1 false)
+  store target("dx.RawBuffer", <4 x float>, 1, 0) %h, ptr %buf, align 4
+
+  %b = load target("dx.RawBuffer", <4 x float>, 1, 0), ptr %buf, align 4
+  %l = call { <4 x float>, i1 } @llvm.dx.resource.load.rawbuffer(target("dx.RawBuffer", <4 x float>, 1, 0) %b, i32 0, i32 0)
+  %x = extractvalue { <4 x float>, i1 } %l, 0
+  %v = extractelement <4 x float> %x, i32 0
+
+  ret float %v
+}
diff --git a/llvm/test/CodeGen/DirectX/ForwardHandleAccesses/ambiguous.ll b/llvm/test/CodeGen/DirectX/ForwardHandleAccesses/ambiguous.ll
new file mode 100644
index 0000000000000..62cd04e0032fb
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ForwardHandleAccesses/ambiguous.ll
@@ -0,0 +1,21 @@
+; RUN: not opt -S -dxil-forward-handle-accesses -mtriple=dxil--shadermodel6.3-library %s 2>&1 | FileCheck %s
+
+; CHECK: error: Handle at "h2" overwrites handle at "h1"
+
+%"class.hlsl::RWStructuredBuffer" = type { target("dx.RawBuffer", <4 x float>, 1, 0) }
+@Buf = internal global %"class.hlsl::RWStructuredBuffer" poison, align 4
+
+define float @f() {
+entry:
+  %h1 = call target("dx.RawBuffer", <4 x float>, 1, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 0, i32 1, i32 0, i1 false)
+  store target("dx.RawBuffer", <4 x float>, 1, 0) %h1, ptr @Buf, align 4
+  %h2 = call target("dx.RawBuffer", <4 x float>, 1, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 1, i32 1, i32 0, i1 false)
+  store target("dx.RawBuffer", <4 x float>, 1, 0) %h2, ptr @Buf, align 4
+
+  %b = load target("dx.RawBuffer", <4 x float>, 1, 0), ptr @Buf, align 4
+  %l = call { <4 x float>, i1 } @llvm.dx.resource.load.rawbuffer(target("dx.RawBuffer", <4 x float>, 1, 0) %b, i32 0, i32 0)
+  %x = extractvalue { <4 x float>, i1 } %l, 0
+  %v = extractelement <4 x float> %x, i32 0
+
+  ret float %v
+}
diff --git a/llvm/test/CodeGen/DirectX/ForwardHandleAccesses/buffer-O0.ll b/llvm/test/CodeGen/DirectX/ForwardHandleAccesses/buffer-O0.ll
new file mode 100644
index 0000000000000..880fefd57e029
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ForwardHandleAccesses/buffer-O0.ll
@@ -0,0 +1,44 @@
+; RUN: opt -S -dxil-forward-handle-accesses -mtriple=dxil--shadermodel6.3-library %s | FileCheck %s
+
+%"class.hlsl::RWStructuredBuffer" = type { target("dx.RawBuffer", <4 x float>, 1, 0) }
+
+@_ZL2In = internal global %"class.hlsl::RWStructuredBuffer" poison, align 4
+@_ZL3Out = internal global %"class.hlsl::RWStructuredBuffer" poison, align 4
+
+define void @main() #1 {
+entry:
+  %this.addr.i.i.i = alloca ptr, align 4
+  %this.addr.i.i = alloca ptr, align 4
+  %this.addr.i1 = alloca ptr, align 4
+  %Index.addr.i2 = alloca i32, align 4
+  %this.addr.i = alloca ptr, align 4
+  %Index.addr.i = alloca i32, align 4
+  ; CHECK: [[IN:%.*]] = call target("dx.RawBuffer", <4 x float>, 1, 0) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_v4f32_1_0t(i32 0, i32 0, i32 1, i32 0, i1 false)
+  %_ZL2In_h.i.i = call target("dx.RawBuffer", <4 x float>, 1, 0) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_v4f32_1_0t(i32 0, i32 0, i32 1, i32 0, i1 false)
+  store target("dx.RawBuffer", <4 x float>, 1, 0) %_ZL2In_h.i.i, ptr @_ZL2In, align 4
+  store ptr @_ZL2In, ptr %this.addr.i.i, align 4
+  %this1.i.i = load ptr, ptr %this.addr.i.i, align 4
+  ; CHECK: [[OUT:%.*]] = call target("dx.RawBuffer", <4 x float>, 1, 0) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_v4f32_1_0t(i32 0, i32 1, i32 1, i32 0, i1 false)
+  %_ZL3Out_h.i.i = call target("dx.RawBuffer", <4 x float>, 1, 0) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_v4f32_1_0t(i32 0, i32 1, i32 1, i32 0, i1 false)
+  store target("dx.RawBuffer", <4 x float>, 1, 0) %_ZL3Out_h.i.i, ptr @_ZL3Out, align 4
+  store ptr @_ZL3Out, ptr %this.addr.i.i.i, align 4
+  %this1.i.i.i = load ptr, ptr %this.addr.i.i.i, align 4
+  store ptr @_ZL2In, ptr %this.addr.i1, align 4
+  store i32 0, ptr %Index.addr.i2, align 4
+  %this1.i3 = load ptr, ptr %this.addr.i1, align 4
+  ; CHECK-NOT: load target("dx.RawBuffer", <4 x float>, 1, 0)
+  %0 = load target("dx.RawBuffer", <4 x float>, 1, 0), ptr %this1.i3, align 4
+  %1 = load i32, ptr %Index.addr.i2, align 4
+  ; CHECK: call { <4 x float>, i1 } @llvm.dx.resource.load.rawbuffer.v4f32.tdx.RawBuffer_v4f32_1_0t(target("dx.RawBuffer", <4 x float>, 1, 0) [[IN]],
+  %2 = call { <4 x float>, i1 } @llvm.dx.resource.load.rawbuffer.v4f32.tdx.RawBuffer_v4f32_1_0t(target("dx.RawBuffer", <4 x float>, 1, 0) %0, i32 %1, i32 0)
+  %3 = extractvalue { <4 x float>, i1 } %2, 0
+  store ptr @_ZL3Out, ptr %this.addr.i, align 4
+  store i32 0, ptr %Index.addr.i, align 4
+  %this1.i = load ptr, ptr %this.addr.i, align 4
+  ; CHECK-NOT: load target("dx.RawBuffer", <4 x float>, 1, 0)
+  %4 = load target("dx.RawBuffer", <4 x float>, 1, 0), ptr %this1.i, align 4
+  %5 = load i32, ptr %Index.addr.i, align 4
+  ; CHECK: call void @llvm.dx.resource.store.rawbuffer.tdx.RawBuffer_v4f32_1_0t.v4f32(target("dx.RawBuffer", <4 x float>, 1, 0) [[OUT]],
+  call void @llvm.dx.resource.store.rawbuffer.tdx.RawBuffer_v4f32_1_0t.v4f32(target("dx.RawBuffer", <4 x float>, 1, 0) %4, i32 %5, i32 0, <4 x float> %3)
+  ret void
+}
diff --git a/llvm/test/CodeGen/DirectX/ForwardHandleAccesses/cbuffer-access.ll b/llvm/test/CodeGen/DirectX/ForwardHandleAccesses/cbuffer-access.ll
new file mode 100644
index 0000000000000..7790cd3ad2ec6
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ForwardHandleAccesses/cbuffer-access.ll
@@ -0,0 +1,23 @@
+; RUN: opt -S -dxil-forward-handle-accesses -mtriple=dxil--shadermodel6.3-library %s | FileCheck %s
+
+%__cblayout_CB = type <{ float, i32, i32 }>
+%struct.Scalars = type { float, i32, i32 }
+
+@CB.cb = local_unnamed_addr global target("dx.CBuffer", target("dx.Layout", %__cblayout_CB, 12, 0, 4, 8)) poison
+
+define void @main() local_unnamed_addr #1 {
+entry:
+  ; CHECK: [[CB:%.*]] = tail call target({{.*}}) @llvm.dx.resource.handlefrombinding
+  %h = tail call target("dx.CBuffer", target("dx.Layout", %__cblayout_CB, 12, 0, 4, 8)) @llvm.dx.resource.handlefrombinding(i32 0, i32 0, i32 1, i32 0, i1 false)
+  store target("dx.CBuffer", target("dx.Layout", %__cblayout_CB, 12, 0, 4, 8)) %h, ptr @CB.cb, align 4
+  %_ZL3Out_h.i.i = tail call target("dx.RawBuffer", %struct.Scalars, 1, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 0, i32 1, i32 0, i1 false)
+  ; CHECK-NOT: load target({{.*}}), ptr @CB.cb
+  %cb = load target("dx.CBuffer", target("dx.Layout", %__cblayout_CB, 12, 0, 4, 8)), ptr @CB.cb, align 4
+  ; CHECK: call { float, float, float, float } @llvm.dx.resource.load.cbufferrow.4.{{.*}}(target({{.*}}) [[CB]], i32 0)
+  %0 = call { float, float, float, float } @llvm.dx.resource.load.cbufferrow.4(target("dx.CBuffer", target("dx.Layout", %__cblayout_CB, 12, 0, 4, 8)) %cb, i32 0)
+  %1 = extractvalue { float, float, float, float } %0, 0
+  call void @llvm.dx.resource.store.rawbuffer(target("dx.RawBuffer", %struct.Scalars, 1, 0) %_ZL3Out_h.i.i, i32 0, i32 0, float %1)
+  ret void
+}
+
+attributes #0 = { mustprogress nofree noinline norecurse nosync nounwind willreturn memory(readwrite, argmem: write, inaccessiblemem: none) "approx-func-fp-math"="false" "frame-pointer"="all" "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
diff --git a/llvm/test/CodeGen/DirectX/ForwardHandleAccesses/undominated.ll b/llvm/test/CodeGen/DirectX/ForwardHandleAccesses/undominated.ll
new file mode 100644
index 0000000000000..03406ca97c62f
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ForwardHandleAccesses/undominated.ll
@@ -0,0 +1,16 @@
+; RUN: not opt -S -dxil-forward-handle-accesses -mtriple=dxil--shadermodel6.3-library %s 2>&1 | FileCheck %s
+
+; CHECK: error: Load at "b" is not dominated by handle creation at "h1"
+
+%"class.hlsl::RWStructuredBuffer" = type { target("dx.RawBuffer", <4 x float>, 1, 0) }
+@Buf = internal global %"class.hlsl::RWStructuredBuffer" poison, align 4
+
+define void @f() {
+entry:
+  %b = load target("dx.RawBuffer", <4 x float>, 1, 0), ptr @Buf, align 4
+
+  %h1 = call target("dx.RawBuffer", <4 x fl...
[truncated]

@bogner
Copy link
Contributor Author

bogner commented Apr 11, 2025

@s-perron This pass would probably be better shared between the DXIL and SPIR-V backends like you're trying to do with #134260, but it's a bit awkward to do so since it needs to know two target specific things:

  1. What a handle creation intrinsic looks like
  2. What a resource handle global looks like

I suppose we could make it a generic pass and have some kind of target hook to get that information, but it's not obvious to me what that should look like. Do you have any ideas here?

@s-perron
Copy link
Contributor

s-perron commented Apr 11, 2025

I'll take a closer look, and try to get back to you next week. I need to try to understand what the input will be.

I took a look, and I'm fine with this being DXIL only (at least for now). We can reconsider later.

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.

LGTM!

Comment on lines +16 to +18
%l = call { <4 x float>, i1 } @llvm.dx.resource.load.rawbuffer(target("dx.RawBuffer", <4 x float>, 1, 0) %b, i32 0, i32 0)
%x = extractvalue { <4 x float>, i1 } %l, 0
%v = extractelement <4 x float> %x, i32 0
Copy link
Member

Choose a reason for hiding this comment

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

Nit - not really related to what this is testing. If the type of the "dx.RawBuffer" is <4 x float>, should it be returning the vector <4 x float> instead of struct {<4 x float>}, and then the extractvalue is not needed? Or am I missing something here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

llvm.dx.resource.load.rawbuffer returns the value (a <4 x float> here) and the i1 for CheckAccessFullyMapped, so the extractvalue is necessary to get at the <4 x float>

Comment on lines +76 to +82
switch (II->getIntrinsicID()) {
case Intrinsic::dx_resource_handlefrombinding:
processHandle(II, HandleMap);
break;
default:
continue;
}
Copy link
Member

Choose a reason for hiding this comment

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

Just curious - why is this not anif? Are you expecting more cases to add later on?

if (II->getIntrinsicID() == Intrinsic::dx_resource_handlefrombinding)
 processHandle(II, HandleMap);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'd expect to add Intrinsic::dx_resourcehandlefrom* as we add heap and library resources.

@farzonl
Copy link
Member

farzonl commented Apr 14, 2025

Does this change mean we won't run GlobalOpt anymore?
I ask because I'm seeing about 427 crashes associated with llvm/lib/Transforms/IPO/GlobalOpt.cpp CleanupConstantGlobalUsers and if this pass replaces it then I'll exclude the GlobalOpt crashes from my triage.

@bogner
Copy link
Contributor Author

bogner commented Apr 23, 2025

Does this change mean we won't run GlobalOpt anymore? I ask because I'm seeing about 427 crashes associated with llvm/lib/Transforms/IPO/GlobalOpt.cpp CleanupConstantGlobalUsers and if this pass replaces it then I'll exclude the GlobalOpt crashes from my triage.

No, I think we still want to run GlobalOpt. GlobalOpt is still useful in the optimization pipeline even if we don't rely on it for cleaning up resource handles.

This pass attempts to forward resource handle creation to accesses of
the handle global. This avoids dependence on optimizations like CSE and
GlobalOpt for correctness of DXIL.

Fixes llvm#134574.
@bogner bogner force-pushed the 2025-04-11-forward-handle-access branch from e6de05e to ca7c111 Compare April 23, 2025 16:26
@bogner bogner merged commit a83b4a2 into llvm:main Apr 23, 2025
7 of 10 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Apr 23, 2025

LLVM Buildbot has detected a new failure on builder lldb-aarch64-ubuntu running on linaro-lldb-aarch64-ubuntu while building llvm at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/16510

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
UNSUPPORTED: lldb-api :: tools/lldb-server/TestGdbRemoteForkResume.py (1203 of 2124)
PASS: lldb-api :: tools/lldb-server/TestGdbRemoteCompletion.py (1204 of 2124)
PASS: lldb-api :: tools/lldb-server/TestGdbRemoteAttach.py (1205 of 2124)
PASS: lldb-api :: tools/lldb-server/TestGdbRemoteExitCode.py (1206 of 2124)
PASS: lldb-api :: tools/lldb-server/TestGdbRemoteHostInfo.py (1207 of 2124)
PASS: lldb-api :: tools/lldb-server/TestGdbRemoteModuleInfo.py (1208 of 2124)
PASS: lldb-api :: tools/lldb-server/TestGdbRemoteAuxvSupport.py (1209 of 2124)
PASS: lldb-api :: tools/lldb-server/TestGdbRemoteKill.py (1210 of 2124)
PASS: lldb-api :: terminal/TestEditlineCompletions.py (1211 of 2124)
UNRESOLVED: lldb-api :: tools/lldb-dap/variables/TestDAP_variables.py (1212 of 2124)
******************** TEST 'lldb-api :: tools/lldb-dap/variables/TestDAP_variables.py' FAILED ********************
Script:
--
/usr/bin/python3.10 /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin --arch aarch64 --build-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/lldb --compiler /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/clang --dsymutil /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin --lldb-obj-root /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb --lldb-libs-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./lib /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/tools/lldb-dap/variables -p TestDAP_variables.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 21.0.0git (https://github.com/llvm/llvm-project.git revision a83b4a2dc9706d9e898f3462b5c2ff5ed05589d2)
  clang revision a83b4a2dc9706d9e898f3462b5c2ff5ed05589d2
  llvm revision a83b4a2dc9706d9e898f3462b5c2ff5ed05589d2
Skipping the following test categories: ['libc++', 'dsym', 'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_darwin_dwarf_missing_obj (TestDAP_variables.TestDAP_variables) (requires one of macosx, darwin, ios, tvos, watchos, bridgeos, iphonesimulator, watchsimulator, appletvsimulator) 
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_darwin_dwarf_missing_obj_with_symbol_ondemand_enabled (TestDAP_variables.TestDAP_variables) (requires one of macosx, darwin, ios, tvos, watchos, bridgeos, iphonesimulator, watchsimulator, appletvsimulator) 
========= DEBUG ADAPTER PROTOCOL LOGS =========
1745427307.865384102 --> (stdin/stdout) {"command":"initialize","type":"request","arguments":{"adapterID":"lldb-native","clientID":"vscode","columnsStartAt1":true,"linesStartAt1":true,"locale":"en-us","pathFormat":"path","supportsRunInTerminalRequest":true,"supportsVariablePaging":true,"supportsVariableType":true,"supportsStartDebuggingRequest":true,"supportsProgressReporting":true,"$__lldb_sourceInitFile":false},"seq":1}
1745427307.867473841 <-- (stdin/stdout) {"body":{"$__lldb_version":"lldb version 21.0.0git (https://github.com/llvm/llvm-project.git revision a83b4a2dc9706d9e898f3462b5c2ff5ed05589d2)\n  clang revision a83b4a2dc9706d9e898f3462b5c2ff5ed05589d2\n  llvm revision a83b4a2dc9706d9e898f3462b5c2ff5ed05589d2","completionTriggerCharacters":["."," ","\t"],"exceptionBreakpointFilters":[{"default":false,"filter":"cpp_catch","label":"C++ Catch"},{"default":false,"filter":"cpp_throw","label":"C++ Throw"},{"default":false,"filter":"objc_catch","label":"Objective-C Catch"},{"default":false,"filter":"objc_throw","label":"Objective-C Throw"}],"supportTerminateDebuggee":true,"supportsBreakpointLocationsRequest":true,"supportsCancelRequest":true,"supportsCompletionsRequest":true,"supportsConditionalBreakpoints":true,"supportsConfigurationDoneRequest":true,"supportsDataBreakpoints":true,"supportsDelayedStackTraceLoading":true,"supportsDisassembleRequest":true,"supportsEvaluateForHovers":true,"supportsExceptionInfoRequest":true,"supportsExceptionOptions":true,"supportsFunctionBreakpoints":true,"supportsHitConditionalBreakpoints":true,"supportsInstructionBreakpoints":true,"supportsLogPoints":true,"supportsModulesRequest":true,"supportsReadMemoryRequest":true,"supportsRestartRequest":true,"supportsSetVariable":true,"supportsStepInTargetsRequest":true,"supportsSteppingGranularity":true,"supportsValueFormattingOptions":true},"command":"initialize","request_seq":1,"seq":0,"success":true,"type":"response"}
1745427307.867716789 --> (stdin/stdout) {"command":"launch","type":"request","arguments":{"program":"/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/tools/lldb-dap/variables/TestDAP_variables.test_indexedVariables/a.out","initCommands":["settings clear -all","settings set symbols.enable-external-lookup false","settings set target.inherit-tcc true","settings set target.disable-aslr false","settings set target.detach-on-error false","settings set target.auto-apply-fixits false","settings set plugin.process.gdb-remote.packet-timeout 60","settings set symbols.clang-modules-cache-path \"/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api\"","settings set use-color false","settings set show-statusline false"],"disableASLR":false,"enableAutoVariableSummaries":false,"enableSyntheticChildDebugging":false,"displayExtendedBacktrace":false,"commandEscapePrefix":null},"seq":2}
1745427307.867914438 <-- (stdin/stdout) {"body":{"category":"console","output":"Running initCommands:\n"},"event":"output","seq":0,"type":"event"}
1745427307.867938757 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings clear -all\n"},"event":"output","seq":0,"type":"event"}
1745427307.867949963 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings set symbols.enable-external-lookup false\n"},"event":"output","seq":0,"type":"event"}
1745427307.867959499 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings set target.inherit-tcc true\n"},"event":"output","seq":0,"type":"event"}
1745427307.867968559 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings set target.disable-aslr false\n"},"event":"output","seq":0,"type":"event"}
1745427307.867976427 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings set target.detach-on-error false\n"},"event":"output","seq":0,"type":"event"}
1745427307.867984533 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings set target.auto-apply-fixits false\n"},"event":"output","seq":0,"type":"event"}
1745427307.867992640 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings set plugin.process.gdb-remote.packet-timeout 60\n"},"event":"output","seq":0,"type":"event"}
1745427307.868013144 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings set symbols.clang-modules-cache-path \"/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api\"\n"},"event":"output","seq":0,"type":"event"}
1745427307.868021488 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings set use-color false\n"},"event":"output","seq":0,"type":"event"}
1745427307.868029356 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings set show-statusline false\n"},"event":"output","seq":0,"type":"event"}
1745427307.945983171 <-- (stdin/stdout) {"command":"launch","request_seq":2,"seq":0,"success":true,"type":"response"}
1745427307.946031570 <-- (stdin/stdout) {"body":{"isLocalProcess":true,"name":"/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/tools/lldb-dap/variables/TestDAP_variables.test_indexedVariables/a.out","startMethod":"launch","systemProcessId":1582479},"event":"process","seq":0,"type":"event"}
1745427307.946050644 <-- (stdin/stdout) {"event":"initialized","seq":0,"type":"event"}
1745427307.946326256 --> (stdin/stdout) {"command":"setBreakpoints","type":"request","arguments":{"source":{"name":"main.cpp","path":"main.cpp"},"sourceModified":false,"lines":[40],"breakpoints":[{"line":40}]},"seq":3}
1745427307.947721481 <-- (stdin/stdout) {"body":{"breakpoints":[{"column":1,"id":1,"instructionReference":"0xAAAAB67A0C54","line":41,"source":{"name":"main.cpp","path":"main.cpp"},"verified":true}]},"command":"setBreakpoints","request_seq":3,"seq":0,"success":true,"type":"response"}

@llvm-ci
Copy link
Collaborator

llvm-ci commented Apr 24, 2025

LLVM Buildbot has detected a new failure on builder lld-x86_64-win running on as-worker-93 while building llvm at step 7 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/2773

Here is the relevant piece of the build log for the reference
Step 7 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM-Unit :: Support/./SupportTests.exe/90/95' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe-LLVM-Unit-20272-90-95.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=95 GTEST_SHARD_INDEX=90 C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe
--

Script:
--
C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe --gtest_filter=ProgramEnvTest.CreateProcessLongPath
--
C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp(160): error: Expected equality of these values:
  0
  RC
    Which is: -2

C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp(163): error: fs::remove(Twine(LongPath)): did not return errc::success.
error number: 13
error message: permission denied



C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp:160
Expected equality of these values:
  0
  RC
    Which is: -2

C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp:163
fs::remove(Twine(LongPath)): did not return errc::success.
error number: 13
error message: permission denied




********************


@damyanp damyanp moved this to Closed in HLSL Support Apr 25, 2025
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
This pass attempts to forward resource handle creation to accesses of
the handle global. This avoids dependence on optimizations like CSE and
GlobalOpt for correctness of DXIL.

Fixes llvm#134574.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
This pass attempts to forward resource handle creation to accesses of
the handle global. This avoids dependence on optimizations like CSE and
GlobalOpt for correctness of DXIL.

Fixes llvm#134574.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
This pass attempts to forward resource handle creation to accesses of
the handle global. This avoids dependence on optimizations like CSE and
GlobalOpt for correctness of DXIL.

Fixes llvm#134574.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

[DirectX] Add a pass to forward handle creation intrinsics to the accesses of those handles
7 participants