Skip to content

Commit 1c608f4

Browse files
author
joaosaffran
committed
address comments
1 parent 7a4382e commit 1c608f4

File tree

2 files changed

+60
-86
lines changed

2 files changed

+60
-86
lines changed

‎llvm/include/llvm/BinaryFormat/DXContainer.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#ifndef LLVM_BINARYFORMAT_DXCONTAINER_H
1414
#define LLVM_BINARYFORMAT_DXCONTAINER_H
1515

16+
#include "llvm/ADT/BitmaskEnum.h"
1617
#include "llvm/ADT/StringRef.h"
1718
#include "llvm/Support/Compiler.h"
1819
#include "llvm/Support/Error.h"
@@ -40,6 +41,8 @@ template <typename T> struct EnumEntry;
4041

4142
namespace dxbc {
4243

44+
LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
45+
4346
inline Triple::EnvironmentType getShaderStage(uint32_t Kind) {
4447
assert(Kind <= Triple::Amplification - Triple::Pixel &&
4548
"Shader kind out of expected range.");
@@ -167,6 +170,8 @@ enum class RootDescriptorFlag : uint32_t {
167170
#define DESCRIPTOR_RANGE_FLAG(Num, Val) Val = 1ull << Num,
168171
enum class DescriptorRangeFlag : uint32_t {
169172
#include "DXContainerConstants.def"
173+
174+
LLVM_MARK_AS_BITMASK_ENUM(DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS)
170175
};
171176

172177
#define ROOT_PARAMETER(Val, Enum) Enum = Val,

‎llvm/lib/Target/DirectX/DXILRootSignature.cpp‎

Lines changed: 55 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,8 @@ static bool parseDescriptorRange(LLVMContext *Ctx,
232232
static bool parseDescriptorTable(LLVMContext *Ctx,
233233
mcdxbc::RootSignatureDesc &RSD,
234234
MDNode *DescriptorTableNode) {
235-
if (DescriptorTableNode->getNumOperands() < 2)
235+
const unsigned int NumOperands = DescriptorTableNode->getNumOperands();
236+
if (NumOperands < 2)
236237
return reportError(Ctx, "Invalid format for Descriptor Table");
237238

238239
dxbc::RTS0::v1::RootParameterHeader Header;
@@ -245,7 +246,7 @@ static bool parseDescriptorTable(LLVMContext *Ctx,
245246
Header.ParameterType =
246247
llvm::to_underlying(dxbc::RootParameterType::DescriptorTable);
247248

248-
for (unsigned int I = 2; I < DescriptorTableNode->getNumOperands(); I++) {
249+
for (unsigned int I = 2; I < NumOperands; I++) {
249250
MDNode *Element = dyn_cast<MDNode>(DescriptorTableNode->getOperand(I));
250251
if (Element == nullptr)
251252
return reportError(Ctx, "Missing Root Element Metadata Node.");
@@ -340,63 +341,40 @@ static bool verifyRangeType(uint32_t Type) {
340341
return false;
341342
}
342343

343-
template <typename... FlagTypes>
344-
static bool isFlagSet(uint32_t Flags, FlagTypes... FlagsToCheck) {
345-
return ((Flags & llvm::to_underlying(FlagsToCheck)) | ...) == Flags;
346-
}
347-
348344
static bool verifyDescriptorRangeFlag(uint32_t Version, uint32_t Type,
349-
uint32_t Flags) {
345+
uint32_t FlagsVal) {
346+
using FlagT = dxbc::DescriptorRangeFlag;
347+
FlagT Flags = FlagT(FlagsVal);
348+
350349
const bool IsSampler =
351350
(Type == llvm::to_underlying(dxbc::DescriptorRangeType::Sampler));
352351

353-
if (Version == 1) {
354-
if (IsSampler) {
355-
return Flags == 0;
352+
// The data-specific flags are mutually exclusive.
353+
FlagT DataFlags = FlagT::DATA_VOLATILE | FlagT::DATA_STATIC |
354+
FlagT::DATA_STATIC_WHILE_SET_AT_EXECUTE;
355+
356+
if (popcount(llvm::to_underlying(Flags & DataFlags)) > 1)
357+
return false;
358+
359+
// For volatile descriptors, DATA_STATIC is never valid.
360+
if ((Flags & FlagT::DESCRIPTORS_VOLATILE) == FlagT::DESCRIPTORS_VOLATILE) {
361+
FlagT Mask = FlagT::DESCRIPTORS_VOLATILE;
362+
if (!IsSampler) {
363+
Mask |= FlagT::DATA_VOLATILE;
364+
Mask |= FlagT::DATA_STATIC_WHILE_SET_AT_EXECUTE;
356365
}
357-
return Flags ==
358-
llvm::to_underlying(dxbc::DescriptorRangeFlag::DESCRIPTORS_VOLATILE);
366+
return (Flags & ~Mask) == FlagT::NONE;
359367
}
360368

361-
if (Version == 2) {
362-
if (IsSampler) {
363-
return Flags == 0 ||
364-
isFlagSet(Flags, dxbc::DescriptorRangeFlag::DATA_VOLATILE) ||
365-
isFlagSet(Flags,
366-
dxbc::DescriptorRangeFlag::
367-
DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS);
368-
}
369-
// Valid flag combinations for non-sampler Version 2
370-
return Flags == 0 ||
371-
isFlagSet(Flags, dxbc::DescriptorRangeFlag::DESCRIPTORS_VOLATILE) ||
372-
isFlagSet(Flags, dxbc::DescriptorRangeFlag::DATA_VOLATILE) ||
373-
isFlagSet(Flags, dxbc::DescriptorRangeFlag::DATA_STATIC) ||
374-
isFlagSet(
375-
Flags,
376-
dxbc::DescriptorRangeFlag::DATA_STATIC_WHILE_SET_AT_EXECUTE) ||
377-
isFlagSet(Flags,
378-
dxbc::DescriptorRangeFlag::
379-
DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS) ||
380-
isFlagSet(Flags, dxbc::DescriptorRangeFlag::DESCRIPTORS_VOLATILE,
381-
dxbc::DescriptorRangeFlag::DATA_VOLATILE) ||
382-
isFlagSet(
383-
Flags, dxbc::DescriptorRangeFlag::DESCRIPTORS_VOLATILE,
384-
dxbc::DescriptorRangeFlag::DATA_STATIC_WHILE_SET_AT_EXECUTE) ||
385-
isFlagSet(Flags,
386-
dxbc::DescriptorRangeFlag::
387-
DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS,
388-
dxbc::DescriptorRangeFlag::DATA_VOLATILE) ||
389-
isFlagSet(Flags,
390-
dxbc::DescriptorRangeFlag::
391-
DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS,
392-
dxbc::DescriptorRangeFlag::DATA_STATIC) ||
393-
isFlagSet(
394-
Flags,
395-
dxbc::DescriptorRangeFlag::
396-
DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS,
397-
dxbc::DescriptorRangeFlag::DATA_STATIC_WHILE_SET_AT_EXECUTE);
369+
// For default (static) or "STATIC_KEEPING_BUFFER_BOUNDS_CHECKS" descriptors,
370+
// the other data-specific flags may all be set.
371+
FlagT Mask = FlagT::DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS;
372+
if (!IsSampler) {
373+
Mask |= FlagT::DATA_VOLATILE;
374+
Mask |= FlagT::DATA_STATIC;
375+
Mask |= FlagT::DATA_STATIC_WHILE_SET_AT_EXECUTE;
398376
}
399-
return false;
377+
return (Flags & ~Mask) == FlagT::NONE;
400378
}
401379

402380
static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) {
@@ -556,7 +534,6 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M,
556534

557535
OS << "Root Signature Definitions"
558536
<< "\n";
559-
uint8_t Space = 0;
560537
for (const Function &F : M) {
561538
auto It = RSDMap.find(&F);
562539
if (It == RSDMap.end())
@@ -565,76 +542,68 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M,
565542
OS << "Definition for '" << F.getName() << "':\n";
566543

567544
// start root signature header
568-
OS << indent(Space) << "Flags: " << format_hex(RS.Flags, 8) << "\n";
569-
OS << indent(Space) << "Version: " << RS.Version << "\n";
570-
OS << indent(Space) << "RootParametersOffset: " << RS.RootParameterOffset
571-
<< "\n";
572-
OS << indent(Space) << "NumParameters: " << RS.ParametersContainer.size()
573-
<< "\n";
545+
OS << "Flags: " << format_hex(RS.Flags, 8) << "\n";
546+
OS << "Version: " << RS.Version << "\n";
547+
OS << "RootParametersOffset: " << RS.RootParameterOffset << "\n";
548+
OS << "NumParameters: " << RS.ParametersContainer.size() << "\n";
574549
for (size_t I = 0; I < RS.ParametersContainer.size(); I++) {
575550
const auto &[Type, Loc] =
576551
RS.ParametersContainer.getTypeAndLocForParameter(I);
577552
const dxbc::RTS0::v1::RootParameterHeader Header =
578553
RS.ParametersContainer.getHeader(I);
579554

580-
OS << indent(Space) << "- Parameter Type: " << Type << "\n";
581-
OS << indent(Space + 2)
582-
<< "Shader Visibility: " << Header.ShaderVisibility << "\n";
555+
OS << "- Parameter Type: " << Type << "\n";
556+
OS << indent(2) << "Shader Visibility: " << Header.ShaderVisibility
557+
<< "\n";
583558

584559
switch (Type) {
585560
case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): {
586561
const dxbc::RTS0::v1::RootConstants &Constants =
587562
RS.ParametersContainer.getConstant(Loc);
588-
OS << indent(Space + 2) << "Register Space: " << Constants.RegisterSpace
563+
OS << indent(2) << "Register Space: " << Constants.RegisterSpace
564+
<< "\n";
565+
OS << indent(2) << "Shader Register: " << Constants.ShaderRegister
566+
<< "\n";
567+
OS << indent(2) << "Num 32 Bit Values: " << Constants.Num32BitValues
589568
<< "\n";
590-
OS << indent(Space + 2)
591-
<< "Shader Register: " << Constants.ShaderRegister << "\n";
592-
OS << indent(Space + 2)
593-
<< "Num 32 Bit Values: " << Constants.Num32BitValues << "\n";
594569
break;
595570
}
596571
case llvm::to_underlying(dxbc::RootParameterType::CBV):
597572
case llvm::to_underlying(dxbc::RootParameterType::UAV):
598573
case llvm::to_underlying(dxbc::RootParameterType::SRV): {
599574
const dxbc::RTS0::v2::RootDescriptor &Descriptor =
600575
RS.ParametersContainer.getRootDescriptor(Loc);
601-
OS << indent(Space + 2)
602-
<< "Register Space: " << Descriptor.RegisterSpace << "\n";
603-
OS << indent(Space + 2)
604-
<< "Shader Register: " << Descriptor.ShaderRegister << "\n";
576+
OS << indent(2) << "Register Space: " << Descriptor.RegisterSpace
577+
<< "\n";
578+
OS << indent(2) << "Shader Register: " << Descriptor.ShaderRegister
579+
<< "\n";
605580
if (RS.Version > 1)
606-
OS << indent(Space + 2) << "Flags: " << Descriptor.Flags << "\n";
581+
OS << indent(2) << "Flags: " << Descriptor.Flags << "\n";
607582
break;
608583
}
609584
case llvm::to_underlying(dxbc::RootParameterType::DescriptorTable): {
610585
const mcdxbc::DescriptorTable &Table =
611586
RS.ParametersContainer.getDescriptorTable(Loc);
612-
OS << indent(Space + 2) << "NumRanges: " << Table.Ranges.size() << "\n";
587+
OS << indent(2) << "NumRanges: " << Table.Ranges.size() << "\n";
613588

614589
for (const dxbc::RTS0::v2::DescriptorRange Range : Table) {
615-
OS << indent(Space + 2) << "- Range Type: " << Range.RangeType
616-
<< "\n";
617-
OS << indent(Space + 4) << "Register Space: " << Range.RegisterSpace
618-
<< "\n";
619-
OS << indent(Space + 4)
590+
OS << indent(2) << "- Range Type: " << Range.RangeType << "\n";
591+
OS << indent(4) << "Register Space: " << Range.RegisterSpace << "\n";
592+
OS << indent(4)
620593
<< "Base Shader Register: " << Range.BaseShaderRegister << "\n";
621-
OS << indent(Space + 4) << "Num Descriptors: " << Range.NumDescriptors
594+
OS << indent(4) << "Num Descriptors: " << Range.NumDescriptors
622595
<< "\n";
623-
OS << indent(Space + 4) << "Offset In Descriptors From Table Start: "
596+
OS << indent(4) << "Offset In Descriptors From Table Start: "
624597
<< Range.OffsetInDescriptorsFromTableStart << "\n";
625598
if (RS.Version > 1)
626-
OS << indent(Space + 4) << "Flags: " << Range.Flags << "\n";
599+
OS << indent(4) << "Flags: " << Range.Flags << "\n";
627600
}
628601
break;
629602
}
630603
}
631604
}
632-
OS << indent(Space) << "NumStaticSamplers: " << 0 << "\n";
633-
OS << indent(Space) << "StaticSamplersOffset: " << RS.StaticSamplersOffset
634-
<< "\n";
635-
636-
Space--;
637-
// end root signature header
605+
OS << "NumStaticSamplers: " << 0 << "\n";
606+
OS << "StaticSamplersOffset: " << RS.StaticSamplersOffset << "\n";
638607
}
639608
return PreservedAnalyses::all();
640609
}

0 commit comments

Comments
 (0)