[HLSL] Analyze update counter usage #130356
Open
+388
−42
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-backend-directx @llvm/pr-subscribers-llvm-analysis Author: Ashley Coleman (V-FEXrt) ChangesAnalyzes calls to Increment/Decrement count and stores the analysis. Pass also verifies that a given resource is only incremented or decremented but never both. Full diff: https://github.com/llvm/llvm-project/pull/130356.diff 7 Files Affected:
diff --git a/llvm/include/llvm/Analysis/DXILResource.h b/llvm/include/llvm/Analysis/DXILResource.h
index d399457e16916..61ee3b99d250f 100644
--- a/llvm/include/llvm/Analysis/DXILResource.h
+++ b/llvm/include/llvm/Analysis/DXILResource.h
@@ -18,6 +18,10 @@
#include "llvm/Support/Alignment.h"
#include "llvm/Support/DXILABI.h"
+#include <algorithm>
+#include <cstdint>
+#include <unordered_map>
+
namespace llvm {
class CallInst;
class DataLayout;
@@ -407,6 +411,77 @@ class DXILResourceTypeMap {
}
};
+enum ResourceCounterDirection {
+ Increment,
+ Decrement,
+ Unknown,
+};
+
+class DXILResourceCounterDirectionMap {
+ std::vector<std::pair<dxil::ResourceBindingInfo, ResourceCounterDirection>>
+ CounterDirections;
+
+public:
+ bool invalidate(Module &M, const PreservedAnalyses &PA,
+ ModuleAnalysisManager::Invalidator &Inv);
+
+ void populate(Module &M, ModuleAnalysisManager &AM);
+
+ ResourceCounterDirection
+ operator[](const dxil::ResourceBindingInfo &Info) const {
+ auto Lower = std::lower_bound(
+ CounterDirections.begin(), CounterDirections.end(),
+ std::pair{Info, ResourceCounterDirection::Unknown},
+ [](auto lhs, auto rhs) { return lhs.first < rhs.first; });
+
+ if (Lower == CounterDirections.end()) {
+ return ResourceCounterDirection::Unknown;
+ }
+
+ if (Lower->first != Info) {
+ return ResourceCounterDirection::Unknown;
+ }
+
+ return Lower->second;
+ }
+};
+
+class DXILResourceCounterDirectionAnalysis
+ : public AnalysisInfoMixin<DXILResourceCounterDirectionAnalysis> {
+ friend AnalysisInfoMixin<DXILResourceCounterDirectionAnalysis>;
+
+ static AnalysisKey Key;
+
+public:
+ using Result = DXILResourceCounterDirectionMap;
+
+ DXILResourceCounterDirectionMap run(Module &M, ModuleAnalysisManager &AM) {
+ DXILResourceCounterDirectionMap DRCDM{};
+ DRCDM.populate(M, AM);
+ return DRCDM;
+ }
+};
+
+class DXILResourceCounterDirectionWrapperPass : public ImmutablePass {
+ DXILResourceCounterDirectionMap DRCDM;
+
+ virtual void anchor();
+
+public:
+ static char ID;
+ DXILResourceCounterDirectionWrapperPass();
+
+ DXILResourceCounterDirectionMap &getResourceCounterDirectionMap() {
+ return DRCDM;
+ }
+ const DXILResourceCounterDirectionMap &
+ getResourceCounterDirectionMap() const {
+ return DRCDM;
+ }
+};
+
+ModulePass *createDXILResourceCounterDirectionWrapperPassPass();
+
class DXILResourceTypeAnalysis
: public AnalysisInfoMixin<DXILResourceTypeAnalysis> {
friend AnalysisInfoMixin<DXILResourceTypeAnalysis>;
diff --git a/llvm/include/llvm/InitializePasses.h b/llvm/include/llvm/InitializePasses.h
index 0dfb46a210c7e..3848f0dd6fe03 100644
--- a/llvm/include/llvm/InitializePasses.h
+++ b/llvm/include/llvm/InitializePasses.h
@@ -85,6 +85,7 @@ void initializeDCELegacyPassPass(PassRegistry &);
void initializeDXILMetadataAnalysisWrapperPassPass(PassRegistry &);
void initializeDXILMetadataAnalysisWrapperPrinterPass(PassRegistry &);
void initializeDXILResourceBindingWrapperPassPass(PassRegistry &);
+void initializeDXILResourceCounterDirectionWrapperPassPass(PassRegistry &);
void initializeDXILResourceTypeWrapperPassPass(PassRegistry &);
void initializeDeadMachineInstructionElimPass(PassRegistry &);
void initializeDebugifyMachineModulePass(PassRegistry &);
diff --git a/llvm/lib/Analysis/DXILResource.cpp b/llvm/lib/Analysis/DXILResource.cpp
index c0319c9a354a6..b5e28199593a4 100644
--- a/llvm/lib/Analysis/DXILResource.cpp
+++ b/llvm/lib/Analysis/DXILResource.cpp
@@ -9,6 +9,7 @@
#include "llvm/Analysis/DXILResource.h"
#include "llvm/ADT/APInt.h"
#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/DerivedTypes.h"
#include "llvm/IR/DiagnosticInfo.h"
@@ -19,6 +20,7 @@
#include "llvm/IR/Module.h"
#include "llvm/InitializePasses.h"
#include "llvm/Support/FormatVariadic.h"
+#include <algorithm>
#define DEBUG_TYPE "dxil-resource"
@@ -818,8 +820,92 @@ DXILBindingMap::findByUse(const Value *Key) const {
//===----------------------------------------------------------------------===//
+static bool isUpdateCounterIntrinsic(Function &F) {
+ return F.getIntrinsicID() == Intrinsic::dx_resource_updatecounter;
+}
+
+void DXILResourceCounterDirectionMap::populate(Module &M,
+ ModuleAnalysisManager &AM) {
+ DXILBindingMap &DBM = AM.getResult<DXILResourceBindingAnalysis>(M);
+ CounterDirections.clear();
+
+ for (Function &F : M.functions()) {
+ if (!isUpdateCounterIntrinsic(F))
+ continue;
+
+ for (const User *U : F.users()) {
+ const CallInst *CI = dyn_cast<CallInst>(U);
+ assert(CI && "Users of dx_resource_updateCounter must be call instrs");
+
+ // Determine if the use is an increment or decrement
+ Value *CountArg = CI->getArgOperand(1);
+ ConstantInt *CountValue = dyn_cast<ConstantInt>(CountArg);
+ int64_t CountLiteral = CountValue->getSExtValue();
+
+ ResourceCounterDirection Direction = ResourceCounterDirection::Unknown;
+ if (CountLiteral > 0) {
+ Direction = ResourceCounterDirection::Increment;
+ }
+ if (CountLiteral < 0) {
+ Direction = ResourceCounterDirection::Decrement;
+ }
+
+ // Collect all potential creation points for the handle arg
+ Value *HandleArg = CI->getArgOperand(0);
+ SmallVector<dxil::ResourceBindingInfo> RBInfos = DBM.findByUse(HandleArg);
+ for (const dxil::ResourceBindingInfo RBInfo : RBInfos) {
+ CounterDirections.emplace_back(RBInfo, Direction);
+ }
+ }
+ }
+
+ // An entry that is not in the map is considered unknown so its wasted
+ // overhead and increased complexity to keep an entry explicitly marked
+ // unknown
+ const auto RemoveEnd = std::remove_if(
+ CounterDirections.begin(), CounterDirections.end(), [](const auto &Item) {
+ return Item.second == ResourceCounterDirection::Unknown;
+ });
+
+ // Order for fast lookup
+ std::sort(CounterDirections.begin(), RemoveEnd);
+
+ // Remove the duplicate entries. Since direction is considered for equality
+ // a unique resource with more than one direction will not be deduped.
+ const auto UniqueEnd = std::unique(CounterDirections.begin(), RemoveEnd);
+
+ // Actually erase the items invalidated by remove_if + unique
+ CounterDirections.erase(UniqueEnd, CounterDirections.end());
+
+ // If any duplicate entries still exist at this point then it must be a
+ // resource that was both incremented and decremented which is not allowed.
+ const auto DuplicateEntry = std::adjacent_find(
+ CounterDirections.begin(), CounterDirections.end(),
+ [](const auto &LHS, const auto &RHS) { return LHS.first == RHS.first; });
+ if (DuplicateEntry == CounterDirections.end())
+ return;
+
+ StringRef Message = "RWStructuredBuffers may increment or decrement their "
+ "counters, but not both.";
+ M.getContext().diagnose(DiagnosticInfoGeneric(Message));
+}
+
+bool DXILResourceCounterDirectionMap::invalidate(
+ Module &M, const PreservedAnalyses &PA,
+ ModuleAnalysisManager::Invalidator &Inv) {
+ // Passes that introduce resource types must explicitly invalidate this pass.
+ // auto PAC = PA.getChecker<DXILResourceTypeAnalysis>();
+ // return !PAC.preservedWhenStateless();
+ return false;
+}
+
+void DXILResourceCounterDirectionWrapperPass::anchor() {}
+
+//===----------------------------------------------------------------------===//
+
AnalysisKey DXILResourceTypeAnalysis::Key;
AnalysisKey DXILResourceBindingAnalysis::Key;
+AnalysisKey DXILResourceCounterDirectionAnalysis::Key;
DXILBindingMap DXILResourceBindingAnalysis::run(Module &M,
ModuleAnalysisManager &AM) {
@@ -838,6 +924,16 @@ DXILResourceBindingPrinterPass::run(Module &M, ModuleAnalysisManager &AM) {
return PreservedAnalyses::all();
}
+INITIALIZE_PASS(DXILResourceCounterDirectionWrapperPass,
+ "dxil-resource-counter", "DXIL Resource Counter Analysis",
+ false, true)
+
+DXILResourceCounterDirectionWrapperPass::
+ DXILResourceCounterDirectionWrapperPass()
+ : ImmutablePass(ID) {
+ initializeDXILResourceTypeWrapperPassPass(*PassRegistry::getPassRegistry());
+}
+
void DXILResourceTypeWrapperPass::anchor() {}
DXILResourceTypeWrapperPass::DXILResourceTypeWrapperPass() : ImmutablePass(ID) {
@@ -847,11 +943,16 @@ DXILResourceTypeWrapperPass::DXILResourceTypeWrapperPass() : ImmutablePass(ID) {
INITIALIZE_PASS(DXILResourceTypeWrapperPass, "dxil-resource-type",
"DXIL Resource Type Analysis", false, true)
char DXILResourceTypeWrapperPass::ID = 0;
+char DXILResourceCounterDirectionWrapperPass::ID = 0;
ModulePass *llvm::createDXILResourceTypeWrapperPassPass() {
return new DXILResourceTypeWrapperPass();
}
+ModulePass *llvm::createDXILResourceCounterDirectionWrapperPassPass() {
+ return new DXILResourceCounterDirectionWrapperPass();
+}
+
DXILResourceBindingWrapperPass::DXILResourceBindingWrapperPass()
: ModulePass(ID) {
initializeDXILResourceBindingWrapperPassPass(
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index a664d6fd7085f..5812add8c701a 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -24,6 +24,7 @@ MODULE_ANALYSIS("ctx-prof-analysis", CtxProfAnalysis())
MODULE_ANALYSIS("dxil-metadata", DXILMetadataAnalysis())
MODULE_ANALYSIS("dxil-resource-binding", DXILResourceBindingAnalysis())
MODULE_ANALYSIS("dxil-resource-type", DXILResourceTypeAnalysis())
+MODULE_ANALYSIS("dxil-resource-counter-direction", DXILResourceCounterDirectionAnalysis())
MODULE_ANALYSIS("inline-advisor", InlineAdvisorAnalysis())
MODULE_ANALYSIS("ir-similarity", IRSimilarityAnalysis())
MODULE_ANALYSIS("last-run-tracking", LastRunTrackingAnalysis())
diff --git a/llvm/lib/Target/DirectX/DXILOpLowering.cpp b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
index 83cc4b18824c7..ba2f2d9096d03 100644
--- a/llvm/lib/Target/DirectX/DXILOpLowering.cpp
+++ b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
@@ -816,6 +816,8 @@ class OpLowerer {
PreservedAnalyses DXILOpLowering::run(Module &M, ModuleAnalysisManager &MAM) {
DXILBindingMap &DBM = MAM.getResult<DXILResourceBindingAnalysis>(M);
DXILResourceTypeMap &DRTM = MAM.getResult<DXILResourceTypeAnalysis>(M);
+ DXILResourceCounterDirectionMap &DRCDM =
+ MAM.getResult<DXILResourceCounterDirectionAnalysis>(M);
bool MadeChanges = OpLowerer(M, DBM, DRTM).lowerIntrinsics();
if (!MadeChanges)
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 0000000000000..48ce32031ef00
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/resource_counter_error.ll
@@ -0,0 +1,13 @@
+; RUN: not opt -S -passes='dxil-op-lower' -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.tdx.RawBuffer_f32_1_0t(i32 1, i32 2, i32 3, i32 4, i1 false)
+ call i32 @llvm.dx.resource.updatecounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer", float, 1, 0) %handle, i8 -1)
+ call i32 @llvm.dx.resource.updatecounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer", float, 1, 0) %handle, i8 1)
+ ret void
+}
+
+declare target("dx.RawBuffer", float, 1, 0) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_f32_1_0t(i32, i32, i32, i32, i1)
+declare i32 @llvm.dx.resource.updatecounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer", float, 1, 0), i8)
diff --git a/llvm/unittests/Target/DirectX/UniqueResourceFromUseTests.cpp b/llvm/unittests/Target/DirectX/UniqueResourceFromUseTests.cpp
index f272381c0c250..b653bc3127ffc 100644
--- a/llvm/unittests/Target/DirectX/UniqueResourceFromUseTests.cpp
+++ b/llvm/unittests/Target/DirectX/UniqueResourceFromUseTests.cpp
@@ -35,6 +35,7 @@ class UniqueResourceFromUseTest : public testing::Test {
PB->registerModuleAnalyses(*MAM);
MAM->registerPass([&] { return DXILResourceTypeAnalysis(); });
MAM->registerPass([&] { return DXILResourceBindingAnalysis(); });
+ MAM->registerPass([&] { return DXILResourceCounterDirectionAnalysis(); });
}
virtual void TearDown() {
@@ -280,4 +281,171 @@ declare target("dx.RawBuffer", float, 1, 0) @ind.func(target("dx.RawBuffer", flo
}
}
+TEST_F(UniqueResourceFromUseTest, TestResourceCounterDecrement) {
+ StringRef Assembly = R"(
+define void @main() {
+entry:
+ %handle = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_f32_1_0t(i32 1, i32 2, i32 3, i32 4, i1 false)
+ call i32 @llvm.dx.resource.updatecounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer", float, 1, 0) %handle, i8 -1)
+ call i32 @llvm.dx.resource.updatecounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer", float, 1, 0) %handle, i8 -1)
+ call i32 @llvm.dx.resource.updatecounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer", float, 1, 0) %handle, i8 -1)
+ call void @a.func(target("dx.RawBuffer", float, 1, 0) %handle)
+ ret void
+}
+
+declare target("dx.RawBuffer", float, 1, 0) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_f32_1_0t(i32, i32, i32, i32, i1)
+declare i32 @llvm.dx.resource.updatecounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer", float, 1, 0), i8)
+declare void @a.func(target("dx.RawBuffer", float, 1, 0) %handle)
+ )";
+
+ LLVMContext Context;
+ SMDiagnostic Error;
+ auto M = parseAssemblyString(Assembly, Error, Context);
+ ASSERT_TRUE(M) << "Bad assembly?";
+
+ const DXILBindingMap &DBM = MAM->getResult<DXILResourceBindingAnalysis>(*M);
+ const DXILResourceCounterDirectionMap &DCDM =
+ MAM->getResult<DXILResourceCounterDirectionAnalysis>(*M);
+
+ for (const Function &F : M->functions()) {
+ if (F.getName() != "a.func") {
+ continue;
+ }
+
+ for (const User *U : F.users()) {
+ const CallInst *CI = cast<CallInst>(U);
+ const Value *Handle = CI->getArgOperand(0);
+ const auto Bindings = DBM.findByUse(Handle);
+ ASSERT_EQ(Bindings.size(), 1u);
+ ASSERT_EQ(DCDM[Bindings.front()], ResourceCounterDirection::Decrement);
+ }
+ }
+}
+
+TEST_F(UniqueResourceFromUseTest, TestResourceCounterIncrement) {
+ StringRef Assembly = R"(
+define void @main() {
+entry:
+ %handle = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_f32_1_0t(i32 1, i32 2, i32 3, i32 4, i1 false)
+ call i32 @llvm.dx.resource.updatecounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer", float, 1, 0) %handle, i8 1)
+ call i32 @llvm.dx.resource.updatecounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer", float, 1, 0) %handle, i8 1)
+ call i32 @llvm.dx.resource.updatecounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer", float, 1, 0) %handle, i8 1)
+ call void @a.func(target("dx.RawBuffer", float, 1, 0) %handle)
+ ret void
+}
+
+declare target("dx.RawBuffer", float, 1, 0) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_f32_1_0t(i32, i32, i32, i32, i1)
+declare i32 @llvm.dx.resource.updatecounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer", float, 1, 0), i8)
+declare void @a.func(target("dx.RawBuffer", float, 1, 0) %handle)
+ )";
+
+ LLVMContext Context;
+ SMDiagnostic Error;
+ auto M = parseAssemblyString(Assembly, Error, Context);
+ ASSERT_TRUE(M) << "Bad assembly?";
+
+ const DXILBindingMap &DBM = MAM->getResult<DXILResourceBindingAnalysis>(*M);
+ const DXILResourceCounterDirectionMap &DCDM =
+ MAM->getResult<DXILResourceCounterDirectionAnalysis>(*M);
+
+ for (const Function &F : M->functions()) {
+ if (F.getName() != "a.func") {
+ continue;
+ }
+
+ for (const User *U : F.users()) {
+ const CallInst *CI = cast<CallInst>(U);
+ const Value *Handle = CI->getArgOperand(0);
+ const auto Bindings = DBM.findByUse(Handle);
+ ASSERT_EQ(Bindings.size(), 1u);
+ ASSERT_EQ(DCDM[Bindings.front()], ResourceCounterDirection::Increment);
+ }
+ }
+}
+
+TEST_F(UniqueResourceFromUseTest, TestResourceCounterUnknown) {
+ StringRef Assembly = R"(
+define void @main() {
+entry:
+ %handle = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_f32_1_0t(i32 1, i32 2, i32 3, i32 4, i1 false)
+ call void @a.func(target("dx.RawBuffer", float, 1, 0) %handle)
+ ret void
+}
+
+declare target("dx.RawBuffer", float, 1, 0) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_f32_1_0t(i32, i32, i32, i32, i1)
+declare i32 @llvm.dx.resource.updatecounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer", float, 1, 0), i8)
+declare void @a.func(target("dx.RawBuffer", float, 1, 0) %handle)
+ )";
+
+ LLVMContext Context;
+ SMDiagnostic Error;
+ auto M = parseAssemblyString(Assembly, Error, Context);
+ ASSERT_TRUE(M) << "Bad assembly?";
+
+ const DXILBindingMap &DBM = MAM->getResult<DXILResourceBindingAnalysis>(*M);
+ const DXILResourceCounterDirectionMap &DCDM =
+ MAM->getResult<DXILResourceCounterDirectionAnalysis>(*M);
+
+ for (const Function &F : M->functions()) {
+ if (F.getName() != "a.func") {
+ continue;
+ }
+
+ for (const User *U : F.users()) {
+ const CallInst *CI = cast<CallInst>(U);
+ const Value *Handle = CI->getArgOperand(0);
+ const auto Bindings = DBM.findByUse(Handle);
+ ASSERT_EQ(Bindings.size(), 1u);
+ ASSERT_EQ(DCDM[Bindings.front()], ResourceCounterDirection::Unknown);
+ }
+ }
+}
+
+TEST_F(UniqueResourceFromUseTest, TestResourceCounterMultiple) {
+ StringRef Assembly = R"(
+define void @main() {
+entry:
+ %handle1 = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_f32_1_0t(i32 1, i32 2, i32 3, i32 4, i1 false)
+ %handle2 = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_f32_1_0t(i32 4, i32 3, i32 2, i32 1, i1 false)
+ call i32 @llvm.dx.resource.updatecounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer", float, 1, 0) %handle1, i8 -1)
+ call i32 @llvm.dx.resource.updatecounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer", float, 1, 0) %handle2, i8 1)
+ call void @a.func(target("dx.RawBuffer", float, 1, 0) %handle1)
+ call void @b.func(target("dx.RawBuffer", float, 1, 0) %handle2)
+ ret void
+}
+
+declare target("dx.RawBuffer", float, 1, 0) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_f32_1_0t(i32, i32, i32, i32, i1)
+declare i32 @llvm.dx.resource.updatecounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer", float, 1, 0), i8)
+declare void @a.func(target("dx.RawBuffer", float, 1, 0) %handle)
+declare void @b.func(target("dx.RawBuffer", float, 1, 0) %handle)
+ )";
+
+ LLVMContext Context;
+ SMDiagnostic Error;
+ auto M = parseAssemblyString(Assembly, Error, Context);
+ ASSERT_TRUE(M) << "Bad assembly?";
+
+ const DXILBindingMap &DBM = MAM->getResult<DXILResourceBindingAnalysis>(*M);
+ const DXILResourceCounterDirectionMap &DCDM =
+ MAM->getResult<DXILResourceCounterDirectionAnalysis>(*M);
+
+ for (const Function &F : M->functions()) {
+ StringRef FName = F.getName();
+ if (FName != "a.func" && FName != "b.func") {
+ continue;
+ }
+
+ auto Dir = FName == "a.func" ? ResourceCounterDirection::Decrement
+ : ResourceCounterDirection::Increment;
+
+ for (const User *U : F.users()) {
+ const CallInst *CI = cast<CallInst>(U);
+ const Value *Handle = CI->getArgOperand(0);
+ const auto Bindings = DBM.findByUse(Handle);
+ ASSERT_EQ(Bindings.size(), 1u);
+ ASSERT_EQ(DCDM[Bindings.front()], Dir);
+ }
+ }
+}
+
} // namespace
|
V-FEXrt
commented
Mar 7, 2025
V-FEXrt
commented
Mar 7, 2025
V-FEXrt
commented
Mar 7, 2025
V-FEXrt
commented
Mar 7, 2025
joaosaffran
reviewed
Mar 7, 2025
inbelic
reviewed
Mar 10, 2025
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
V-FEXrt
commented
Mar 12, 2025
V-FEXrt
commented
Mar 12, 2025
bogner
reviewed
Mar 20, 2025
llvm/lib/Analysis/DXILResource.cpp
Outdated
Comment on lines
928
to
929
| M.getContext().diagnose( | ||
| DiagnosticInfoGenericWithLoc(Message, *F, CI->getDebugLoc())); |
There was a problem hiding this comment.
With the uniquing above, does this emit a diagnostic for exactly one (arbitrarily chosen) increment and one decrement? We should think a little bit about the user experience here and make sure it's going to be reasonable.
There was a problem hiding this comment.
ahhhhh yep. That's not ideal. I think it may still be an improvement over dxc where the diag is only emitted for a single call. Not sure if its arbitrary or consistent on which one is picked
There was a problem hiding this comment.
Do you have any opinions on what we should do?
There was a problem hiding this comment.
stable_sort should make it at least consistent but its still going to be arbitrary which one is picked
There was a problem hiding this comment.
What does DXC do if there are multiple mismatched increments or decrements? Does it report multiple errors?
There was a problem hiding this comment.
Looks to be fairly arbitrary what it does. Here are some links:
https://godbolt.org/z/cns8ToYGb
https://godbolt.org/z/o9qz4fdaG
https://godbolt.org/z/cKdbzdcKn
https://godbolt.org/z/7W8YTbfsx
I thought it might be picking the direction with the least total number of uses but
https://godbolt.org/z/qnG9ahdrr
disproves that
There was a problem hiding this comment.
Yeah, it looks like it is not picking the direction with least total number of uses, which would be nice. However, it is reporting multiple errors, and I think we should do that too. If I have a shader with 2 invalid decrements, I would prefer to get 2 errors. And if I fix one of them, I'd expect the number of errors to go from 2 to 1.
hekota
reviewed
Mar 24, 2025
Co-authored-by: Helena Kotas <hekotas@microsoft.com>
|
After some offline discussions with @hekota and @bogner, it was decided that it's better to have one pass do the analysis and a separate "errors" pass that actually raises Diags, not just for this error but other classes of DXIL errors as well. With that in mind I'm going to put up another patch that decreases the scope of this change then later push up a PR for handling/raising the Diag |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds an analysis that determines if a specific resource binding's counter was incremented, decremented, both, or neither. The analysis provides a map like structure that can later be used to determine the direction of a resource.
This PR is a partial implementation of #114130