Skip to content

[HLSL] Move Resource Instance Properties from TypeInfo #135259

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.

Sign up for GitHub

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 4 commits into from
Apr 14, 2025
Merged

[HLSL] Move Resource Instance Properties from TypeInfo #135259

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a7998f3
[HLSL] Move Resouce Instance Properties from TypeInfo
V-FEXrt Apr 10, 2025
8c8a560
Address Comments
V-FEXrt Apr 11, 2025
8751ed5
Address Comments
V-FEXrt Apr 11, 2025
c7d083a
Address Comments
V-FEXrt Apr 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 19 additions & 25 deletions llvm/include/llvm/Analysis/DXILResource.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,19 +222,11 @@ class LayoutExtType : public TargetExtType {
class ResourceTypeInfo {
public:
struct UAVInfo {
bool GloballyCoherent;
bool HasCounter;
bool IsROV;

bool operator==(const UAVInfo &RHS) const {
return std::tie(GloballyCoherent, HasCounter, IsROV) ==
std::tie(RHS.GloballyCoherent, RHS.HasCounter, RHS.IsROV);
}
bool operator==(const UAVInfo &RHS) const { return IsROV == RHS.IsROV; }
bool operator!=(const UAVInfo &RHS) const { return !(*this == RHS); }
bool operator<(const UAVInfo &RHS) const {
return std::tie(GloballyCoherent, HasCounter, IsROV) <
std::tie(RHS.GloballyCoherent, RHS.HasCounter, RHS.IsROV);
}
bool operator<(const UAVInfo &RHS) const { return IsROV < RHS.IsROV; }
};

struct StructInfo {
Expand Down Expand Up @@ -272,23 +264,14 @@ class ResourceTypeInfo {
private:
TargetExtType *HandleTy;

// GloballyCoherent and HasCounter aren't really part of the type and need to
// be determined by analysis, so they're just provided directly by the
// DXILResourceTypeMap when we construct these.
bool GloballyCoherent;
bool HasCounter;

dxil::ResourceClass RC;
dxil::ResourceKind Kind;

public:
ResourceTypeInfo(TargetExtType *HandleTy, const dxil::ResourceClass RC,
const dxil::ResourceKind Kind, bool GloballyCoherent = false,
bool HasCounter = false);
ResourceTypeInfo(TargetExtType *HandleTy, bool GloballyCoherent = false,
bool HasCounter = false)
: ResourceTypeInfo(HandleTy, {}, dxil::ResourceKind::Invalid,
GloballyCoherent, HasCounter) {}
const dxil::ResourceKind Kind);
ResourceTypeInfo(TargetExtType *HandleTy)
: ResourceTypeInfo(HandleTy, {}, dxil::ResourceKind::Invalid) {}

TargetExtType *getHandleTy() const { return HandleTy; }
StructType *createElementStruct();
Expand All @@ -314,9 +297,6 @@ class ResourceTypeInfo {
dxil::ResourceClass getResourceClass() const { return RC; }
dxil::ResourceKind getResourceKind() const { return Kind; }

void setGloballyCoherent(bool V) { GloballyCoherent = V; }
void setHasCounter(bool V) { HasCounter = V; }

bool operator==(const ResourceTypeInfo &RHS) const;
bool operator!=(const ResourceTypeInfo &RHS) const { return !(*this == RHS); }
bool operator<(const ResourceTypeInfo &RHS) const;
Expand All @@ -326,6 +306,13 @@ class ResourceTypeInfo {

//===----------------------------------------------------------------------===//

enum class ResourceCounterDirection {
Increment,
Decrement,
Unknown,
Invalid,
};

class ResourceInfo {
public:
struct ResourceBinding {
Expand Down Expand Up @@ -353,6 +340,9 @@ class ResourceInfo {
GlobalVariable *Symbol = nullptr;

public:
bool GloballyCoherent = false;
ResourceCounterDirection CounterDirection = ResourceCounterDirection::Unknown;
Comment on lines +343 to +344
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised to see public member variables as well as setter functions for them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a preference for one vs the other? The members need to be read and written externally so I can make them private and add getters or just delete the setters.

FWIW the previous location for these members (I'm just moving them) had public+setters though I don't think the setters were used

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll have to defer to @llvm-beanz or @bogner here I think. I have opinions but I'm not sure they're aligned to LLVM's best practices.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

picked the one with least churn so we can get the PR in. If someone comes along later to disagree, I'll happily create a new PR to swap them!


ResourceInfo(uint32_t RecordID, uint32_t Space, uint32_t LowerBound,
uint32_t Size, TargetExtType *HandleTy,
GlobalVariable *Symbol = nullptr)
Expand All @@ -361,6 +351,10 @@ class ResourceInfo {

void setBindingID(unsigned ID) { Binding.RecordID = ID; }

bool hasCounter() const {
return CounterDirection != ResourceCounterDirection::Unknown;
}

const ResourceBinding &getBinding() const { return Binding; }
TargetExtType *getHandleTy() const { return HandleTy; }
const StringRef getName() const { return Symbol ? Symbol->getName() : ""; }
Expand Down
43 changes: 28 additions & 15 deletions llvm/lib/Analysis/DXILResource.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,8 @@ static dxil::ElementType toDXILElementType(Type *Ty, bool IsSigned) {

ResourceTypeInfo::ResourceTypeInfo(TargetExtType *HandleTy,
const dxil::ResourceClass RC_,
const dxil::ResourceKind Kind_,
bool GloballyCoherent, bool HasCounter)
: HandleTy(HandleTy), GloballyCoherent(GloballyCoherent),
HasCounter(HasCounter) {
const dxil::ResourceKind Kind_)
: HandleTy(HandleTy) {
// If we're provided a resource class and kind, trust them.
if (Kind_ != dxil::ResourceKind::Invalid) {
RC = RC_;
Expand Down Expand Up @@ -377,7 +375,7 @@ static bool isROV(dxil::ResourceKind Kind, TargetExtType *Ty) {

ResourceTypeInfo::UAVInfo ResourceTypeInfo::getUAV() const {
assert(isUAV() && "Not a UAV");
return {GloballyCoherent, HasCounter, isROV(Kind, HandleTy)};
return {isROV(Kind, HandleTy)};
}

uint32_t ResourceTypeInfo::getCBufferSize(const DataLayout &DL) const {
Expand Down Expand Up @@ -469,8 +467,7 @@ uint32_t ResourceTypeInfo::getMultiSampleCount() const {
}

bool ResourceTypeInfo::operator==(const ResourceTypeInfo &RHS) const {
return std::tie(HandleTy, GloballyCoherent, HasCounter) ==
std::tie(RHS.HandleTy, RHS.GloballyCoherent, RHS.HasCounter);
return HandleTy == RHS.HandleTy;
}

bool ResourceTypeInfo::operator<(const ResourceTypeInfo &RHS) const {
Expand Down Expand Up @@ -510,9 +507,7 @@ void ResourceTypeInfo::print(raw_ostream &OS, const DataLayout &DL) const {
} else {
if (isUAV()) {
UAVInfo UAVFlags = getUAV();
OS << " Globally Coherent: " << UAVFlags.GloballyCoherent << "\n"
<< " HasCounter: " << UAVFlags.HasCounter << "\n"
<< " IsROV: " << UAVFlags.IsROV << "\n";
OS << " IsROV: " << UAVFlags.IsROV << "\n";
}
if (isMultiSample())
OS << " Sample Count: " << getMultiSampleCount() << "\n";
Expand Down Expand Up @@ -577,8 +572,8 @@ MDTuple *ResourceInfo::getAsMetadata(Module &M,

if (RTI.isUAV()) {
ResourceTypeInfo::UAVInfo UAVFlags = RTI.getUAV();
MDVals.push_back(getBoolMD(UAVFlags.GloballyCoherent));
MDVals.push_back(getBoolMD(UAVFlags.HasCounter));
MDVals.push_back(getBoolMD(GloballyCoherent));
MDVals.push_back(getBoolMD(hasCounter()));
MDVals.push_back(getBoolMD(UAVFlags.IsROV));
} else {
// All SRVs include sample count in the metadata, but it's only meaningful
Expand Down Expand Up @@ -619,10 +614,10 @@ ResourceInfo::getAnnotateProps(Module &M, dxil::ResourceTypeInfo &RTI) const {
ResourceTypeInfo::UAVInfo UAVFlags =
IsUAV ? RTI.getUAV() : ResourceTypeInfo::UAVInfo{};
bool IsROV = IsUAV && UAVFlags.IsROV;
bool IsGloballyCoherent = IsUAV && UAVFlags.GloballyCoherent;
bool IsGloballyCoherent = IsUAV && GloballyCoherent;
uint8_t SamplerCmpOrHasCounter = 0;
if (IsUAV)
SamplerCmpOrHasCounter = UAVFlags.HasCounter;
SamplerCmpOrHasCounter = hasCounter();
else if (RTI.isSampler())
SamplerCmpOrHasCounter = RTI.getSamplerType() == SamplerType::Comparison;

Expand Down Expand Up @@ -671,6 +666,24 @@ void ResourceInfo::print(raw_ostream &OS, dxil::ResourceTypeInfo &RTI,
<< " Lower Bound: " << Binding.LowerBound << "\n"
<< " Size: " << Binding.Size << "\n";

OS << " Globally Coherent: " << GloballyCoherent << "\n";
OS << " Counter Direction: ";

switch (CounterDirection) {
case ResourceCounterDirection::Increment:
OS << "Increment\n";
break;
case ResourceCounterDirection::Decrement:
OS << "Decrement\n";
break;
case ResourceCounterDirection::Unknown:
OS << "Unknown\n";
break;
case ResourceCounterDirection::Invalid:
OS << "Invalid\n";
break;
}

RTI.print(OS, DL);
}

Expand Down Expand Up @@ -767,7 +780,7 @@ void DXILResourceMap::populate(Module &M, DXILResourceTypeMap &DRTM) {
void DXILResourceMap::print(raw_ostream &OS, DXILResourceTypeMap &DRTM,
const DataLayout &DL) const {
for (unsigned I = 0, E = Infos.size(); I != E; ++I) {
OS << "Binding " << I << ":\n";
OS << "Resource " << I << ":\n";
const dxil::ResourceInfo &RI = Infos[I];
RI.print(OS, DRTM[RI.getHandleTy()], DL);
OS << "\n";
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/DirectX/DXContainerGlobals.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ void DXContainerGlobals::addResourcesForPSV(Module &M, PSVRuntimeInfo &PSV) {

dxil::ResourceTypeInfo &TypeInfo = DRTM[RI.getHandleTy()];
dxbc::PSV::ResourceType ResType;
if (TypeInfo.getUAV().HasCounter)
if (RI.hasCounter())
ResType = dxbc::PSV::ResourceType::UAVStructuredWithCounter;
else if (TypeInfo.isStruct())
ResType = dxbc::PSV::ResourceType::UAVStructured;
Expand Down
11 changes: 7 additions & 4 deletions llvm/lib/Target/DirectX/DXILPrettyPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,11 @@ static StringRef getTextureDimName(dxil::ResourceKind RK) {
namespace {
struct FormatResourceDimension
: public llvm::FormatAdapter<const dxil::ResourceTypeInfo &> {
explicit FormatResourceDimension(const dxil::ResourceTypeInfo &RI)
: llvm::FormatAdapter<const dxil::ResourceTypeInfo &>(RI) {}
FormatResourceDimension(const dxil::ResourceTypeInfo &RI, bool HasCounter)
: llvm::FormatAdapter<const dxil::ResourceTypeInfo &>(RI),
HasCounter(HasCounter) {}

bool HasCounter;

void format(llvm::raw_ostream &OS, StringRef Style) override {
dxil::ResourceKind RK = Item.getResourceKind();
Expand All @@ -155,7 +158,7 @@ struct FormatResourceDimension
case dxil::ResourceKind::StructuredBuffer:
if (!Item.isUAV())
OS << "r/o";
else if (Item.getUAV().HasCounter)
else if (HasCounter)
OS << "r/w+cnt";
else
OS << "r/w";
Expand Down Expand Up @@ -238,7 +241,7 @@ static void prettyPrintResources(raw_ostream &OS, const DXILResourceMap &DRM,
StringRef Name(RI.getName());
StringRef Type(getRCName(RC));
StringRef Format(getFormatName(RTI));
FormatResourceDimension Dim(RTI);
FormatResourceDimension Dim(RTI, RI.hasCounter());
FormatBindingID ID(RI, RTI);
FormatBindingLocation Bind(RI, RTI);
FormatBindingSize Count(RI);
Expand Down
30 changes: 15 additions & 15 deletions llvm/test/Analysis/DXILResource/buffer-frombinding.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ define void @test_typedbuffer() {
%srv0 = call target("dx.RawBuffer", void, 0, 0)
@llvm.dx.resource.handlefrombinding.tdx.RawBuffer_i8_0_0t(
i32 1, i32 8, i32 1, i32 0, i1 false)
; CHECK: Binding [[SRV0:[0-9]+]]:
; CHECK: Resource [[SRV0:[0-9]+]]:
; CHECK: Binding:
; CHECK: Record ID: 0
; CHECK: Space: 1
Expand All @@ -21,7 +21,7 @@ define void @test_typedbuffer() {
%srv1 = call target("dx.RawBuffer", {<4 x float>, <4 x i32>}, 0, 0)
@llvm.dx.resource.handlefrombinding.tdx.RawBuffer_sl_v4f32v4i32s_0_0t(
i32 4, i32 2, i32 1, i32 0, i1 false)
; CHECK: Binding [[SRV1:[0-9]+]]:
; CHECK: Resource [[SRV1:[0-9]+]]:
; CHECK: Binding:
; CHECK: Record ID: 1
; CHECK: Space: 4
Expand All @@ -36,7 +36,7 @@ define void @test_typedbuffer() {
%srv2 = call target("dx.TypedBuffer", <4 x i32>, 0, 0, 0)
@llvm.dx.resource.handlefrombinding.tdx.TypedBuffer_i32_0_0t(
i32 5, i32 3, i32 24, i32 0, i1 false)
; CHECK: Binding [[SRV2:[0-9]+]]:
; CHECK: Resource [[SRV2:[0-9]+]]:
; CHECK: Binding:
; CHECK: Record ID: 2
; CHECK: Space: 5
Expand All @@ -51,16 +51,16 @@ define void @test_typedbuffer() {
%uav0 = call target("dx.TypedBuffer", i32, 1, 0, 1)
@llvm.dx.resource.handlefrombinding.tdx.TypedBuffer_i32_1_0t(
i32 2, i32 7, i32 1, i32 0, i1 false)
; CHECK: Binding [[UAV0:[0-9]+]]:
; CHECK: Resource [[UAV0:[0-9]+]]:
; CHECK: Binding:
; CHECK: Record ID: 0
; CHECK: Space: 2
; CHECK: Lower Bound: 7
; CHECK: Size: 1
; CHECK: Globally Coherent: 0
; CHECK: Counter Direction: Unknown
; CHECK: Class: UAV
; CHECK: Kind: TypedBuffer
; CHECK: Globally Coherent: 0
; CHECK: HasCounter: 0
; CHECK: IsROV: 0
; CHECK: Element Type: i32
; CHECK: Element Count: 1
Expand All @@ -69,16 +69,16 @@ define void @test_typedbuffer() {
%uav1 = call target("dx.TypedBuffer", <4 x float>, 1, 0, 0)
@llvm.dx.resource.handlefrombinding.tdx.TypedBuffer_f32_1_0(
i32 3, i32 5, i32 1, i32 0, i1 false)
; CHECK: Binding [[UAV1:[0-9]+]]:
; CHECK: Resource [[UAV1:[0-9]+]]:
; CHECK: Binding:
; CHECK: Record ID: 1
; CHECK: Space: 3
; CHECK: Lower Bound: 5
; CHECK: Size: 1
; CHECK: Globally Coherent: 0
; CHECK: Counter Direction: Unknown
; CHECK: Class: UAV
; CHECK: Kind: TypedBuffer
; CHECK: Globally Coherent: 0
; CHECK: HasCounter: 0
; CHECK: IsROV: 0
; CHECK: Element Type: f32
; CHECK: Element Count: 4
Expand All @@ -92,23 +92,23 @@ define void @test_typedbuffer() {
%uav2_2 = call target("dx.TypedBuffer", <4 x float>, 1, 0, 0)
@llvm.dx.resource.handlefrombinding.tdx.TypedBuffer_f32_1_0(
i32 4, i32 0, i32 10, i32 5, i1 false)
; CHECK: Binding [[UAV2:[0-9]+]]:
; CHECK: Resource [[UAV2:[0-9]+]]:
; CHECK: Binding:
; CHECK: Record ID: 2
; CHECK: Space: 4
; CHECK: Lower Bound: 0
; CHECK: Size: 10
; CHECK: Globally Coherent: 0
; CHECK: Counter Direction: Unknown
; CHECK: Class: UAV
; CHECK: Kind: TypedBuffer
; CHECK: Globally Coherent: 0
; CHECK: HasCounter: 0
; CHECK: IsROV: 0
; CHECK: Element Type: f32
; CHECK: Element Count: 4

%cb0 = call target("dx.CBuffer", {float})
@llvm.dx.resource.handlefrombinding(i32 1, i32 0, i32 1, i32 0, i1 false)
; CHECK: Binding [[CB0:[0-9]+]]:
; CHECK: Resource [[CB0:[0-9]+]]:
; CHECK: Binding:
; CHECK: Record ID: 0
; CHECK: Space: 1
Expand All @@ -120,7 +120,7 @@ define void @test_typedbuffer() {

%cb1 = call target("dx.CBuffer", target("dx.Layout", {float}, 4, 0))
@llvm.dx.resource.handlefrombinding(i32 1, i32 8, i32 1, i32 0, i1 false)
; CHECK: Binding [[CB1:[0-9]+]]:
; CHECK: Resource [[CB1:[0-9]+]]:
; CHECK: Binding:
; CHECK: Record ID: 1
; CHECK: Space: 1
Expand All @@ -130,7 +130,7 @@ define void @test_typedbuffer() {
; CHECK: Kind: CBuffer
; CHECK: CBuffer size: 4

; CHECK-NOT: Binding {{[0-9]+}}:
; CHECK-NOT: Resource {{[0-9]+}}:

ret void
}
Expand Down
Loading
Loading