Make Instrumentation format version 2 the default (Step 1) (#3096)

* Make Instrumentation format version 2 the default (Step 1)

Add new interfaces without version number argument. Remove version 1
logic and tests. Version interfaces will be removed in step 2 after
layers have transitioned to new interface.

* Add error messages to InstrumentPass().
This commit is contained in:
greg-lunarg 2019-12-16 12:18:47 -07:00 committed by Steven Perron
parent 96354f5047
commit fccbc00aca
7 changed files with 125 additions and 4169 deletions

View File

@ -35,10 +35,9 @@ namespace spvtools {
// generated by InstrumentPass::GenDebugStreamWrite. This method is utilized
// by InstBindlessCheckPass.
//
// kInst2* values support version 2 of the output record format. These should
// be used if available and version 2 is enabled. Version 1 is DEPRECATED.
// Specifically, version 1 uses two words for the stage-specific section of
// the output record; version 2 uses three words.
// kInst2* values support version 2 of the output record format and were used
// for the transition to this format. These values have now been transferred
// to the original kInst* values. The kInst2* values are therefore DEPRECATED.
//
// The first member of the debug output buffer contains the next available word
// in the data stream to be written. Shaders will atomically read and update
@ -124,7 +123,7 @@ static const int kInstRayTracingOutLaunchIdY = kInstCommonOutCnt + 1;
static const int kInstRayTracingOutLaunchIdZ = kInstCommonOutCnt + 2;
// Size of Common and Stage-specific Members
static const int kInstStageOutCnt = kInstCommonOutCnt + 2;
static const int kInstStageOutCnt = kInstCommonOutCnt + 3;
static const int kInst2StageOutCnt = kInstCommonOutCnt + 3;
// Validation Error Code Offset
@ -160,6 +159,10 @@ static const int kInst2BindlessUninitOutCnt = kInst2StageOutCnt + 3;
// A buffer address unalloc error will output the 64-bit pointer in
// two 32-bit pieces, lower bits first.
static const int kInstBuffAddrUnallocOutDescPtrLo = kInstStageOutCnt + 1;
static const int kInstBuffAddrUnallocOutDescPtrHi = kInstStageOutCnt + 2;
static const int kInstBuffAddrUnallocOutCnt = kInstStageOutCnt + 3;
static const int kInst2BuffAddrUnallocOutDescPtrLo = kInst2StageOutCnt + 1;
static const int kInst2BuffAddrUnallocOutDescPtrHi = kInst2StageOutCnt + 2;
static const int kInst2BuffAddrUnallocOutCnt = kInst2StageOutCnt + 3;

View File

@ -28,18 +28,19 @@ namespace opt {
// external design may change as the layer evolves.
class InstBindlessCheckPass : public InstrumentPass {
public:
// For test harness only
InstBindlessCheckPass()
: InstrumentPass(7, 23, kInstValidationIdBindless, 1),
input_length_enabled_(true),
input_init_enabled_(true) {}
// For all other interfaces
// Deprecated interface
InstBindlessCheckPass(uint32_t desc_set, uint32_t shader_id,
bool input_length_enable, bool input_init_enable,
uint32_t version)
: InstrumentPass(desc_set, shader_id, kInstValidationIdBindless, version),
input_length_enabled_(input_length_enable),
input_init_enabled_(input_init_enable) {}
// Preferred Interface
InstBindlessCheckPass(uint32_t desc_set, uint32_t shader_id,
bool input_length_enable, bool input_init_enable)
: InstrumentPass(desc_set, shader_id, kInstValidationIdBindless),
input_length_enabled_(input_length_enable),
input_init_enabled_(input_init_enable) {}
~InstBindlessCheckPass() override = default;

View File

@ -28,13 +28,13 @@ namespace opt {
// external design of this class may change as the layer evolves.
class InstBuffAddrCheckPass : public InstrumentPass {
public:
// For test harness only
InstBuffAddrCheckPass()
: InstrumentPass(7, 23, kInstValidationIdBuffAddr, 1) {}
// For all other interfaces
// Deprecated interface
InstBuffAddrCheckPass(uint32_t desc_set, uint32_t shader_id, uint32_t version)
: InstrumentPass(desc_set, shader_id, kInstValidationIdBuffAddr,
version) {}
// Preferred interface
InstBuffAddrCheckPass(uint32_t desc_set, uint32_t shader_id)
: InstrumentPass(desc_set, shader_id, kInstValidationIdBuffAddr) {}
~InstBuffAddrCheckPass() override = default;

View File

@ -185,32 +185,12 @@ void InstrumentPass::GenStageStreamWriteCode(uint32_t stage_idx,
GetUintId(), SpvOpCompositeExtract, load_id, 1);
Instruction* z_inst = builder->AddIdLiteralOp(
GetUintId(), SpvOpCompositeExtract, load_id, 2);
if (version_ == 1) {
// For version 1 format, as a stopgap, pack uvec3 into first word:
// x << 21 | y << 10 | z. Second word is unused. (DEPRECATED)
Instruction* x_shft_inst = builder->AddBinaryOp(
GetUintId(), SpvOpShiftLeftLogical, x_inst->result_id(),
builder->GetUintConstantId(21));
Instruction* y_shft_inst = builder->AddBinaryOp(
GetUintId(), SpvOpShiftLeftLogical, y_inst->result_id(),
builder->GetUintConstantId(10));
Instruction* x_or_y_inst = builder->AddBinaryOp(
GetUintId(), SpvOpBitwiseOr, x_shft_inst->result_id(),
y_shft_inst->result_id());
Instruction* x_or_y_or_z_inst =
builder->AddBinaryOp(GetUintId(), SpvOpBitwiseOr,
x_or_y_inst->result_id(), z_inst->result_id());
GenDebugOutputFieldCode(base_offset_id, kInstCompOutGlobalInvocationId,
x_or_y_or_z_inst->result_id(), builder);
} else {
// For version 2 format, write all three words
GenDebugOutputFieldCode(base_offset_id, kInstCompOutGlobalInvocationIdX,
x_inst->result_id(), builder);
GenDebugOutputFieldCode(base_offset_id, kInstCompOutGlobalInvocationIdY,
y_inst->result_id(), builder);
GenDebugOutputFieldCode(base_offset_id, kInstCompOutGlobalInvocationIdZ,
z_inst->result_id(), builder);
}
GenDebugOutputFieldCode(base_offset_id, kInstCompOutGlobalInvocationIdX,
x_inst->result_id(), builder);
GenDebugOutputFieldCode(base_offset_id, kInstCompOutGlobalInvocationIdY,
y_inst->result_id(), builder);
GenDebugOutputFieldCode(base_offset_id, kInstCompOutGlobalInvocationIdZ,
z_inst->result_id(), builder);
} break;
case SpvExecutionModelGeometry: {
// Load and store PrimitiveId and InvocationId.
@ -231,30 +211,23 @@ void InstrumentPass::GenStageStreamWriteCode(uint32_t stage_idx,
kInstTessCtlOutPrimitiveId, base_offset_id, builder);
} break;
case SpvExecutionModelTessellationEvaluation: {
if (version_ == 1) {
// For format version 1, load and store InvocationId.
GenBuiltinOutputCode(
context()->GetBuiltinInputVarId(SpvBuiltInInvocationId),
kInstTessOutInvocationId, base_offset_id, builder);
} else {
// For format version 2, load and store PrimitiveId and TessCoord.uv
GenBuiltinOutputCode(
context()->GetBuiltinInputVarId(SpvBuiltInPrimitiveId),
kInstTessEvalOutPrimitiveId, base_offset_id, builder);
uint32_t load_id = GenVarLoad(
context()->GetBuiltinInputVarId(SpvBuiltInTessCoord), builder);
Instruction* uvec3_cast_inst =
builder->AddUnaryOp(GetVec3UintId(), SpvOpBitcast, load_id);
uint32_t uvec3_cast_id = uvec3_cast_inst->result_id();
Instruction* u_inst = builder->AddIdLiteralOp(
GetUintId(), SpvOpCompositeExtract, uvec3_cast_id, 0);
Instruction* v_inst = builder->AddIdLiteralOp(
GetUintId(), SpvOpCompositeExtract, uvec3_cast_id, 1);
GenDebugOutputFieldCode(base_offset_id, kInstTessEvalOutTessCoordU,
u_inst->result_id(), builder);
GenDebugOutputFieldCode(base_offset_id, kInstTessEvalOutTessCoordV,
v_inst->result_id(), builder);
}
// Load and store PrimitiveId and TessCoord.uv
GenBuiltinOutputCode(
context()->GetBuiltinInputVarId(SpvBuiltInPrimitiveId),
kInstTessEvalOutPrimitiveId, base_offset_id, builder);
uint32_t load_id = GenVarLoad(
context()->GetBuiltinInputVarId(SpvBuiltInTessCoord), builder);
Instruction* uvec3_cast_inst =
builder->AddUnaryOp(GetVec3UintId(), SpvOpBitcast, load_id);
uint32_t uvec3_cast_id = uvec3_cast_inst->result_id();
Instruction* u_inst = builder->AddIdLiteralOp(
GetUintId(), SpvOpCompositeExtract, uvec3_cast_id, 0);
Instruction* v_inst = builder->AddIdLiteralOp(
GetUintId(), SpvOpCompositeExtract, uvec3_cast_id, 1);
GenDebugOutputFieldCode(base_offset_id, kInstTessEvalOutTessCoordU,
u_inst->result_id(), builder);
GenDebugOutputFieldCode(base_offset_id, kInstTessEvalOutTessCoordV,
v_inst->result_id(), builder);
} break;
case SpvExecutionModelFragment: {
// Load FragCoord and convert to Uint
@ -671,8 +644,7 @@ uint32_t InstrumentPass::GetStreamWriteFunctionId(uint32_t stage_idx,
context(), &*new_blk_ptr,
IRContext::kAnalysisDefUse | IRContext::kAnalysisInstrToBlockMapping);
// Gen test if debug output buffer size will not be exceeded.
uint32_t val_spec_offset =
(version_ == 1) ? kInstStageOutCnt : kInst2StageOutCnt;
uint32_t val_spec_offset = kInstStageOutCnt;
uint32_t obuf_record_sz = val_spec_offset + val_spec_param_cnt;
uint32_t buf_id = GetOutputBufferId();
uint32_t buf_uint_ptr_id = GetOutputBufferPtrId();
@ -892,6 +864,14 @@ bool InstrumentPass::InstProcessCallTreeFromRoots(InstProcessFunction& pfn,
}
bool InstrumentPass::InstProcessEntryPointCallTree(InstProcessFunction& pfn) {
// Check that format version 2 requested
if (version_ != 2u) {
if (consumer()) {
std::string message = "Unsupported instrumentation format requested";
consumer()(SPV_MSG_ERROR, 0, {0, 0, 0}, message.c_str());
}
return false;
}
// Make sure all entry points have the same execution model. Do not
// instrument if they do not.
// TODO(greg-lunarg): Handle mixed stages. Technically, a shader module
@ -905,12 +885,17 @@ bool InstrumentPass::InstProcessEntryPointCallTree(InstProcessFunction& pfn) {
for (auto& e : get_module()->entry_points()) {
if (ecnt == 0)
stage = e.GetSingleWordInOperand(kEntryPointExecutionModelInIdx);
else if (e.GetSingleWordInOperand(kEntryPointExecutionModelInIdx) != stage)
else if (e.GetSingleWordInOperand(kEntryPointExecutionModelInIdx) !=
stage) {
if (consumer()) {
std::string message = "Mixed stage shader module not supported";
consumer()(SPV_MSG_ERROR, 0, {0, 0, 0}, message.c_str());
}
return false;
}
++ecnt;
}
// Only supporting vertex, fragment and compute shaders at the moment.
// TODO(greg-lunarg): Handle all stages.
// Check for supported stages
if (stage != SpvExecutionModelVertex && stage != SpvExecutionModelFragment &&
stage != SpvExecutionModelGeometry &&
stage != SpvExecutionModelGLCompute &&
@ -920,8 +905,14 @@ bool InstrumentPass::InstProcessEntryPointCallTree(InstProcessFunction& pfn) {
stage != SpvExecutionModelIntersectionNV &&
stage != SpvExecutionModelAnyHitNV &&
stage != SpvExecutionModelClosestHitNV &&
stage != SpvExecutionModelMissNV && stage != SpvExecutionModelCallableNV)
stage != SpvExecutionModelMissNV &&
stage != SpvExecutionModelCallableNV) {
if (consumer()) {
std::string message = "Stage not supported by instrumentation";
consumer()(SPV_MSG_ERROR, 0, {0, 0, 0}, message.c_str());
}
return false;
}
// Add together the roots of all entry points
std::queue<uint32_t> roots;
for (auto& e : get_module()->entry_points()) {

View File

@ -81,7 +81,16 @@ class InstrumentPass : public Pass {
protected:
// Create instrumentation pass for |validation_id| which utilizes descriptor
// set |desc_set| for debug input and output buffers and writes |shader_id|
// into debug output records with format |version|.
// into debug output records.
InstrumentPass(uint32_t desc_set, uint32_t shader_id, uint32_t validation_id)
: Pass(),
desc_set_(desc_set),
shader_id_(shader_id),
validation_id_(validation_id),
version_(2u) {}
// Create instrumentation pass for |validation_id| which utilizes descriptor
// set |desc_set| for debug input and output buffers and writes |shader_id|
// into debug output records with format |version|. Deprecated.
InstrumentPass(uint32_t desc_set, uint32_t shader_id, uint32_t validation_id,
uint32_t version)
: Pass(),

File diff suppressed because it is too large Load Diff

View File

@ -310,7 +310,7 @@ OpFunctionEnd
// SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
SinglePassRunAndCheck<InstBuffAddrCheckPass>(
defs_before + func_before, defs_after + func_after + new_funcs, true,
true, 7u, 23u, 2u);
true, 7u, 23u);
}
TEST_F(InstBuffAddrTest, InstPhysicalStorageBufferLoadAndStore) {
@ -612,7 +612,7 @@ OpFunctionEnd
SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
SinglePassRunAndCheck<InstBuffAddrCheckPass>(
defs_before + func_before, defs_after + func_after + new_funcs, true,
true, 7u, 23u, 2u);
true, 7u, 23u);
}
} // namespace