From fa72b3bae503d1cd6930716552becafc8be319f1 Mon Sep 17 00:00:00 2001 From: Simon Moll Date: Thu, 24 Apr 2025 17:03:40 +0200 Subject: [PATCH 1/7] [SER] Validate 'reordercoherent' resource property Validates: All resources All instructions using resources Rules: 'reordercoherent' may only be used in SM6.9+ in resource handles and resource declarations. Increment/DecrementCounter unsupported on 'reordercoherent' resources. Create a new DXIL 1.9 variant of the 'CompileWhenOkThenCheckRDAT' container test and restore the old one without 'reordercoherent' (pre-#7250). The validator now rejects 'reordercoherent' in DXIL 1.3 and accepts from DXIL 1.9+. SER implementation tracker: #7214 --- docs/DXIL.rst | 3 +- lib/DxilValidation/DxilValidation.cpp | 23 ++- .../ser_reordercoherent_invalid_incdec.ll | 92 +++++++++++ .../ser_reordercoherent_invalid_sm.ll | 83 ++++++++++ .../unittests/HLSL/DxilContainerTest.cpp | 143 +++++++++++++++++- utils/hct/hctdb.py | 8 +- 6 files changed, 343 insertions(+), 9 deletions(-) create mode 100644 tools/clang/test/LitDXILValidation/ser_reordercoherent_invalid_incdec.ll create mode 100644 tools/clang/test/LitDXILValidation/ser_reordercoherent_invalid_sm.ll diff --git a/docs/DXIL.rst b/docs/DXIL.rst index 69bcae8c53..0b71db41d6 100644 --- a/docs/DXIL.rst +++ b/docs/DXIL.rst @@ -3175,6 +3175,7 @@ INSTR.OPCONSTRANGE Constant values must be in INSTR.OPERANDRANGE DXIL intrinsic operand must be within defined range INSTR.PARAMMULTIPLE Parameter must be a valid multiple INSTR.PTRBITCAST Pointer type bitcast must be have same size. +INSTR.REORDERCOHERENTREQUIRESSM69 reordercoherent requires SM 6.9 or later. INSTR.RESOURCECLASSFORLOAD load can only run on UAV/SRV resource. INSTR.RESOURCECLASSFORSAMPLERGATHER sample, lod and gather should be on srv resource. INSTR.RESOURCECLASSFORUAVSTORE store should be on uav resource. @@ -3216,6 +3217,7 @@ META.BARYCENTRICSTWOPERSPECTIVES There can only be up to tw META.BRANCHFLATTEN Can't use branch and flatten attributes together. META.CLIPCULLMAXCOMPONENTS Combined elements of SV_ClipDistance and SV_CullDistance must fit in 8 components META.CLIPCULLMAXROWS Combined elements of SV_ClipDistance and SV_CullDistance must fit in two rows. +META.COHERENCENOTONAPPENDCONSUME %0coherent cannot be used with append/consume buffers: '%1'. META.COMPUTEWITHNODE Compute entry must not have node metadata META.CONTROLFLOWHINTNOTONCONTROLFLOW Control flow hint only works on control flow inst. META.DENSERESIDS Resource identifiers must be zero-based and dense. @@ -3223,7 +3225,6 @@ META.DUPLICATESYSVALUE System value may only appe META.ENTRYFUNCTION entrypoint not found. META.FLAGSUSAGE Flags must match usage. META.FORCECASEONSWITCH Attribute forcecase only works for switch. -META.GLCNOTONAPPENDCONSUME globallycoherent cannot be used with append/consume buffers: '%0'. META.INTEGERINTERPMODE Interpolation mode on integer must be Constant META.INTERPMODEINONEROW Interpolation mode must be identical for all elements packed into the same row. META.INTERPMODEVALID Interpolation mode must be valid diff --git a/lib/DxilValidation/DxilValidation.cpp b/lib/DxilValidation/DxilValidation.cpp index bd69cdaf5d..b4028935e5 100644 --- a/lib/DxilValidation/DxilValidation.cpp +++ b/lib/DxilValidation/DxilValidation.cpp @@ -165,7 +165,8 @@ ValidateSignatureAccess(Instruction *I, DxilSignature &Sig, Value *SigId, static DxilResourceProperties GetResourceFromHandle(Value *Handle, ValidationContext &ValCtx) { - if (!isa(Handle)) { + CallInst *HandleCall = dyn_cast(Handle); + if (!HandleCall) { if (Instruction *I = dyn_cast(Handle)) ValCtx.EmitInstrError(I, ValidationRule::InstrHandleNotFromCreateHandle); else @@ -179,6 +180,11 @@ static DxilResourceProperties GetResourceFromHandle(Value *Handle, ValCtx.EmitInstrError(cast(Handle), ValidationRule::InstrHandleNotFromCreateHandle); } + if (RP.Basic.IsReorderCoherent && + !ValCtx.DxilMod.GetShaderModel()->IsSM69Plus()) { + ValCtx.EmitInstrError(HandleCall, + ValidationRule::InstrReorderCoherentRequiresSM69); + } return RP; } @@ -4182,6 +4188,9 @@ static void ValidateResourceOverlap( static void ValidateResource(hlsl::DxilResource &Res, ValidationContext &ValCtx) { + if (Res.IsReorderCoherent() && !ValCtx.DxilMod.GetShaderModel()->IsSM69Plus()) + ValCtx.EmitResourceError(&Res, + ValidationRule::InstrReorderCoherentRequiresSM69); switch (Res.GetKind()) { case DXIL::ResourceKind::RawBuffer: case DXIL::ResourceKind::TypedBuffer: @@ -4413,10 +4422,14 @@ static void ValidateResources(ValidationContext &ValCtx) { ValCtx.EmitResourceError(Uav.get(), ValidationRule::SmCounterOnlyOnStructBuf); } - if (Uav->HasCounter() && Uav->IsGloballyCoherent()) - ValCtx.EmitResourceFormatError(Uav.get(), - ValidationRule::MetaGlcNotOnAppendConsume, - {ValCtx.GetResourceName(Uav.get())}); + const bool UavIsCoherent = + Uav->IsGloballyCoherent() || Uav->IsReorderCoherent(); + if (Uav->HasCounter() && UavIsCoherent) { + StringRef Prefix = Uav->IsGloballyCoherent() ? "globally" : "reorder"; + ValCtx.EmitResourceFormatError( + Uav.get(), ValidationRule::MetaCoherenceNotOnAppendConsume, + {Prefix, ValCtx.GetResourceName(Uav.get())}); + } ValidateResource(*Uav, ValCtx); ValidateResourceOverlap(*Uav, UavAllocator, ValCtx); diff --git a/tools/clang/test/LitDXILValidation/ser_reordercoherent_invalid_incdec.ll b/tools/clang/test/LitDXILValidation/ser_reordercoherent_invalid_incdec.ll new file mode 100644 index 0000000000..aea6e807d4 --- /dev/null +++ b/tools/clang/test/LitDXILValidation/ser_reordercoherent_invalid_incdec.ll @@ -0,0 +1,92 @@ +; REQUIRES: dxil-1-9 +; RUN: not %dxv %s 2>&1 | FileCheck %s + +; COM: Original HLSL source: +; COM: reordercoherent RWStructuredBuffer buffer; +; COM: +; COM: +; COM: [Shader("raygeneration")] +; COM: void +; COM: main() +; COM: { +; COM: buffer.IncrementCounter(); +; COM: buffer.DecrementCounter(); +; COM: } + +; CHECK: error: reordercoherent cannot be used with append/consume buffers: 'buffer'. 'buffer' +; CHECK-NEXT: Validation failed. + +; shader hash: 638950814a9023bf537d61dbb330a4c8 +; +; Buffer Definitions: +; +; Resource bind info for buffer +; { +; +; float $Element; ; Offset: 0 Size: 4 +; +; } +; +; +; Resource Bindings: +; +; Name Type Format Dim ID HLSL Bind Count +; ------------------------------ ---------- ------- ----------- ------- -------------- ------ +; buffer UAV struct r/w+cnt U0u4294967295,space4294967295 1 +; +target datalayout = "e-m:e-p:32:32-i1:32-i8:32-i16:32-i32:32-i64:64-f16:32-f32:32-f64:64-n8:16:32:64" +target triple = "dxil-ms-dx" + +%dx.types.Handle = type { i8* } +%dx.types.ResourceProperties = type { i32, i32 } +%"class.RWStructuredBuffer" = type { float } + +@"\01?buffer@@3V?$RWStructuredBuffer@M@@A" = external constant %dx.types.Handle, align 4 + +; Function Attrs: nounwind +define void @"\01?main@@YAXXZ"() #0 { + %1 = load %dx.types.Handle, %dx.types.Handle* @"\01?buffer@@3V?$RWStructuredBuffer@M@@A", align 4 + %2 = call %dx.types.Handle @dx.op.createHandleForLib.dx.types.Handle(i32 160, %dx.types.Handle %1) ; CreateHandleForLib(Resource) + %3 = call %dx.types.Handle @dx.op.annotateHandle(i32 216, %dx.types.Handle %2, %dx.types.ResourceProperties { i32 102412, i32 4 }) ; AnnotateHandle(res,props) resource: reordercoherent RWStructuredBuffer + %4 = call i32 @dx.op.bufferUpdateCounter(i32 70, %dx.types.Handle %3, i8 1) ; BufferUpdateCounter(uav,inc) + %5 = call %dx.types.Handle @dx.op.createHandleForLib.dx.types.Handle(i32 160, %dx.types.Handle %1) ; CreateHandleForLib(Resource) + %6 = call %dx.types.Handle @dx.op.annotateHandle(i32 216, %dx.types.Handle %5, %dx.types.ResourceProperties { i32 102412, i32 4 }) ; AnnotateHandle(res,props) resource: reordercoherent RWStructuredBuffer + %7 = call i32 @dx.op.bufferUpdateCounter(i32 70, %dx.types.Handle %6, i8 -1) ; BufferUpdateCounter(uav,inc) + ret void +} + +; Function Attrs: nounwind +declare i32 @dx.op.bufferUpdateCounter(i32, %dx.types.Handle, i8) #0 + +; Function Attrs: nounwind readnone +declare %dx.types.Handle @dx.op.annotateHandle(i32, %dx.types.Handle, %dx.types.ResourceProperties) #1 + +; Function Attrs: nounwind readonly +declare %dx.types.Handle @dx.op.createHandleForLib.dx.types.Handle(i32, %dx.types.Handle) #2 + +attributes #0 = { nounwind } +attributes #1 = { nounwind readnone } +attributes #2 = { nounwind readonly } + +!dx.version = !{!0} +!dx.valver = !{!0} +!dx.shaderModel = !{!1} +!dx.resources = !{!2} +!dx.typeAnnotations = !{!6} +!dx.entryPoints = !{!10, !12} + +!0 = !{i32 1, i32 9} +!1 = !{!"lib", i32 6, i32 9} +!2 = !{null, !3, null, null} +!3 = !{!4} +!4 = !{i32 0, %"class.RWStructuredBuffer"* bitcast (%dx.types.Handle* @"\01?buffer@@3V?$RWStructuredBuffer@M@@A" to %"class.RWStructuredBuffer"*), !"buffer", i32 -1, i32 -1, i32 1, i32 12, i1 false, i1 true, i1 false, !5} +!5 = !{i32 1, i32 4, i32 4, i1 true} +!6 = !{i32 1, void ()* @"\01?main@@YAXXZ", !7} +!7 = !{!8} +!8 = !{i32 1, !9, !9} +!9 = !{} +!10 = !{null, !"", null, !2, !11} +!11 = !{i32 0, i64 8589934608} +!12 = !{void ()* @"\01?main@@YAXXZ", !"\01?main@@YAXXZ", null, null, !13} +!13 = !{i32 8, i32 7, i32 5, !14} +!14 = !{i32 0} \ No newline at end of file diff --git a/tools/clang/test/LitDXILValidation/ser_reordercoherent_invalid_sm.ll b/tools/clang/test/LitDXILValidation/ser_reordercoherent_invalid_sm.ll new file mode 100644 index 0000000000..efcb7d3c2b --- /dev/null +++ b/tools/clang/test/LitDXILValidation/ser_reordercoherent_invalid_sm.ll @@ -0,0 +1,83 @@ +; REQUIRES: dxil-1-8 +; RUN: not %dxv %s 2>&1 | FileCheck %s + + +; CHECK: error: reordercoherent requires SM 6.9 or later. 'buf' +; CHECK-NEXT: Function: ?main@@YAXXZ: error: reordercoherent requires SM 6.9 or later. +; CHECK-NEXT: note: at '%3 = call %dx.types.Handle @dx.op.annotateHandle(i32 216, %dx.types.Handle %2, %dx.types.ResourceProperties { i32 69643, i32 0 })' in block '#0' of function '?main@@YAXXZ'. +; CHECK-NEXT: Function: ?main@@YAXXZ: error: reordercoherent requires SM 6.9 or later. +; CHECK-NEXT: note: at '%3 = call %dx.types.Handle @dx.op.annotateHandle(i32 216, %dx.types.Handle %2, %dx.types.ResourceProperties { i32 69643, i32 0 })' in block '#0' of function '?main@@YAXXZ'. +; CHECK-NEXT: Validation failed. +; COM: Original HLSL source: +; COM: reordercoherent RWByteAddressBuffer buf; +; COM: +; COM: [Shader("raygeneration")] +; COM: void main() +; COM: { +; COM: buf.Store(0, 11.f); +; COM: } + +; shader hash: f7be6354830d1423764991adcfc26b0b +; +; Buffer Definitions: +; +; +; Resource Bindings: +; +; Name Type Format Dim ID HLSL Bind Count +; ------------------------------ ---------- ------- ----------- ------- -------------- ------ +; buf UAV byte r/w U0u4294967295,space4294967295 1 +; +target datalayout = "e-m:e-p:32:32-i1:32-i8:32-i16:32-i32:32-i64:64-f16:32-f32:32-f64:64-n8:16:32:64" +target triple = "dxil-ms-dx" + +%dx.types.Handle = type { i8* } +%dx.types.ResourceProperties = type { i32, i32 } +%struct.RWByteAddressBuffer = type { i32 } + +@"\01?buf@@3URWByteAddressBuffer@@A" = external constant %dx.types.Handle, align 4 + +; Function Attrs: nounwind +define void @"\01?main@@YAXXZ"() #0 { + %1 = load %dx.types.Handle, %dx.types.Handle* @"\01?buf@@3URWByteAddressBuffer@@A", align 4 + %2 = call %dx.types.Handle @dx.op.createHandleForLib.dx.types.Handle(i32 160, %dx.types.Handle %1) ; CreateHandleForLib(Resource) + %3 = call %dx.types.Handle @dx.op.annotateHandle(i32 216, %dx.types.Handle %2, %dx.types.ResourceProperties { i32 69643, i32 0 }) ; AnnotateHandle(res,props) resource: reordercoherent RWByteAddressBuffer + call void @dx.op.rawBufferStore.f32(i32 140, %dx.types.Handle %3, i32 0, i32 undef, float 1.100000e+01, float undef, float undef, float undef, i8 1, i32 4) ; RawBufferStore(uav,index,elementOffset,value0,value1,value2,value3,mask,alignment) + ret void +} + +; Function Attrs: nounwind +declare void @dx.op.rawBufferStore.f32(i32, %dx.types.Handle, i32, i32, float, float, float, float, i8, i32) #0 + +; Function Attrs: nounwind readnone +declare %dx.types.Handle @dx.op.annotateHandle(i32, %dx.types.Handle, %dx.types.ResourceProperties) #1 + +; Function Attrs: nounwind readonly +declare %dx.types.Handle @dx.op.createHandleForLib.dx.types.Handle(i32, %dx.types.Handle) #2 + +attributes #0 = { nounwind } +attributes #1 = { nounwind readnone } +attributes #2 = { nounwind readonly } + +!dx.version = !{!0} +!dx.valver = !{!0} +!dx.shaderModel = !{!1} +!dx.resources = !{!2} +!dx.typeAnnotations = !{!3} +!dx.entryPoints = !{!4, !5} + +!0 = !{i32 1, i32 8} +!1 = !{!"lib", i32 6, i32 8} +!2 = !{null, !6, null, null} +!3 = !{i32 1, void ()* @"\01?main@@YAXXZ", !7} +!4 = !{null, !"", null, !2, !8} +!5 = !{void ()* @"\01?main@@YAXXZ", !"\01?main@@YAXXZ", null, null, !9} +!6 = !{!10} +!7 = !{!11} +!8 = !{i32 0, i64 8589934608} +!9 = !{i32 8, i32 7, i32 5, !12} +!10 = !{i32 0, %struct.RWByteAddressBuffer* bitcast (%dx.types.Handle* @"\01?buf@@3URWByteAddressBuffer@@A" to %struct.RWByteAddressBuffer*), !"buf", i32 -1, i32 -1, i32 1, i32 11, i1 false, i1 false, i1 false, !13} +!11 = !{i32 1, !14, !14} +!12 = !{i32 0} +!13 = !{i32 4, i1 true} +!14 = !{} diff --git a/tools/clang/unittests/HLSL/DxilContainerTest.cpp b/tools/clang/unittests/HLSL/DxilContainerTest.cpp index 339b33c655..34b4d338fe 100644 --- a/tools/clang/unittests/HLSL/DxilContainerTest.cpp +++ b/tools/clang/unittests/HLSL/DxilContainerTest.cpp @@ -103,6 +103,7 @@ class DxilContainerTest : public ::testing::Test { TEST_METHOD(CompileCSWaveSizeRange_CheckPSV0) TEST_METHOD(CompileWhenOkThenCheckRDAT) TEST_METHOD(CompileWhenOkThenCheckRDAT2) + TEST_METHOD(CompileWhenOkThenCheckRDATSM69) TEST_METHOD(CompileWhenOkThenCheckReflection1) TEST_METHOD(DxcUtils_CreateReflection) TEST_METHOD(CheckReflectionQueryInterface) @@ -1444,6 +1445,146 @@ TEST_F(DxilContainerTest, CompileCSWaveSizeRange_CheckPSV0) { TEST_F(DxilContainerTest, CompileWhenOkThenCheckRDAT) { if (m_ver.SkipDxilVersion(1, 3)) return; + const char *shader = + "float c_buf;" + "RWTexture1D tex : register(u5);" + "Texture1D tex2 : register(t0);" + "RWByteAddressBuffer b_buf;" + "struct Foo { float2 f2; int2 i2; };" + "AppendStructuredBuffer append_buf;" + "ConsumeStructuredBuffer consume_buf;" + "RasterizerOrderedByteAddressBuffer rov_buf;" + "globallycoherent RWByteAddressBuffer gc_buf;" + "float function_import(float x);" + "export float function0(min16float x) { " + " return x + 1 + tex[0].x; }" + "export float function1(float x, min12int i) {" + " return x + c_buf + b_buf.Load(x) + tex2[i].x; }" + "export float function2(float x) { return x + function_import(x); }" + "export void function3(int i) {" + " Foo f = consume_buf.Consume();" + " f.f2 += 0.5; append_buf.Append(f);" + " rov_buf.Store(i, f.i2.x);" + " gc_buf.Store(i, f.i2.y);" + " b_buf.Store(i, f.i2.x + f.i2.y); }"; + CComPtr pCompiler; + CComPtr pSource; + CComPtr pProgram; + CComPtr pDisassembly; + CComPtr pResult; + + struct CheckResFlagInfo { + std::string name; + hlsl::DXIL::ResourceKind kind; + hlsl::RDAT::DxilResourceFlag flag; + }; + const unsigned numResFlagCheck = 5; + CheckResFlagInfo resFlags[numResFlagCheck] = { + {"b_buf", hlsl::DXIL::ResourceKind::RawBuffer, + hlsl::RDAT::DxilResourceFlag::None}, + {"append_buf", hlsl::DXIL::ResourceKind::StructuredBuffer, + hlsl::RDAT::DxilResourceFlag::UAVCounter}, + {"consume_buf", hlsl::DXIL::ResourceKind::StructuredBuffer, + hlsl::RDAT::DxilResourceFlag::UAVCounter}, + {"gc_buf", hlsl::DXIL::ResourceKind::RawBuffer, + hlsl::RDAT::DxilResourceFlag::UAVGloballyCoherent}, + {"rov_buf", hlsl::DXIL::ResourceKind::RawBuffer, + hlsl::RDAT::DxilResourceFlag::UAVRasterizerOrderedView}}; + + VERIFY_SUCCEEDED(CreateCompiler(&pCompiler)); + CreateBlobFromText(shader, &pSource); + VERIFY_SUCCEEDED(pCompiler->Compile(pSource, L"hlsl.hlsl", L"main", + L"lib_6_3", nullptr, 0, nullptr, 0, + nullptr, &pResult)); + HRESULT hrStatus; + VERIFY_SUCCEEDED(pResult->GetStatus(&hrStatus)); + VERIFY_SUCCEEDED(hrStatus); + VERIFY_SUCCEEDED(pResult->GetResult(&pProgram)); + CComPtr containerReflection; + uint32_t partCount; + IFT(m_dllSupport.CreateInstance(CLSID_DxcContainerReflection, + &containerReflection)); + IFT(containerReflection->Load(pProgram)); + IFT(containerReflection->GetPartCount(&partCount)); + bool blobFound = false; + for (uint32_t i = 0; i < partCount; ++i) { + uint32_t kind; + IFT(containerReflection->GetPartKind(i, &kind)); + if (kind == (uint32_t)hlsl::DxilFourCC::DFCC_RuntimeData) { + blobFound = true; + using namespace hlsl::RDAT; + CComPtr pBlob; + IFT(containerReflection->GetPartContent(i, &pBlob)); + // Validate using DxilRuntimeData + DxilRuntimeData context; + context.InitFromRDAT((char *)pBlob->GetBufferPointer(), + pBlob->GetBufferSize()); + auto funcTable = context.GetFunctionTable(); + auto resTable = context.GetResourceTable(); + VERIFY_ARE_EQUAL(funcTable.Count(), 4U); + std::string str("function"); + for (uint32_t j = 0; j < funcTable.Count(); ++j) { + auto funcReader = funcTable[j]; + std::string funcName(funcReader.getUnmangledName()); + VERIFY_IS_TRUE(str.compare(funcName.substr(0, 8)) == 0); + std::string cur_str = str; + cur_str.push_back('0' + j); + if (cur_str.compare("function0") == 0) { + VERIFY_ARE_EQUAL(funcReader.getResources().Count(), 1U); + hlsl::ShaderFlags flag; + flag.SetUAVLoadAdditionalFormats(true); + flag.SetLowPrecisionPresent(true); + uint64_t rawFlag = flag.GetFeatureInfo(); + VERIFY_ARE_EQUAL(funcReader.GetFeatureFlags(), rawFlag); + auto resReader = funcReader.getResources()[0]; + VERIFY_ARE_EQUAL(resReader.getClass(), + hlsl::DXIL::ResourceClass::UAV); + VERIFY_ARE_EQUAL(resReader.getKind(), + hlsl::DXIL::ResourceKind::Texture1D); + } else if (cur_str.compare("function1") == 0) { + hlsl::ShaderFlags flag; + flag.SetLowPrecisionPresent(true); + uint64_t rawFlag = flag.GetFeatureInfo(); + VERIFY_ARE_EQUAL(funcReader.GetFeatureFlags(), rawFlag); + VERIFY_ARE_EQUAL(funcReader.getResources().Count(), 3U); + } else if (cur_str.compare("function2") == 0) { + VERIFY_ARE_EQUAL(funcReader.GetFeatureFlags() & 0xffffffffffffffff, + 0U); + VERIFY_ARE_EQUAL(funcReader.getResources().Count(), 0U); + std::string dependency = funcReader.getFunctionDependencies()[0]; + VERIFY_IS_TRUE(dependency.find("function_import") != + std::string::npos); + } else if (cur_str.compare("function3") == 0) { + VERIFY_ARE_EQUAL(funcReader.GetFeatureFlags() & 0xffffffffffffffff, + 0U); + VERIFY_ARE_EQUAL(funcReader.getResources().Count(), numResFlagCheck); + for (unsigned i = 0; i < funcReader.getResources().Count(); ++i) { + auto resReader = funcReader.getResources()[0]; + VERIFY_ARE_EQUAL(resReader.getClass(), + hlsl::DXIL::ResourceClass::UAV); + unsigned j = 0; + for (; j < numResFlagCheck; ++j) { + if (resFlags[j].name.compare(resReader.getName()) == 0) + break; + } + VERIFY_IS_LESS_THAN(j, numResFlagCheck); + VERIFY_ARE_EQUAL(resReader.getKind(), resFlags[j].kind); + VERIFY_ARE_EQUAL(resReader.getFlags(), + static_cast(resFlags[j].flag)); + } + } else { + IFTBOOLMSG(false, E_FAIL, "unknown function name"); + } + } + VERIFY_ARE_EQUAL(resTable.Count(), 8U); + } + } + IFTBOOLMSG(blobFound, E_FAIL, "failed to find RDAT blob after compiling"); +} + +TEST_F(DxilContainerTest, CompileWhenOkThenCheckRDATSM69) { + if (m_ver.SkipDxilVersion(1, 9)) + return; const char *shader = "float c_buf;" "RWTexture1D tex : register(u5);" @@ -1497,7 +1638,7 @@ TEST_F(DxilContainerTest, CompileWhenOkThenCheckRDAT) { VERIFY_SUCCEEDED(CreateCompiler(&pCompiler)); CreateBlobFromText(shader, &pSource); VERIFY_SUCCEEDED(pCompiler->Compile(pSource, L"hlsl.hlsl", L"main", - L"lib_6_3", nullptr, 0, nullptr, 0, + L"lib_6_9", nullptr, 0, nullptr, 0, nullptr, &pResult)); HRESULT hrStatus; VERIFY_SUCCEEDED(pResult->GetStatus(&hrStatus)); diff --git a/utils/hct/hctdb.py b/utils/hct/hctdb.py index 57f2574005..3598375368 100644 --- a/utils/hct/hctdb.py +++ b/utils/hct/hctdb.py @@ -8016,8 +8016,8 @@ def build_valrules(self): ) self.add_valrule("Meta.ValidSamplerMode", "Invalid sampler mode on sampler .") self.add_valrule( - "Meta.GlcNotOnAppendConsume", - "globallycoherent cannot be used with append/consume buffers: '%0'.", + "Meta.CoherenceNotOnAppendConsume", + "%0coherent cannot be used with append/consume buffers: '%1'.", ) self.add_valrule_msg( "Meta.StructBufAlignment", @@ -8409,6 +8409,10 @@ def build_valrules(self): "Instr.MayReorderThreadUndefCoherenceHintParam", "Use of undef coherence hint or num coherence hint bits in MaybeReorderThread.", ) + self.add_valrule( + "Instr.ReorderCoherentRequiresSM69", + "reordercoherent requires SM 6.9 or later.", + ) # Linalg ops self.add_valrule_msg( From 8a6955c7a16b92d40567f0f801daa60161315cf5 Mon Sep 17 00:00:00 2001 From: Simon Moll Date: Tue, 6 May 2025 18:04:57 +0200 Subject: [PATCH 2/7] nfc: LLVM Coding Standards --- lib/DxilValidation/DxilValidation.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/DxilValidation/DxilValidation.cpp b/lib/DxilValidation/DxilValidation.cpp index b4028935e5..c7c9c28ac7 100644 --- a/lib/DxilValidation/DxilValidation.cpp +++ b/lib/DxilValidation/DxilValidation.cpp @@ -176,15 +176,13 @@ static DxilResourceProperties GetResourceFromHandle(Value *Handle, } DxilResourceProperties RP = ValCtx.GetResourceFromVal(Handle); - if (RP.getResourceClass() == DXIL::ResourceClass::Invalid) { + if (RP.getResourceClass() == DXIL::ResourceClass::Invalid) ValCtx.EmitInstrError(cast(Handle), ValidationRule::InstrHandleNotFromCreateHandle); - } if (RP.Basic.IsReorderCoherent && - !ValCtx.DxilMod.GetShaderModel()->IsSM69Plus()) { + !ValCtx.DxilMod.GetShaderModel()->IsSM69Plus()) ValCtx.EmitInstrError(HandleCall, ValidationRule::InstrReorderCoherentRequiresSM69); - } return RP; } From 562593aa4907e8c39b087a0cfe7b997bd258dceb Mon Sep 17 00:00:00 2001 From: Simon Moll Date: Tue, 6 May 2025 18:20:48 +0200 Subject: [PATCH 3/7] Apply suggestions from code review Co-authored-by: Tex Riddell --- lib/DxilValidation/DxilValidation.cpp | 2 +- .../LitDXILValidation/ser_reordercoherent_invalid_incdec.ll | 2 +- utils/hct/hctdb.py | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/DxilValidation/DxilValidation.cpp b/lib/DxilValidation/DxilValidation.cpp index c7c9c28ac7..d23e9b707f 100644 --- a/lib/DxilValidation/DxilValidation.cpp +++ b/lib/DxilValidation/DxilValidation.cpp @@ -4426,7 +4426,7 @@ static void ValidateResources(ValidationContext &ValCtx) { StringRef Prefix = Uav->IsGloballyCoherent() ? "globally" : "reorder"; ValCtx.EmitResourceFormatError( Uav.get(), ValidationRule::MetaCoherenceNotOnAppendConsume, - {Prefix, ValCtx.GetResourceName(Uav.get())}); + {Prefix}); } ValidateResource(*Uav, ValCtx); diff --git a/tools/clang/test/LitDXILValidation/ser_reordercoherent_invalid_incdec.ll b/tools/clang/test/LitDXILValidation/ser_reordercoherent_invalid_incdec.ll index aea6e807d4..1f68a9a95f 100644 --- a/tools/clang/test/LitDXILValidation/ser_reordercoherent_invalid_incdec.ll +++ b/tools/clang/test/LitDXILValidation/ser_reordercoherent_invalid_incdec.ll @@ -13,7 +13,7 @@ ; COM: buffer.DecrementCounter(); ; COM: } -; CHECK: error: reordercoherent cannot be used with append/consume buffers: 'buffer'. 'buffer' +; CHECK: error: reordercoherent cannot be used on buffer with counter 'buffer' ; CHECK-NEXT: Validation failed. ; shader hash: 638950814a9023bf537d61dbb330a4c8 diff --git a/utils/hct/hctdb.py b/utils/hct/hctdb.py index 3598375368..6e872f68fe 100644 --- a/utils/hct/hctdb.py +++ b/utils/hct/hctdb.py @@ -8017,7 +8017,8 @@ def build_valrules(self): self.add_valrule("Meta.ValidSamplerMode", "Invalid sampler mode on sampler .") self.add_valrule( "Meta.CoherenceNotOnAppendConsume", - "%0coherent cannot be used with append/consume buffers: '%1'.", + "globally/reorder coherent incompatible with append/consume/counter buffers", + "%0coherent cannot be used on buffer with counter", ) self.add_valrule_msg( "Meta.StructBufAlignment", From e580147b590d8a91f16b24d1a939cb50f44bcfc5 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 6 May 2025 17:12:24 +0000 Subject: [PATCH 4/7] chore: autopublish 2025-05-06T17:12:23Z --- lib/DxilValidation/DxilValidation.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/DxilValidation/DxilValidation.cpp b/lib/DxilValidation/DxilValidation.cpp index d23e9b707f..28917e0600 100644 --- a/lib/DxilValidation/DxilValidation.cpp +++ b/lib/DxilValidation/DxilValidation.cpp @@ -4425,8 +4425,7 @@ static void ValidateResources(ValidationContext &ValCtx) { if (Uav->HasCounter() && UavIsCoherent) { StringRef Prefix = Uav->IsGloballyCoherent() ? "globally" : "reorder"; ValCtx.EmitResourceFormatError( - Uav.get(), ValidationRule::MetaCoherenceNotOnAppendConsume, - {Prefix}); + Uav.get(), ValidationRule::MetaCoherenceNotOnAppendConsume, {Prefix}); } ValidateResource(*Uav, ValCtx); From e614f468031942d28ee642af94496a00b67c34a2 Mon Sep 17 00:00:00 2001 From: Tex Riddell Date: Tue, 6 May 2025 10:15:08 -0700 Subject: [PATCH 5/7] Fix break in hctdb.py from earlier PR suggestion. --- utils/hct/hctdb.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/hct/hctdb.py b/utils/hct/hctdb.py index 6e872f68fe..a633a0cf3e 100644 --- a/utils/hct/hctdb.py +++ b/utils/hct/hctdb.py @@ -8015,7 +8015,7 @@ def build_valrules(self): "Hull Shader MaxTessFactor must be [%0..%1]. %2 specified.", ) self.add_valrule("Meta.ValidSamplerMode", "Invalid sampler mode on sampler .") - self.add_valrule( + self.add_valrule_msg( "Meta.CoherenceNotOnAppendConsume", "globally/reorder coherent incompatible with append/consume/counter buffers", "%0coherent cannot be used on buffer with counter", From 5ab331c7abb8a07b138b547919a67d20f6de1f5b Mon Sep 17 00:00:00 2001 From: Tex Riddell Date: Tue, 6 May 2025 11:37:45 -0700 Subject: [PATCH 6/7] Update generated DXIL.rst to unblock build --- docs/DXIL.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/DXIL.rst b/docs/DXIL.rst index 0b71db41d6..7532ec3c42 100644 --- a/docs/DXIL.rst +++ b/docs/DXIL.rst @@ -3217,7 +3217,7 @@ META.BARYCENTRICSTWOPERSPECTIVES There can only be up to tw META.BRANCHFLATTEN Can't use branch and flatten attributes together. META.CLIPCULLMAXCOMPONENTS Combined elements of SV_ClipDistance and SV_CullDistance must fit in 8 components META.CLIPCULLMAXROWS Combined elements of SV_ClipDistance and SV_CullDistance must fit in two rows. -META.COHERENCENOTONAPPENDCONSUME %0coherent cannot be used with append/consume buffers: '%1'. +META.COHERENCENOTONAPPENDCONSUME globally/reorder coherent incompatible with append/consume/counter buffers META.COMPUTEWITHNODE Compute entry must not have node metadata META.CONTROLFLOWHINTNOTONCONTROLFLOW Control flow hint only works on control flow inst. META.DENSERESIDS Resource identifiers must be zero-based and dense. From 2db3eb45c6a34f9bab74f2ebfdebfdda0d15640d Mon Sep 17 00:00:00 2001 From: Simon Moll Date: Tue, 6 May 2025 18:30:59 +0200 Subject: [PATCH 7/7] Fix expected msg in StructBufGlobalCoherentAndCounter --- tools/clang/unittests/HLSL/ValidationTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/clang/unittests/HLSL/ValidationTest.cpp b/tools/clang/unittests/HLSL/ValidationTest.cpp index 01f24e0227..980bf6c7c2 100644 --- a/tools/clang/unittests/HLSL/ValidationTest.cpp +++ b/tools/clang/unittests/HLSL/ValidationTest.cpp @@ -1488,7 +1488,7 @@ TEST_F(ValidationTest, StructBufGlobalCoherentAndCounter) { L"..\\DXILValidation\\struct_buf1.hlsl", "ps_6_0", "!\"buf2\", i32 0, i32 0, i32 1, i32 12, i1 false, i1 false", "!\"buf2\", i32 0, i32 0, i32 1, i32 12, i1 true, i1 true", - "globallycoherent cannot be used with append/consume buffers: 'buf2'"); + "globallycoherent cannot be used on buffer with counter 'buf2'"); } TEST_F(ValidationTest, StructBufStrideAlign) {