Validation: Mesh/Amplification DXIL op checks insufficient and inefficient

[tex3d]

The following checks are intended to diagnose when multiple calls are present to a DXIL op that allows only one call from the entry function (SetMeshOutputCounts, GetMeshPayload, DispatchMesh):

DirectXShaderCompiler/lib/DxilValidation/DxilValidation.cpp

Lines 2729 to 2759 in d5951b7

	// External function validation will check the parameter
	// list. This function will check that the call does not
	// violate any rules.
	if (dxilOpcode == DXIL::OpCode::SetMeshOutputCounts) {
	// validate the call count of SetMeshOutputCounts
	if (setMeshOutputCounts != nullptr) {
	ValCtx.EmitInstrError(
	&I, ValidationRule::InstrMultipleSetMeshOutputCounts);
	}
	setMeshOutputCounts = CI;
	}
	if (dxilOpcode == DXIL::OpCode::GetMeshPayload) {
	// validate the call count of GetMeshPayload
	if (getMeshPayload != nullptr) {
	ValCtx.EmitInstrError(
	&I, ValidationRule::InstrMultipleGetMeshPayload);
	}
	getMeshPayload = CI;
	}
	if (dxilOpcode == DXIL::OpCode::DispatchMesh) {
	// validate the call count of DispatchMesh
	if (dispatchMesh != nullptr) {
	ValCtx.EmitInstrError(&I,
	ValidationRule::InstrNotOnceDispatchMesh);
	}
	dispatchMesh = CI;
	}
	}

as well as capture that call for later deeper validation after instruction iteration in ValidateMsIntrinsics and ValidateAsIntrinsics:

DirectXShaderCompiler/lib/DxilValidation/DxilValidation.cpp

Lines 3009 to 3011 in d5951b7

	ValidateMsIntrinsics(F, ValCtx, setMeshOutputCounts, getMeshPayload);
	ValidateAsIntrinsics(F, ValCtx, dispatchMesh);

ValidateMsIntrinsics creates a DominatorTree and iterates all instructions again in order to ensure that SetMeshOutputCounts is called before stores to verts/prims/indices.

DirectXShaderCompiler/lib/DxilValidation/DxilValidation.cpp

Lines 2236 to 2242 in d5951b7

	DominatorTreeAnalysis DTA;
	DominatorTree DT = DTA.run(*F);
	for (auto b = F->begin(), bend = F->end(); b != bend; ++b) {
	bool foundSetMeshOutputCountsInCurrentBB = false;
	for (auto i = b->begin(), iend = b->end(); i != iend; ++i) {
	llvm::Instruction &I = *i;

ValidateAsIntrinsics creates a PostDominatorTree to verify that the DispatchMesh call post-dominates the entry block.

DirectXShaderCompiler/lib/DxilValidation/DxilValidation.cpp

Line 2352 in d5951b7

PostDominatorTree PDT;

First, a DominatorTree or PostDominatorTree should be created once and cached for any given function, instead of locally (re)computed whenever needed. Other functions creating DominatorTree: ValidateFlowControl, and PostDominatorTree: ValidateTGSMRaceCondition. These should be created and cached by the ValidationContext object instead.

Second, it seems excessive to iterate every instruction in the function again in ValidateMsIntrinsics just to find the call we just found while iterating every instruction. The point is to look for any writes to outputs so the dominance check will catch writes before the SetMeshOutputCounts call. However, there should be a better way, such as by collecting SetMeshOutputCounts calls by calling function (entry) in advance (checking uniqueness at the same time), then with the DominatorTree cache available, just check the store vert/prim/indices DXIL op calls for dominance by the associated SetMeshOutputCounts previously collected for the function (kept by ValidationContext). If it doesn't dominate (or is missing), that's an error case.

Third, this approach is currently insufficient, since it's possible to have a [noinline] user function that calls one of these DXIL ops. This call must be attributed to the entry function that leads to the call, or disallowed entirely. Disallowing the call in [noinline] user functions is the easiest.

If we wanted to allow these calls, there would be some work to do, and it should be noted that for library targets, validation currently treats [noinline] functions as if they are export functions, which means that these particular calls are currently disallowed in [noinline] functions, even though they are not really exported functions.

Shader that passes validation, but should fail: https://godbolt.org/z/8MW9rdhqE