[DirectX] Add DXIL_MODULE_FLAG for ShaderFlags. #83217
Conversation
|
@llvm/pr-subscribers-llvm-support @llvm/pr-subscribers-backend-directx Author: Xiang Li (python3kgae) ChangesAdd DXILConstants.def which stores DXILModuleFlags. Use DXILModuleFlags for ComputedShaderFlags instead of ShaderFeatureFlags. ComputedShaderFlags::getFeatureInfo() was added to get FeatureInfoFlags. Fixes #57925 Full diff: https://github.com/llvm/llvm-project/pull/83217.diff 13 Files Affected:
diff --git a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def
index 80ed86bc3a499e..28bb8ab3f14c4a 100644
--- a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def
+++ b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def
@@ -44,8 +44,11 @@ SHADER_FEATURE_FLAG(27, RESERVED, "<RESERVED>")
SHADER_FEATURE_FLAG(28, AtomicInt64OnHeapResource, "64-bit Atomics on Heap Resources")
SHADER_FEATURE_FLAG(29, AdvancedTextureOps, "Advanced Texture Ops")
SHADER_FEATURE_FLAG(30, WriteableMSAATextures, "Writeable MSAA Textures")
+SHADER_FEATURE_FLAG(31, WaveMMA, "Wave Matrix Multiply Accumulate")
+SHADER_FEATURE_FLAG(32, SampleCmpGradientOrBias, "SampleCmpGradient or SampleCmpLevelZeroBias")
+SHADER_FEATURE_FLAG(33, ExtendedCommandInfo, "StartVertexLocation and StartInstanceLocation")
-SHADER_FEATURE_FLAG(31, NextUnusedBit, "Next reserved shader flag bit (not a flag)")
+SHADER_FEATURE_FLAG(34, NextUnusedBit, "Next reserved shader flag bit (not a flag)")
#undef SHADER_FEATURE_FLAG
#endif
diff --git a/llvm/include/llvm/Support/DXILConstants.def b/llvm/include/llvm/Support/DXILConstants.def
new file mode 100644
index 00000000000000..e7e8fd7089505f
--- /dev/null
+++ b/llvm/include/llvm/Support/DXILConstants.def
@@ -0,0 +1,76 @@
+
+#ifdef DXIL_MODULE_FLAG
+
+// DXIL_MODULE_FLAG(bit offset for the flag, bit offset for feature flag (-1 for no feature flag), name, description.
+DXIL_MODULE_FLAG( 0, -1, DisableOptimizations, "D3D11_1_SB_GLOBAL_FLAG_SKIP_OPTIMIZATION")
+DXIL_MODULE_FLAG( 1, -1, DisableMathRefactoring, "D3D10_SB_GLOBAL_FLAG_REFACTORING_ALLOWED")
+DXIL_MODULE_FLAG( 2, 0, EnableDoublePrecision, "Double-precision floating point")
+DXIL_MODULE_FLAG( 3, -1, ForceEarlyDepthStencil, "D3D11_SB_GLOBAL_FLAG_FORCE_EARLY_DEPTH_STENCIL")
+
+DXIL_MODULE_FLAG( 4, 1, EnableRawAndStructuredBuffers, "Raw and Structured buffers")
+DXIL_MODULE_FLAG( 5, 4, LowPrecisionPresent, "Minimum-precision data types")
+DXIL_MODULE_FLAG( 6, 5, EnableDoubleExtensions, "Double-precision extensions for 11.1")
+DXIL_MODULE_FLAG( 7, 6, EnableMSAD, "Shader extensions for 11.1")
+
+DXIL_MODULE_FLAG( 8, 1, AllResourcesBound, "D3D12_SB_GLOBAL_FLAG_ALL_RESOURCES_BOUND")
+DXIL_MODULE_FLAG( 9, 13, ViewportAndRTArrayIndex, "SV_RenderTargetArrayIndex or SV_ViewportArrayIndex from any shader feeding rasterizer")
+DXIL_MODULE_FLAG(10, 10, InnerCoverage, "PS Inner Coverage")
+DXIL_MODULE_FLAG(11, 9, StencilRef, "PS Output Stencil Ref")
+
+DXIL_MODULE_FLAG(12, 8, TiledResources, "Tiled resources")
+DXIL_MODULE_FLAG(13, 11, UAVLoadAdditionalFormats, "Typed UAV Load Additional Formats")
+DXIL_MODULE_FLAG(14, 7, Level9ComparisonFiltering, "Comparison filtering for feature level 9")
+DXIL_MODULE_FLAG(15, 3, Max64UAVs, "64 UAV slots")
+
+DXIL_MODULE_FLAG(16, 2, UAVsAtEveryStage, "UAVs at every shader stage")
+DXIL_MODULE_FLAG(17, -1, CSRawAndStructuredViaShader4X, "SHADER_FEATURE_COMPUTE_SHADERS_PLUS_RAW_AND_STRUCTURED_BUFFERS_VIA_SHADER_4_X")
+DXIL_MODULE_FLAG(18, 12, ROVS, "Raster Ordered UAVs")
+DXIL_MODULE_FLAG(19, 14, WaveOps, "Wave level operations")
+
+DXIL_MODULE_FLAG(20, 15, Int64Ops, "64-Bit integer")
+
+// SM 6.1+
+DXIL_MODULE_FLAG(21, 16, ViewID, "View Instancing")
+DXIL_MODULE_FLAG(22, 17, Barycentrics, "Barycentrics")
+
+// SM 6.2+
+DXIL_MODULE_FLAG(23, 18, UseNativeLowPrecision, "Use native low precision")
+
+// SM 6.4+
+DXIL_MODULE_FLAG(24, 19, ShadingRate, "Shading Rate")
+
+// SM 6.5+
+DXIL_MODULE_FLAG(25, 20, RaytracingTier1_1, "Raytracing tier 1.1 features")
+DXIL_MODULE_FLAG(26, 21, SamplerFeedback, "Sampler feedback")
+
+// SM 6.6+
+DXIL_MODULE_FLAG(27, 22, AtomicInt64OnTypedResource, "64-bit Atomics on Typed Resources")
+
+DXIL_MODULE_FLAG(28, 23, AtomicInt64OnGroupShared, "64-bit Atomics on Group Shared")
+DXIL_MODULE_FLAG(29, 24, DerivativesInMeshAndAmpShaders, "Derivatives in mesh and amplification shaders")
+DXIL_MODULE_FLAG(30, 25, ResourceDescriptorHeapIndexing, "Resource descriptor heap indexing")
+DXIL_MODULE_FLAG(31, 26, SamplerDescriptorHeapIndexing, "Sampler descriptor heap indexing")
+
+DXIL_MODULE_FLAG(32, 28, AtomicInt64OnHeapResource, "64-bit Atomics on Heap Resources")
+
+// SM 6.7+
+// Global flag indicating that any UAV may not alias any other UAV.
+// Set if UAVs are used, unless -res-may-alias was specified.
+// For modules compiled against validator version < 1.7, this flag will be
+// cleared, and it must be assumed that UAV resources may alias.
+DXIL_MODULE_FLAG(33, -1, ResMayNotAlias, "Any UAV may not alias any other UAV")
+
+DXIL_MODULE_FLAG(34, 29, AdvancedTextureOps, "Advanced Texture Ops")
+DXIL_MODULE_FLAG(35, 30, WriteableMSAATextures, "Writeable MSAA Textures")
+
+// Experimental SM 6.9+ - Reserved, not yet supported.
+DXIL_MODULE_FLAG(36, 31, WaveMMA, "Wave Matrix Multiply Accumulate")
+
+// SM 6.8+
+DXIL_MODULE_FLAG(37, 32, SampleCmpGradientOrBias, "SampleCmpGradient or SampleCmpLevelZeroBias")
+DXIL_MODULE_FLAG(38, 33, ExtendedCommandInfo, "StartVertexLocation and StartInstanceLocation")
+DXIL_MODULE_FLAG(39, -1, UsesDerivatives, "SHADER_FEATURE_OPT_USES_DERIVATIVES")
+
+#undef DXIL_MODULE_FLAG
+#endif
+
diff --git a/llvm/lib/Target/DirectX/DXContainerGlobals.cpp b/llvm/lib/Target/DirectX/DXContainerGlobals.cpp
index 56063b487f6847..b716d12b3076f1 100644
--- a/llvm/lib/Target/DirectX/DXContainerGlobals.cpp
+++ b/llvm/lib/Target/DirectX/DXContainerGlobals.cpp
@@ -28,7 +28,7 @@ using namespace llvm::dxil;
namespace {
class DXContainerGlobals : public llvm::ModulePass {
- GlobalVariable *getShaderFlags(Module &M);
+ GlobalVariable *getShaderFeatureInfo(Module &M);
GlobalVariable *computeShaderHash(Module &M);
public:
@@ -53,21 +53,24 @@ class DXContainerGlobals : public llvm::ModulePass {
bool DXContainerGlobals::runOnModule(Module &M) {
llvm::SmallVector<GlobalValue *> Globals;
- Globals.push_back(getShaderFlags(M));
+ Globals.push_back(getShaderFeatureInfo(M));
Globals.push_back(computeShaderHash(M));
appendToCompilerUsed(M, Globals);
return true;
}
-GlobalVariable *DXContainerGlobals::getShaderFlags(Module &M) {
- const uint64_t Flags =
- (uint64_t)(getAnalysis<ShaderFlagsAnalysisWrapper>().getShaderFlags());
+GlobalVariable *DXContainerGlobals::getShaderFeatureInfo(Module &M) {
+ const uint64_t FeatureInfo =
+ (uint64_t)(getAnalysis<ShaderFlagsAnalysisWrapper>()
+ .getShaderFlags()
+ .getFeatureInfo());
- Constant *FlagsConstant = ConstantInt::get(M.getContext(), APInt(64, Flags));
- auto *GV = new llvm::GlobalVariable(M, FlagsConstant->getType(), true,
+ Constant *FeatureInfoConstant =
+ ConstantInt::get(M.getContext(), APInt(64, FeatureInfo));
+ auto *GV = new llvm::GlobalVariable(M, FeatureInfoConstant->getType(), true,
GlobalValue::PrivateLinkage,
- FlagsConstant, "dx.sfi0");
+ FeatureInfoConstant, "dx.sfi0");
GV->setSection("SFI0");
GV->setAlignment(Align(4));
return GV;
diff --git a/llvm/lib/Target/DirectX/DXILShaderFlags.cpp b/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
index 66a9dc46bcbfbf..487d691d64a6ba 100644
--- a/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
+++ b/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
@@ -23,14 +23,14 @@ using namespace llvm::dxil;
static void updateFlags(ComputedShaderFlags &Flags, const Instruction &I) {
Type *Ty = I.getType();
if (Ty->isDoubleTy()) {
- Flags.Doubles = true;
+ Flags.EnableDoublePrecision = true;
switch (I.getOpcode()) {
case Instruction::FDiv:
case Instruction::UIToFP:
case Instruction::SIToFP:
case Instruction::FPToUI:
case Instruction::FPToSI:
- Flags.DX11_1_DoubleExtensions = true;
+ Flags.EnableDoubleExtensions = true;
break;
}
}
@@ -51,10 +51,10 @@ void ComputedShaderFlags::print(raw_ostream &OS) const {
if (FlagVal == 0)
return;
OS << "; Note: shader requires additional functionality:\n";
-#define SHADER_FEATURE_FLAG(bit, FlagName, Str) \
+#define DXIL_MODULE_FLAG(bit, featureBit, FlagName, Str) \
if (FlagName) \
OS << "; " Str "\n";
-#include "llvm/BinaryFormat/DXContainerConstants.def"
+#include "llvm/Support/DXILConstants.def"
OS << ";\n";
}
diff --git a/llvm/lib/Target/DirectX/DXILShaderFlags.h b/llvm/lib/Target/DirectX/DXILShaderFlags.h
index 574a7b090f5281..8b69ebce9743e1 100644
--- a/llvm/lib/Target/DirectX/DXILShaderFlags.h
+++ b/llvm/lib/Target/DirectX/DXILShaderFlags.h
@@ -14,7 +14,6 @@
#ifndef LLVM_TARGET_DIRECTX_DXILSHADERFLAGS_H
#define LLVM_TARGET_DIRECTX_DXILSHADERFLAGS_H
-#include "llvm/BinaryFormat/DXContainer.h"
#include "llvm/IR/PassManager.h"
#include "llvm/Pass.h"
#include "llvm/Support/Compiler.h"
@@ -29,20 +28,31 @@ class GlobalVariable;
namespace dxil {
struct ComputedShaderFlags {
-#define SHADER_FEATURE_FLAG(bit, FlagName, Str) bool FlagName : 1;
-#include "llvm/BinaryFormat/DXContainerConstants.def"
+#define DXIL_MODULE_FLAG(bit, featureBit, FlagName, Str) bool FlagName : 1;
+#include "llvm/Support/DXILConstants.def"
-#define SHADER_FEATURE_FLAG(bit, FlagName, Str) FlagName = false;
+#define DXIL_MODULE_FLAG(bit, featureBit, FlagName, Str) FlagName = false;
ComputedShaderFlags() {
-#include "llvm/BinaryFormat/DXContainerConstants.def"
+#include "llvm/Support/DXILConstants.def"
+ }
+ static uint64_t getFeatureInfoMask(int featureBit) {
+ return featureBit != -1 ? (uint64_t)1 << featureBit : 0ull;
+ }
+ uint64_t getFeatureInfo() const {
+ uint64_t FeatureInfo = 0;
+#define DXIL_MODULE_FLAG(bit, featureBit, FlagName, Str) \
+ FeatureInfo |= FlagName ? getFeatureInfoMask(featureBit) : 0ull;
+
+#include "llvm/Support/DXILConstants.def"
+
+ return FeatureInfo;
}
operator uint64_t() const {
uint64_t FlagValue = 0;
-#define SHADER_FEATURE_FLAG(bit, FlagName, Str) \
- FlagValue |= \
- FlagName ? static_cast<uint64_t>(dxbc::FeatureFlags::FlagName) : 0ull;
-#include "llvm/BinaryFormat/DXContainerConstants.def"
+#define DXIL_MODULE_FLAG(bit, featureBit, FlagName, Str) \
+ FlagValue |= FlagName ? (uint64_t)1 << bit : 0ull;
+#include "llvm/Support/DXILConstants.def"
return FlagValue;
}
diff --git a/llvm/test/CodeGen/DirectX/ShaderFlags/double-extensions.ll b/llvm/test/CodeGen/DirectX/ShaderFlags/double-extensions.ll
index 865fefeac335dd..687e0036db78f2 100644
--- a/llvm/test/CodeGen/DirectX/ShaderFlags/double-extensions.ll
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/double-extensions.ll
@@ -3,7 +3,7 @@
target triple = "dxil-pc-shadermodel6.7-library"
-; CHECK: ; Shader Flags Value: 0x00000021
+; CHECK: ; Shader Flags Value: 0x00000044
; CHECK: ; Note: shader requires additional functionality:
; CHECK-NEXT: ; Double-precision floating point
; CHECK-NEXT: ; Double-precision extensions for 11.1
diff --git a/llvm/test/CodeGen/DirectX/ShaderFlags/doubles.ll b/llvm/test/CodeGen/DirectX/ShaderFlags/doubles.ll
index f90db61661f09f..70b1ac798b15e2 100644
--- a/llvm/test/CodeGen/DirectX/ShaderFlags/doubles.ll
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/doubles.ll
@@ -3,10 +3,10 @@
target triple = "dxil-pc-shadermodel6.7-library"
-; CHECK: ; Shader Flags Value: 0x00000001
+; CHECK: ; Shader Flags Value: 0x00000004
; CHECK: ; Note: shader requires additional functionality:
; CHECK-NEXT: ; Double-precision floating point
-; CHECK-NEXT: {{^;$}}
+
define double @add(double %a, double %b) {
%sum = fadd double %a, %b
ret double %sum
diff --git a/llvm/test/CodeGen/DirectX/lib_entry.ll b/llvm/test/CodeGen/DirectX/lib_entry.ll
index 9208d6d3f32246..5254a088055888 100644
--- a/llvm/test/CodeGen/DirectX/lib_entry.ll
+++ b/llvm/test/CodeGen/DirectX/lib_entry.ll
@@ -7,7 +7,7 @@ target triple = "dxil-unknown-shadermodel6.7-library"
; Make sure generate empty entry for lib profile.
;CHECK:![[empty_entry]] = !{null, !"", null, null, ![[shader_flags:[0-9]+]]}
; Make sure double is marked for shader flags.
-;CHECK:![[shader_flags]] = !{i32 0, i64 1}
+;CHECK:![[shader_flags]] = !{i32 0, i64 4}
;CHECK:![[entry]] = !{ptr @entry, !"entry", null, null, ![[extra:[0-9]+]]}
;CHECK:![[extra]] = !{i32 8, i32 5, i32 4, ![[numthreads:[0-9]+]]}
;CHECK:![[numthreads]] = !{i32 1, i32 2, i32 1}
diff --git a/llvm/test/ObjectYAML/DXContainer/DomainMaskVectors.yaml b/llvm/test/ObjectYAML/DXContainer/DomainMaskVectors.yaml
index 713fbc61e094b5..dbf93149c06606 100644
--- a/llvm/test/ObjectYAML/DXContainer/DomainMaskVectors.yaml
+++ b/llvm/test/ObjectYAML/DXContainer/DomainMaskVectors.yaml
@@ -44,6 +44,9 @@ Parts:
AtomicInt64OnHeapResource: false
AdvancedTextureOps: false
WriteableMSAATextures: false
+ WaveMMA: false
+ SampleCmpGradientOrBias: false
+ ExtendedCommandInfo: false
NextUnusedBit: false
- Name: ISG1
Size: 52
diff --git a/llvm/test/ObjectYAML/DXContainer/GeometryMaskVectors.yaml b/llvm/test/ObjectYAML/DXContainer/GeometryMaskVectors.yaml
index bf29d0866b3628..b27b37eb86af2d 100644
--- a/llvm/test/ObjectYAML/DXContainer/GeometryMaskVectors.yaml
+++ b/llvm/test/ObjectYAML/DXContainer/GeometryMaskVectors.yaml
@@ -44,6 +44,9 @@ Parts:
AtomicInt64OnHeapResource: false
AdvancedTextureOps: false
WriteableMSAATextures: false
+ WaveMMA: false
+ SampleCmpGradientOrBias: false
+ ExtendedCommandInfo: false
NextUnusedBit: false
- Name: ISG1
Size: 120
diff --git a/llvm/test/ObjectYAML/DXContainer/HullMaskVectors.yaml b/llvm/test/ObjectYAML/DXContainer/HullMaskVectors.yaml
index 0eaa2f8e97fb99..e997ca30350774 100644
--- a/llvm/test/ObjectYAML/DXContainer/HullMaskVectors.yaml
+++ b/llvm/test/ObjectYAML/DXContainer/HullMaskVectors.yaml
@@ -44,6 +44,9 @@ Parts:
AtomicInt64OnHeapResource: false
AdvancedTextureOps: false
WriteableMSAATextures: false
+ WaveMMA: false
+ SampleCmpGradientOrBias: false
+ ExtendedCommandInfo: false
NextUnusedBit: false
- Name: ISG1
Size: 60
diff --git a/llvm/test/tools/obj2yaml/DXContainer/ShaderFlags.yaml b/llvm/test/tools/obj2yaml/DXContainer/ShaderFlags.yaml
index 165b397c145771..73707356608afb 100644
--- a/llvm/test/tools/obj2yaml/DXContainer/ShaderFlags.yaml
+++ b/llvm/test/tools/obj2yaml/DXContainer/ShaderFlags.yaml
@@ -44,6 +44,9 @@ Parts:
AtomicInt64OnHeapResource: false
AdvancedTextureOps: true
WriteableMSAATextures: false
+ WaveMMA: false
+ SampleCmpGradientOrBias: false
+ ExtendedCommandInfo: false
NextUnusedBit: true
- Name: FKE0
Size: 8
@@ -84,4 +87,7 @@ Parts:
# CHECK-NEXT: AtomicInt64OnHeapResource: false
# CHECK-NEXT: AdvancedTextureOps: true
# CHECK-NEXT: WriteableMSAATextures: false
+# CHECK-NEXT: WaveMMA: false
+# CHECK-NEXT: SampleCmpGradientOrBias: false
+# CHECK-NEXT: ExtendedCommandInfo: false
# CHECK-NEXT: NextUnusedBit: true
diff --git a/llvm/test/tools/obj2yaml/DXContainer/ShaderFlagsEmpty.yaml b/llvm/test/tools/obj2yaml/DXContainer/ShaderFlagsEmpty.yaml
index 086c7e90dcd5fe..4f8a080774c25c 100644
--- a/llvm/test/tools/obj2yaml/DXContainer/ShaderFlagsEmpty.yaml
+++ b/llvm/test/tools/obj2yaml/DXContainer/ShaderFlagsEmpty.yaml
@@ -43,6 +43,9 @@ Parts:
AtomicInt64OnHeapResource: false
AdvancedTextureOps: false
WriteableMSAATextures: false
+ WaveMMA: false
+ SampleCmpGradientOrBias: false
+ ExtendedCommandInfo: false
NextUnusedBit: false
- Name: FKE0
Size: 8
|
| @@ -28,7 +28,7 @@ using namespace llvm::dxil; | |||
| namespace { | |||
| class DXContainerGlobals : public llvm::ModulePass { | |||
|
|
|||
| GlobalVariable *getShaderFlags(Module &M); | |||
| GlobalVariable *getShaderFeatureInfo(Module &M); | |||
There was a problem hiding this comment.
This name is confusing. Why are we calling a flag encoding "feature info"?
There was a problem hiding this comment.
So just getFeatureInfo(Module &M)?
There was a problem hiding this comment.
"feature info" doesn't mean anything to me when I read the code. The GlobalVaraible returned here, and the uint64_t returned in other places they aren't a "feature info". They are "feature flags".
There was a problem hiding this comment.
Changed to getFeatureFlags.
python3kgae
force-pushed
the
dxil_module_flag
branch
from
4908b49
to
8b123b2
Compare
| @@ -29,23 +28,35 @@ class GlobalVariable; | |||
| namespace dxil { | |||
|
|
|||
| struct ComputedShaderFlags { | |||
| #define SHADER_FEATURE_FLAG(bit, FlagName, Str) bool FlagName : 1; | |||
| #define SHADER_FEATURE_FLAG(bit, DxilModuleBit, FlagName, Str) \ | |||
There was a problem hiding this comment.
The name of this parameter changes here, which is a little confusing
| @@ -51,7 +51,10 @@ void ComputedShaderFlags::print(raw_ostream &OS) const { | |||
| if (FlagVal == 0) | |||
| return; | |||
| OS << "; Note: shader requires additional functionality:\n"; | |||
| #define SHADER_FEATURE_FLAG(bit, FlagName, Str) \ | |||
| #define SHADER_FEATURE_FLAG(bit, DxilModuleNum, FlagName, Str) \ | |||
There was a problem hiding this comment.
| #define SHADER_FEATURE_FLAG(bit, DxilModuleNum, FlagName, Str) \ | |
| #define SHADER_FEATURE_FLAG(bit, DxilModuleBit, FlagName, Str) \ |
| @@ -28,7 +28,7 @@ using namespace llvm::dxil; | |||
| namespace { | |||
| class DXContainerGlobals : public llvm::ModulePass { | |||
|
|
|||
| GlobalVariable *getShaderFlags(Module &M); | |||
| GlobalVariable *getShaderFeatureInfo(Module &M); | |||
There was a problem hiding this comment.
"feature info" doesn't mean anything to me when I read the code. The GlobalVaraible returned here, and the uint64_t returned in other places they aren't a "feature info". They are "feature flags".
| @@ -51,7 +51,10 @@ void ComputedShaderFlags::print(raw_ostream &OS) const { | |||
| if (FlagVal == 0) | |||
| return; | |||
| OS << "; Note: shader requires additional functionality:\n"; | |||
| #define SHADER_FEATURE_FLAG(bit, FlagName, Str) \ | |||
| #define SHADER_FEATURE_FLAG(bit, DxilModuleNum, FlagName, Str) \ | |||
There was a problem hiding this comment.
Your suggestion is good, but also please fix the capitalization on the first argument:
| #define SHADER_FEATURE_FLAG(bit, DxilModuleNum, FlagName, Str) \ | |
| #define SHADER_FEATURE_FLAG(Bit, DxilModuleBit, FlagName, Str) \ |
| #define SHADER_FEATURE_FLAG(bit, DxilModuleNum, FlagName, Str) \ | ||
| if (FlagName) \ | ||
| OS << "; " Str "\n"; | ||
| #define DXIL_MODULE_FLAG(bit, FlagName, Str) \ |
There was a problem hiding this comment.
| #define DXIL_MODULE_FLAG(bit, FlagName, Str) \ | |
| #define DXIL_MODULE_FLAG(Bit, FlagName, Str) \ |
| @@ -29,23 +28,35 @@ class GlobalVariable; | |||
| namespace dxil { | |||
|
|
|||
| struct ComputedShaderFlags { | |||
| #define SHADER_FEATURE_FLAG(bit, FlagName, Str) bool FlagName : 1; | |||
| #define SHADER_FEATURE_FLAG(bit, DxilModuleBit, FlagName, Str) \ | |||
There was a problem hiding this comment.
| #define SHADER_FEATURE_FLAG(bit, DxilModuleBit, FlagName, Str) \ | |
| #define SHADER_FEATURE_FLAG(Bit, DxilModuleBit, FlagName, Str) \ |
Change FeatureInfo to FeatureFlags.
| SHADER_FEATURE_FLAG(15, 20, Int64Ops, "64-Bit integer") | ||
| SHADER_FEATURE_FLAG(16, 21, ViewID, "View Instancing") | ||
| SHADER_FEATURE_FLAG(17, 22, Barycentrics, "Barycentrics") | ||
| SHADER_FEATURE_FLAG(18, 23, NativeLowPrecision, "Use native low precision") |
There was a problem hiding this comment.
NativeLowPrecision, and MinimumPrecision, are not 1:1 mappings between module flags and feature flags. DxilModuleFlags has LowPrecisionPresent and UseNativeLowPrecision, which are used together to set the feature flags. Neither are set if LowPrecisionPresent is unset, and one is set if LowPrecisionPresent is set, depending on the state of UseNativeLowPrecision.
DXC's code:
Flags |= m_bLowPrecisionPresent && !m_bUseNativeLowPrecision
? hlsl::DXIL::ShaderFeatureInfo_MinimumPrecision
: 0;
Flags |= m_bLowPrecisionPresent && m_bUseNativeLowPrecision
? hlsl::DXIL::ShaderFeatureInfo_NativeLowPrecision
: 0;There was a problem hiding this comment.
Added separate DXIL_MODULE_FLAG.
| #define SHADER_FEATURE_FLAG(Bit, DxilModuleBit, FlagName, Str) \ | ||
| bool FlagName : 1; | ||
| #define DXIL_MODULE_FLAG(Bit, FlagName, Str) bool FlagName : 1; |
There was a problem hiding this comment.
In these macros, could we name the flags more clearly? For instance:
| #define SHADER_FEATURE_FLAG(Bit, DxilModuleBit, FlagName, Str) \ | |
| bool FlagName : 1; | |
| #define DXIL_MODULE_FLAG(Bit, FlagName, Str) bool FlagName : 1; | |
| #define SHADER_FEATURE_FLAG(FeatureBit, DxilModuleBit, FlagName, Str) \ | |
| bool FlagName : 1; | |
| #define DXIL_MODULE_FLAG(DxilModuleBit, FlagName, Str) bool FlagName : 1; |
| SHADER_FEATURE_FLAG(30, WriteableMSAATextures, "Writeable MSAA Textures") | ||
|
|
||
| SHADER_FEATURE_FLAG(31, NextUnusedBit, "Next reserved shader flag bit (not a flag)") | ||
| // SHADER_FEATURE_FLAG(bit offset for the flag, bit offset for DXIL module flag, name, description. |
There was a problem hiding this comment.
bit offset for the flag - in what context? This is the offset in the Shader Feature Info flags (in SFI0 part) or per-function FeatureInfo* flags (in RDAT part). Should this read bit offset for the shader feature info flag?
Next, I'm not sure DXIL module flag is the right term for all of these, since most of them are computed on a per-function or shader basis, and the resulting feature flags are preserved per-function for libraries in RDAT. These are unioned into module-level shader flags, but they aren't all strictly module flags. These are previously referred to as the DXIL Shader Flags, with the 64-bit flags value referred to as the raw shader flags.
Some of the flags are module-scope flags though, so it would seem we need to clarify the distinctions, or this will just be more confusing. It would seem important to preserve true module-level flags/settings from CodeGen on, separately from the flags computed based on function usage later. We should have a term for this subset of flags, separate from the raw shader flags, and something like DXIL module flags or DXIL module settings (if there are more than just flags) would seem appropriate for these. Grouping them can make it easier to check for consistent settings between modules when linking, for instance.
These flags are part of the shader interface:
- Feature Flags, aka. Shader Feature Info flags encoded into the container's
SFI0part, or per-function into theRDATpart. This is part of the runtime interface, used to validate required feature support at runtime. Many, but not all, of these flags have a 1:1 mapping from DxilShaderFlags. - DXIL Shader Flags: all the flags including module flags and per-function/shader computed flags indicating feature usage. This is a 64-bit value that is part of the driver interface. This is what's being referred to as DXIL module flags here.
You could think of DxilShaderFlags as breaking down into:
- Global module flags indicating module interpretation, or allowed optimizations, such as
UseNativeLowPrecision,AllResourcesBound,DisableMathRefactoring, etc... These are set based on compilation options. - Shader flags computed from functions/entry points or global state (or ComputedShaderFlags):
- most indicating a required feature flag like most of the flags defined with
SHADER_FEATURE_FLAGhere - some potentially indicating something for the driver, without mapping to a required feature bit, such as
ForceEarlyDepthStencil - others potentially indicating usage of a feature in a function that may restrict the context from which that function may be called.
- most indicating a required feature flag like most of the flags defined with
I don't think this explanation gives a definitive answer to how we should clarify our flags terminology and organization, but I don't think we have it quite right at this time.
There was a problem hiding this comment.
How about adding something like FlagKind to these SHADER_FEATURE_FLAG/DXIL_SHADER_FLAG ?
The kind could be FEATURE_INFO, GLOBAL_OPTION or FUNCTION.
There was a problem hiding this comment.
Will adding a FlagKind for SHADER_FEATURE_FLAG/DXIL_MODULE_FLAG make things clearer and solve the problem?
There was a problem hiding this comment.
I'm not sure that's solving the issue. I'm trying to think about a breakdown design that results in consistent terminology, and exposes the important distinctions.
What do you think of this?
Terminology:
- "DXIL Module Flags" - flag values used in DXIL metadata - could these be called "DXIL Shader Flags"?
- "Shader Feature Flags" - flags used in shader feature info (SFI0) part and FeatureInfo fields in RDAT part - Could we call these "Shader Feature Info Flags"? It's a bit longer, but clarifies their connection with something more specific.
- Option flags - we could refer to these simply as "Option Flags". These are a subset of "DXIL Shader Flags".
- Computed flags - we could refer to these as "Computed Shader Flags". These are a subset of "DXIL Shader Flags", most of which are reflected directly into Shader Feature Info flags, some indirectly (like min-precision), and some not at all.
With this terminology, we could use three different macros for defining the flags:
SHADER_FEATURE_INFO_FLAG- also defines correspondingDXIL_SHADER_COMPUTED_FLAGbit for ones with 1:1 correspondence (not requiring that separate declaration, like it is now), otherwise -1. If -1, we need another way to compute this flag from the DXIL Shader Flags. If we allowed an expression to be supplied (perhaps with a different variant of the macro), it could be computed using that expression. This could solve the min-precision and low-precision feature flag cases.DXIL_SHADER_OPTION_FLAG- option flag, not computed from usage. Copied from module when initializing flags for computation per-function, compared when checking linking compatibility, etc... Flags could be module scope or per-function or entry point scope if needed.DXIL_SHADER_COMPUTED_FLAG- for additional computed flags that do not have 1:1 correspondence toSHADER_FEATURE_INFO_FLAG.
Potential definition for the SHADER_FEATURE_INFO_FLAG variant computed from DXIL shader flags with expression:
// SHADER_FEATURE_FLAG2(bit offset for the shader info flag, expression based on DXIL shader flags, name, description)
#ifndef SHADER_FEATURE_INFO_FLAG2
#define SHADER_FEATURE_INFO_FLAG2(FeatureInfoBit, ShaderFlagExpr, FlagName, Str) \
SHADER_FEATURE_INFO_FLAG(FeatureInfoBit, -1, FlagName, Str)
#endif
Maybe these should all be defined in a TableGen file instead. What do you think about that?
There was a problem hiding this comment.
I've been arguing against the phrase "feature info" because it seems nonsensical to me. The word "info" in that conveys no meaning and just adds an extra word that seems out of place. It really feels like that name came about because we use four characters for the part names and needed to fill space rather than a concise and meaningful term.
I don't think we should use TableGen for this. Writing a TableGen backend for this adds a lot more complexity than we need for these fairly straightforward expansions.
| // Only save DXIL module flags which not map to feature flags here. | ||
| DXIL_MODULE_FLAG( 0, DisableOptimizations, "D3D11_1_SB_GLOBAL_FLAG_SKIP_OPTIMIZATION") | ||
| DXIL_MODULE_FLAG( 1, DisableMathRefactoring, "D3D10_SB_GLOBAL_FLAG_REFACTORING_ALLOWED") | ||
| DXIL_MODULE_FLAG( 3, ForceEarlyDepthStencil, "D3D11_SB_GLOBAL_FLAG_FORCE_EARLY_DEPTH_STENCIL") |
There was a problem hiding this comment.
The *_SB_GLOBAL_FLAG_* values were carried over from DXBC, and were global for a particular shader. Most of these can be considered global module-level flags, but not ForceEarlyDepthStencil, which applies to a particular pixel shader only (which only makes a difference for libraries). However, we could also potentially support some at a more granular level, such as DisableOptimizations or DisableMathRefactoring, though more granular settings for these could be expressed differently.
That said, there are ones missing from this list, and there may be overlap between flags that could be exposed through feature flags or global flags corresponding to DXBC's *_SB_GLOBAL_FLAG_*, such as DX11_1_ShaderExtensions.
There was a problem hiding this comment.
DX11_1_ShaderExtension is marked as map to DxilModuleFlag bit offset 7.
| #define SHADER_FEATURE_FLAG(Bit, DxilModuleNum, FlagName, Str) \ | ||
| if (FlagName) \ | ||
| OS << "; " Str "\n"; | ||
| #define DXIL_MODULE_FLAG(Bit, FlagName, Str) \ |
There was a problem hiding this comment.
I don't think module flags should be listed as additional functionality required, but it could still be useful to list them, without implying that they are optional feature requirements.
| DXIL_MODULE_FLAG( 1, DisableMathRefactoring, "D3D10_SB_GLOBAL_FLAG_REFACTORING_ALLOWED") | ||
| DXIL_MODULE_FLAG( 3, ForceEarlyDepthStencil, "D3D11_SB_GLOBAL_FLAG_FORCE_EARLY_DEPTH_STENCIL") | ||
| DXIL_MODULE_FLAG( 8, AllResourcesBound, "D3D12_SB_GLOBAL_FLAG_ALL_RESOURCES_BOUND") | ||
| DXIL_MODULE_FLAG(17, CSRawAndStructuredViaShader4X, "SHADER_FEATURE_COMPUTE_SHADERS_PLUS_RAW_AND_STRUCTURED_BUFFERS_VIA_SHADER_4_X") |
There was a problem hiding this comment.
CSRawAndStructuredViaShader4X is a shader feature flag, isn't it? What's it doing here? Did this get mixed up with the EnableRawAndStructuredBuffers flag corresponding to D3D11_SB_GLOBAL_FLAG_ENABLE_RAW_AND_STRUCTURED_BUFFERS that belongs here instead?
There was a problem hiding this comment.
Good catch.
Fixed.
| // SHADER_FEATURE_FLAG(bit offset for the flag, bit offset for DXIL module flag, name, description. | ||
|
|
||
| SHADER_FEATURE_FLAG(0, 2, Doubles, "Double-precision floating point") | ||
| SHADER_FEATURE_FLAG(1, 4, ComputeShadersPlusRawAndStructuredBuffers, "Raw and Structured buffers") |
There was a problem hiding this comment.
This should be the feature flag CSRawAndStructuredViaShader4X that's mistakenly listed under global flags instead.
Print DXIL module flag in ComputedShaderFlags::print too.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
| (uint64_t)(getAnalysis<ShaderFlagsAnalysisWrapper>().getShaderFlags()); | ||
| GlobalVariable *DXContainerGlobals::getFeatureFlags(Module &M) { | ||
| const uint64_t FeatureFlags = | ||
| (uint64_t)(getAnalysis<ShaderFlagsAnalysisWrapper>() |
| #define SHADER_FEATURE_FLAG(bit, FlagName, Str) \ | ||
| #define SHADER_FEATURE_FLAG(FeatureBit, DxilModuleNum, FlagName, Str) \ | ||
| if (FlagName) \ | ||
| OS << "; " Str "\n"; |
There was a problem hiding this comment.
i'm not sure what the llvm equivalent is but seems like we should be doing something like std::setw.
There was a problem hiding this comment.
Fixed with raw_ostream::indent.
There was a problem hiding this comment.
Fixed with raw_ostream::indent.
| @@ -9,47 +9,64 @@ CONTAINER_PART(OSG1) | |||
| CONTAINER_PART(PSG1) | |||
|
|
|||
| #undef CONTAINER_PART | |||
| #endif | |||
| #endif | |||
There was a problem hiding this comment.
maybe a comment here like // CONTAINER_PART
Use static_cast for cast to uint64_t.
Add DXILConstants.def which stores DXILModuleFlags.
Use DXILModuleFlags for ComputedShaderFlags instead of ShaderFeatureFlags.
ComputedShaderFlags::getFeatureInfo() was added to get FeatureInfoFlags.
Fixes #57925