Fix inst_buff_addr_check to handle struct loads (#4489)

This commit is contained in:
Greg Fischer 2021-09-23 10:59:38 -06:00 committed by GitHub
parent 134d763f45
commit f125452cf8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 273 additions and 5 deletions

View File

@ -130,13 +130,48 @@ void InstBuffAddrCheckPass::GenCheckCode(
context()->KillInst(ref_inst);
}
uint32_t InstBuffAddrCheckPass::GetTypeAlignment(uint32_t type_id) {
Instruction* type_inst = get_def_use_mgr()->GetDef(type_id);
switch (type_inst->opcode()) {
case SpvOpTypeFloat:
case SpvOpTypeInt:
case SpvOpTypeVector:
return GetTypeLength(type_id);
case SpvOpTypeMatrix:
return GetTypeAlignment(type_inst->GetSingleWordInOperand(0));
case SpvOpTypeArray:
case SpvOpTypeRuntimeArray:
return GetTypeAlignment(type_inst->GetSingleWordInOperand(0));
case SpvOpTypeStruct: {
uint32_t max = 0;
type_inst->ForEachInId([&max, this](const uint32_t* iid) {
uint32_t alignment = GetTypeAlignment(*iid);
max = (alignment > max) ? alignment : max;
});
return max;
}
case SpvOpTypePointer:
assert(type_inst->GetSingleWordInOperand(0) ==
SpvStorageClassPhysicalStorageBufferEXT &&
"unexpected pointer type");
return 8u;
default:
assert(false && "unexpected type");
return 0;
}
}
uint32_t InstBuffAddrCheckPass::GetTypeLength(uint32_t type_id) {
Instruction* type_inst = get_def_use_mgr()->GetDef(type_id);
switch (type_inst->opcode()) {
case SpvOpTypeFloat:
case SpvOpTypeInt:
return type_inst->GetSingleWordInOperand(0) / 8u;
case SpvOpTypeVector:
case SpvOpTypeVector: {
uint32_t raw_cnt = type_inst->GetSingleWordInOperand(1);
uint32_t adj_cnt = (raw_cnt == 3u) ? 4u : raw_cnt;
return adj_cnt * GetTypeLength(type_inst->GetSingleWordInOperand(0));
}
case SpvOpTypeMatrix:
return type_inst->GetSingleWordInOperand(1) *
GetTypeLength(type_inst->GetSingleWordInOperand(0));
@ -145,8 +180,29 @@ uint32_t InstBuffAddrCheckPass::GetTypeLength(uint32_t type_id) {
SpvStorageClassPhysicalStorageBufferEXT &&
"unexpected pointer type");
return 8u;
case SpvOpTypeArray: {
uint32_t const_id = type_inst->GetSingleWordInOperand(1);
Instruction* const_inst = get_def_use_mgr()->GetDef(const_id);
uint32_t cnt = const_inst->GetSingleWordInOperand(0);
return cnt * GetTypeLength(type_inst->GetSingleWordInOperand(0));
}
case SpvOpTypeStruct: {
uint32_t len = 0;
type_inst->ForEachInId([&len, this](const uint32_t* iid) {
// Align struct length
uint32_t alignment = GetTypeAlignment(*iid);
uint32_t mod = len % alignment;
uint32_t diff = (mod != 0) ? alignment - mod : 0;
len += diff;
// Increment struct length by component length
uint32_t comp_len = GetTypeLength(*iid);
len += comp_len;
});
return len;
}
case SpvOpTypeRuntimeArray:
default:
assert(false && "unexpected buffer reference type");
assert(false && "unexpected type");
return 0;
}
}

View File

@ -28,7 +28,9 @@ namespace opt {
// external design of this class may change as the layer evolves.
class InstBuffAddrCheckPass : public InstrumentPass {
public:
// Preferred interface
// For test harness only
InstBuffAddrCheckPass() : InstrumentPass(7, 23, kInstValidationIdBuffAddr) {}
// For all other interfaces
InstBuffAddrCheckPass(uint32_t desc_set, uint32_t shader_id)
: InstrumentPass(desc_set, shader_id, kInstValidationIdBuffAddr) {}
@ -40,8 +42,12 @@ class InstBuffAddrCheckPass : public InstrumentPass {
const char* name() const override { return "inst-bindless-check-pass"; }
private:
// Return byte length of type |type_id|. Must be int, float, vector, matrix
// or physical pointer.
// Return byte alignment of type |type_id|. Must be int, float, vector,
// matrix, struct, array or physical pointer. Uses std430 alignment.
uint32_t GetTypeAlignment(uint32_t type_id);
// Return byte length of type |type_id|. Must be int, float, vector, matrix,
// struct, array or physical pointer. Uses std430 alignment and sizes.
uint32_t GetTypeLength(uint32_t type_id);
// Add |type_id| param to |input_func| and add id to |param_vec|.

View File

@ -615,6 +615,212 @@ OpFunctionEnd
true, 7u, 23u);
}
TEST_F(InstBuffAddrTest, StructLoad) {
// #version 450
// #extension GL_EXT_buffer_reference : enable
// #extension GL_ARB_gpu_shader_int64 : enable
// struct Test {
// float a;
// };
//
// layout(buffer_reference, std430, buffer_reference_align = 16) buffer
// TestBuffer { Test test; };
//
// Test GetTest(uint64_t ptr) {
// return TestBuffer(ptr).test;
// }
//
// void main() {
// GetTest(0xe0000000);
// }
const std::string defs =
R"(
OpCapability Shader
OpCapability Int64
OpCapability PhysicalStorageBufferAddresses
; CHECK: OpExtension "SPV_KHR_storage_buffer_storage_class"
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel PhysicalStorageBuffer64 GLSL450
OpEntryPoint Fragment %main "main"
; CHECK: OpEntryPoint Fragment %main "main" %60 %99 %gl_FragCoord
OpExecutionMode %main OriginUpperLeft
OpSource GLSL 450
OpSourceExtension "GL_ARB_gpu_shader_int64"
OpSourceExtension "GL_EXT_buffer_reference"
OpName %main "main"
OpName %Test "Test"
OpMemberName %Test 0 "a"
OpName %Test_0 "Test"
OpMemberName %Test_0 0 "a"
OpName %TestBuffer "TestBuffer"
OpMemberName %TestBuffer 0 "test"
)";
const std::string decorates =
R"(
OpMemberDecorate %Test_0 0 Offset 0
OpMemberDecorate %TestBuffer 0 Offset 0
OpDecorate %TestBuffer Block
; CHECK: OpDecorate %_runtimearr_ulong ArrayStride 8
; CHECK: OpDecorate %_struct_58 Block
; CHECK: OpMemberDecorate %_struct_58 0 Offset 0
; CHECK: OpDecorate %60 DescriptorSet 7
; CHECK: OpDecorate %60 Binding 2
; CHECK: OpDecorate %_runtimearr_uint ArrayStride 4
; CHECK: OpDecorate %_struct_97 Block
; CHECK: OpMemberDecorate %_struct_97 0 Offset 0
; CHECK: OpMemberDecorate %_struct_97 1 Offset 4
; CHECK: OpDecorate %99 DescriptorSet 7
; CHECK: OpDecorate %99 Binding 0
; CHECK: OpDecorate %gl_FragCoord BuiltIn FragCoord
)";
const std::string globals =
R"(
%void = OpTypeVoid
%3 = OpTypeFunction %void
%ulong = OpTypeInt 64 0
%float = OpTypeFloat 32
%Test = OpTypeStruct %float
OpTypeForwardPointer %_ptr_PhysicalStorageBuffer_TestBuffer PhysicalStorageBuffer
%Test_0 = OpTypeStruct %float
%TestBuffer = OpTypeStruct %Test_0
%_ptr_PhysicalStorageBuffer_TestBuffer = OpTypePointer PhysicalStorageBuffer %TestBuffer
%int = OpTypeInt 32 1
%int_0 = OpConstant %int 0
%_ptr_PhysicalStorageBuffer_Test_0 = OpTypePointer PhysicalStorageBuffer %Test_0
%ulong_18446744073172680704 = OpConstant %ulong 18446744073172680704
; CHECK: %47 = OpTypeFunction %bool %ulong %uint
; CHECK: %_struct_58 = OpTypeStruct %_runtimearr_ulong
; CHECK: %60 = OpVariable %_ptr_StorageBuffer__struct_58 StorageBuffer
; CHECK: %90 = OpTypeFunction %void %uint %uint %uint %uint
; CHECK: %_struct_97 = OpTypeStruct %uint %_runtimearr_uint
; CHECK: %99 = OpVariable %_ptr_StorageBuffer__struct_97 StorageBuffer
; CHECK: %143 = OpConstantNull %Test_0
)";
const std::string main =
R"(
%main = OpFunction %void None %3
%5 = OpLabel
%37 = OpConvertUToPtr %_ptr_PhysicalStorageBuffer_TestBuffer %ulong_18446744073172680704
%38 = OpAccessChain %_ptr_PhysicalStorageBuffer_Test_0 %37 %int_0
%39 = OpLoad %Test_0 %38 Aligned 16
; CHECK-NOT: %39 = OpLoad %Test_0 %38 Aligned 16
; CHECK: %43 = OpConvertPtrToU %ulong %38
; CHECK: %80 = OpFunctionCall %bool %45 %43 %uint_4
; CHECK: OpSelectionMerge %81 None
; CHECK: OpBranchConditional %80 %82 %83
; CHECK: %82 = OpLabel
; CHECK: %84 = OpLoad %Test_0 %38 Aligned 16
; CHECK: OpBranch %81
; CHECK: %83 = OpLabel
; CHECK: %85 = OpUConvert %uint %43
; CHECK: %87 = OpShiftRightLogical %ulong %43 %uint_32
; CHECK: %88 = OpUConvert %uint %87
; CHECK: %142 = OpFunctionCall %void %89 %uint_37 %uint_2 %85 %88
; CHECK: OpBranch %81
; CHECK: %81 = OpLabel
; CHECK: %144 = OpPhi %Test_0 %84 %82 %143 %83
%40 = OpCopyLogical %Test %39
; CHECK-NOT: %40 = OpCopyLogical %Test %39
; CHECK: %40 = OpCopyLogical %Test %144
OpReturn
OpFunctionEnd
)";
const std::string output_funcs =
R"(
; CHECK: %45 = OpFunction %bool None %47
; CHECK: %48 = OpFunctionParameter %ulong
; CHECK: %49 = OpFunctionParameter %uint
; CHECK: %50 = OpLabel
; CHECK: OpBranch %51
; CHECK: %51 = OpLabel
; CHECK: %53 = OpPhi %uint %uint_1 %50 %54 %52
; CHECK: OpLoopMerge %56 %52 None
; CHECK: OpBranch %52
; CHECK: %52 = OpLabel
; CHECK: %54 = OpIAdd %uint %53 %uint_1
; CHECK: %63 = OpAccessChain %_ptr_StorageBuffer_ulong %60 %uint_0 %54
; CHECK: %64 = OpLoad %ulong %63
; CHECK: %65 = OpUGreaterThan %bool %64 %48
; CHECK: OpBranchConditional %65 %56 %51
; CHECK: %56 = OpLabel
; CHECK: %66 = OpISub %uint %54 %uint_1
; CHECK: %67 = OpAccessChain %_ptr_StorageBuffer_ulong %60 %uint_0 %66
; CHECK: %68 = OpLoad %ulong %67
; CHECK: %69 = OpISub %ulong %48 %68
; CHECK: %70 = OpUConvert %ulong %49
; CHECK: %71 = OpIAdd %ulong %69 %70
; CHECK: %72 = OpAccessChain %_ptr_StorageBuffer_ulong %60 %uint_0 %uint_0
; CHECK: %73 = OpLoad %ulong %72
; CHECK: %74 = OpUConvert %uint %73
; CHECK: %75 = OpISub %uint %66 %uint_1
; CHECK: %76 = OpIAdd %uint %75 %74
; CHECK: %77 = OpAccessChain %_ptr_StorageBuffer_ulong %60 %uint_0 %76
; CHECK: %78 = OpLoad %ulong %77
; CHECK: %79 = OpULessThanEqual %bool %71 %78
; CHECK: OpReturnValue %79
; CHECK: OpFunctionEnd
; CHECK: %89 = OpFunction %void None %90
; CHECK: %91 = OpFunctionParameter %uint
; CHECK: %92 = OpFunctionParameter %uint
; CHECK: %93 = OpFunctionParameter %uint
; CHECK: %94 = OpFunctionParameter %uint
; CHECK: %95 = OpLabel
; CHECK: %101 = OpAccessChain %_ptr_StorageBuffer_uint %99 %uint_0
; CHECK: %103 = OpAtomicIAdd %uint %101 %uint_4 %uint_0 %uint_10
; CHECK: %104 = OpIAdd %uint %103 %uint_10
; CHECK: %105 = OpArrayLength %uint %99 1
; CHECK: %106 = OpULessThanEqual %bool %104 %105
; CHECK: OpSelectionMerge %107 None
; CHECK: OpBranchConditional %106 %108 %107
; CHECK: %108 = OpLabel
; CHECK: %109 = OpIAdd %uint %103 %uint_0
; CHECK: %110 = OpAccessChain %_ptr_StorageBuffer_uint %99 %uint_1 %109
; CHECK: OpStore %110 %uint_10
; CHECK: %112 = OpIAdd %uint %103 %uint_1
; CHECK: %113 = OpAccessChain %_ptr_StorageBuffer_uint %99 %uint_1 %112
; CHECK: OpStore %113 %uint_23
; CHECK: %114 = OpIAdd %uint %103 %uint_2
; CHECK: %115 = OpAccessChain %_ptr_StorageBuffer_uint %99 %uint_1 %114
; CHECK: OpStore %115 %91
; CHECK: %117 = OpIAdd %uint %103 %uint_3
; CHECK: %118 = OpAccessChain %_ptr_StorageBuffer_uint %99 %uint_1 %117
; CHECK: OpStore %118 %uint_4
; CHECK: %122 = OpLoad %v4float %gl_FragCoord
; CHECK: %124 = OpBitcast %v4uint %122
; CHECK: %125 = OpCompositeExtract %uint %124 0
; CHECK: %126 = OpIAdd %uint %103 %uint_4
; CHECK: %127 = OpAccessChain %_ptr_StorageBuffer_uint %99 %uint_1 %126
; CHECK: OpStore %127 %125
; CHECK: %128 = OpCompositeExtract %uint %124 1
; CHECK: %130 = OpIAdd %uint %103 %uint_5
; CHECK: %131 = OpAccessChain %_ptr_StorageBuffer_uint %99 %uint_1 %130
; CHECK: OpStore %131 %128
; CHECK: %133 = OpIAdd %uint %103 %uint_7
; CHECK: %134 = OpAccessChain %_ptr_StorageBuffer_uint %99 %uint_1 %133
; CHECK: OpStore %134 %92
; CHECK: %136 = OpIAdd %uint %103 %uint_8
; CHECK: %137 = OpAccessChain %_ptr_StorageBuffer_uint %99 %uint_1 %136
; CHECK: OpStore %137 %93
; CHECK: %139 = OpIAdd %uint %103 %uint_9
; CHECK: %140 = OpAccessChain %_ptr_StorageBuffer_uint %99 %uint_1 %139
; CHECK: OpStore %140 %94
; CHECK: OpBranch %107
; CHECK: %107 = OpLabel
; CHECK: OpReturn
; CHECK: OpFunctionEnd
)";
SetTargetEnv(SPV_ENV_VULKAN_1_2);
SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
SinglePassRunAndMatch<InstBuffAddrCheckPass>(
defs + decorates + globals + main + output_funcs, true);
}
} // namespace
} // namespace opt
} // namespace spvtools