Skip to content

Commit 2aa281c

Browse files
spencer-lunargspencer-lunarg
authored
spirv-val: Add U/S/FConvert SpecConstantOp (KhronosGroup#6576)
First few of KhronosGroup#6564 I chose `UConvert`/`SConvert`/`FConvert` as I knew they had the most "needed to fix in other tests" such that future additions are more "additive" only from here
1 parent 05ac2f3 commit 2aa281c

2 files changed

Lines changed: 103 additions & 49 deletions

File tree

‎source/val/validate_conversion.cpp‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,8 @@ spv_result_t ValidateConvertIntToF(ValidationState_t& _,
166166
return SPV_SUCCESS;
167167
}
168168

169-
spv_result_t ValidateUConvert(ValidationState_t& _, const Instruction* inst) {
169+
spv_result_t ValidateUConvert(ValidationState_t& _, const Instruction* inst,
170+
uint32_t operand_index = 2) {
170171
const spv::Op opcode = inst->opcode();
171172
const uint32_t result_type = inst->type_id();
172173
if (!_.IsUnsignedIntScalarType(result_type) &&
@@ -177,7 +178,7 @@ spv_result_t ValidateUConvert(ValidationState_t& _, const Instruction* inst) {
177178
<< "Expected unsigned int scalar or vector type as Result Type: "
178179
<< spvOpcodeString(opcode);
179180

180-
const uint32_t input_type = _.GetOperandTypeId(inst, 2);
181+
const uint32_t input_type = _.GetOperandTypeId(inst, operand_index);
181182
if (!input_type ||
182183
(!_.IsIntScalarType(input_type) && !_.IsIntVectorType(input_type) &&
183184
!_.IsIntCooperativeMatrixType(input_type) &&
@@ -211,7 +212,8 @@ spv_result_t ValidateUConvert(ValidationState_t& _, const Instruction* inst) {
211212
return SPV_SUCCESS;
212213
}
213214

214-
spv_result_t ValidateSConvert(ValidationState_t& _, const Instruction* inst) {
215+
spv_result_t ValidateSConvert(ValidationState_t& _, const Instruction* inst,
216+
uint32_t operand_index = 2) {
215217
const spv::Op opcode = inst->opcode();
216218
const uint32_t result_type = inst->type_id();
217219
if (!_.IsIntScalarType(result_type) && !_.IsIntVectorType(result_type) &&
@@ -221,7 +223,7 @@ spv_result_t ValidateSConvert(ValidationState_t& _, const Instruction* inst) {
221223
<< "Expected int scalar or vector type as Result Type: "
222224
<< spvOpcodeString(opcode);
223225

224-
const uint32_t input_type = _.GetOperandTypeId(inst, 2);
226+
const uint32_t input_type = _.GetOperandTypeId(inst, operand_index);
225227
if (!input_type ||
226228
(!_.IsIntScalarType(input_type) && !_.IsIntVectorType(input_type) &&
227229
!_.IsIntCooperativeMatrixType(input_type) &&
@@ -255,7 +257,8 @@ spv_result_t ValidateSConvert(ValidationState_t& _, const Instruction* inst) {
255257
return SPV_SUCCESS;
256258
}
257259

258-
spv_result_t ValidateFConvert(ValidationState_t& _, const Instruction* inst) {
260+
spv_result_t ValidateFConvert(ValidationState_t& _, const Instruction* inst,
261+
uint32_t operand_index = 2) {
259262
const spv::Op opcode = inst->opcode();
260263
const uint32_t result_type = inst->type_id();
261264
if (!_.IsFloatScalarType(result_type) && !_.IsFloatVectorType(result_type) &&
@@ -265,7 +268,7 @@ spv_result_t ValidateFConvert(ValidationState_t& _, const Instruction* inst) {
265268
<< "Expected float scalar or vector type as Result Type: "
266269
<< spvOpcodeString(opcode);
267270

268-
const uint32_t input_type = _.GetOperandTypeId(inst, 2);
271+
const uint32_t input_type = _.GetOperandTypeId(inst, operand_index);
269272
if (!input_type ||
270273
(!_.IsFloatScalarType(input_type) && !_.IsFloatVectorType(input_type) &&
271274
!_.IsFloatCooperativeMatrixType(input_type) &&
@@ -831,6 +834,20 @@ spv_result_t ConversionPass(ValidationState_t& _, const Instruction* inst) {
831834
return ValidateCooperativeMatrix(_, inst);
832835
case spv::Op::OpBitCastArrayQCOM:
833836
return ValidateBitCastArray(_, inst);
837+
838+
case spv::Op::OpSpecConstantOp: {
839+
switch (inst->GetOperandAs<spv::Op>(2u)) {
840+
case spv::Op::OpUConvert:
841+
return ValidateUConvert(_, inst, 3);
842+
case spv::Op::OpSConvert:
843+
return ValidateSConvert(_, inst, 3);
844+
case spv::Op::OpFConvert:
845+
return ValidateFConvert(_, inst, 3);
846+
default:
847+
break;
848+
}
849+
break;
850+
}
834851
default:
835852
break;
836853
}

‎test/val/val_constants_test.cpp‎

Lines changed: 80 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -37,37 +37,47 @@ using ::testing::ValuesIn;
3737

3838
using ValidateConstant = spvtest::ValidateBase<bool>;
3939

40-
#define kBasicTypes \
41-
"%bool = OpTypeBool " \
42-
"%uint = OpTypeInt 32 0 " \
43-
"%uint2 = OpTypeVector %uint 2 " \
44-
"%float = OpTypeFloat 32 " \
45-
"%_ptr_uint = OpTypePointer Workgroup %uint " \
46-
"%uint_0 = OpConstantNull %uint " \
47-
"%uint2_0 = OpConstantNull %uint " \
48-
"%float_0 = OpConstantNull %float " \
49-
"%false = OpConstantFalse %bool " \
50-
"%true = OpConstantTrue %bool " \
40+
#define kBasicTypes \
41+
"%bool = OpTypeBool " \
42+
"%uint = OpTypeInt 32 0 " \
43+
"%uint64 = OpTypeInt 64 0 " \
44+
"%uint2 = OpTypeVector %uint 2 " \
45+
"%float = OpTypeFloat 32 " \
46+
"%float64 = OpTypeFloat 64 " \
47+
"%_ptr_uint = OpTypePointer Workgroup %uint " \
48+
"%uint_0 = OpConstantNull %uint " \
49+
"%uint64_0 = OpConstantNull %uint64 " \
50+
"%uint2_0 = OpConstantComposite %uint2 %uint_0 %uint_0 " \
51+
"%float_0 = OpConstantNull %float " \
52+
"%float64_0 = OpConstantNull %float64 " \
53+
"%false = OpConstantFalse %bool " \
54+
"%true = OpConstantTrue %bool " \
5155
"%null = OpConstantNull %_ptr_uint "
5256

5357
#define kShaderPreamble \
5458
"OpCapability Shader\n" \
5559
"OpCapability Linkage\n" \
60+
"OpCapability Int64\n" \
61+
"OpCapability Float64\n" \
5662
"OpCapability VariablePointers\n" \
5763
"OpExtension \"SPV_KHR_variable_pointers\"\n" \
5864
"OpMemoryModel Logical Simple\n"
5965

60-
#define kKernelPreamble \
61-
"OpCapability Kernel\n" \
62-
"OpCapability Linkage\n" \
63-
"OpCapability Addresses\n" \
66+
#define kKernelPreamble \
67+
"OpCapability Kernel\n" \
68+
"OpCapability Linkage\n" \
69+
"OpCapability Int64\n" \
70+
"OpCapability Float64\n" \
71+
"OpCapability GenericPointer\n" \
72+
"OpCapability Addresses\n" \
6473
"OpMemoryModel Physical32 OpenCL\n"
6574

6675
struct ConstantOpCase {
6776
spv_target_env env;
6877
std::string assembly;
6978
bool expect_success;
7079
std::string expect_err;
80+
spv_result_t expected_result = SPV_ERROR_INVALID_ID;
7181
};
7282

7383
using ValidateConstantOp = spvtest::ValidateBase<ConstantOpCase>;
@@ -80,7 +90,7 @@ TEST_P(ValidateConstantOp, Samples) {
8090
EXPECT_EQ(SPV_SUCCESS, result);
8191
EXPECT_THAT(getDiagnosticString(), Eq(""));
8292
} else {
83-
EXPECT_EQ(SPV_ERROR_INVALID_ID, result);
93+
EXPECT_EQ(GetParam().expected_result, result);
8494
EXPECT_THAT(getDiagnosticString(), HasSubstr(GetParam().expect_err));
8595
}
8696
}
@@ -92,9 +102,8 @@ TEST_P(ValidateConstantOp, Samples) {
92102
INSTANTIATE_TEST_SUITE_P(
93103
UniversalInShader, ValidateConstantOp,
94104
ValuesIn(std::vector<ConstantOpCase>{
95-
// TODO(dneto): Conversions must change width.
96-
GOOD_SHADER_10("%v = OpSpecConstantOp %uint SConvert %uint_0"),
97-
GOOD_SHADER_10("%v = OpSpecConstantOp %float FConvert %float_0"),
105+
GOOD_SHADER_10("%v = OpSpecConstantOp %uint SConvert %uint64_0"),
106+
GOOD_SHADER_10("%v = OpSpecConstantOp %float FConvert %float64_0"),
98107
GOOD_SHADER_10("%v = OpSpecConstantOp %uint SNegate %uint_0"),
99108
GOOD_SHADER_10("%v = OpSpecConstantOp %uint Not %uint_0"),
100109
GOOD_SHADER_10("%v = OpSpecConstantOp %uint IAdd %uint_0 %uint_0"),
@@ -149,9 +158,8 @@ INSTANTIATE_TEST_SUITE_P(
149158
INSTANTIATE_TEST_SUITE_P(
150159
UniversalInKernel, ValidateConstantOp,
151160
ValuesIn(std::vector<ConstantOpCase>{
152-
// TODO(dneto): Conversions must change width.
153-
GOOD_KERNEL_10("%v = OpSpecConstantOp %uint SConvert %uint_0"),
154-
GOOD_KERNEL_10("%v = OpSpecConstantOp %float FConvert %float_0"),
161+
GOOD_KERNEL_10("%v = OpSpecConstantOp %uint SConvert %uint64_0"),
162+
GOOD_KERNEL_10("%v = OpSpecConstantOp %float FConvert %float64_0"),
155163
GOOD_KERNEL_10("%v = OpSpecConstantOp %uint SNegate %uint_0"),
156164
GOOD_KERNEL_10("%v = OpSpecConstantOp %uint Not %uint_0"),
157165
GOOD_KERNEL_10("%v = OpSpecConstantOp %uint IAdd %uint_0 %uint_0"),
@@ -206,66 +214,64 @@ INSTANTIATE_TEST_SUITE_P(
206214
INSTANTIATE_TEST_SUITE_P(
207215
UConvert, ValidateConstantOp,
208216
ValuesIn(std::vector<ConstantOpCase>{
209-
// TODO(dneto): Conversions must change width.
210217
{SPV_ENV_UNIVERSAL_1_0,
211218
kKernelPreamble kBasicTypes
212-
"%v = OpSpecConstantOp %uint UConvert %uint_0",
219+
"%v = OpSpecConstantOp %uint UConvert %uint64_0",
213220
true, ""},
214221
{SPV_ENV_UNIVERSAL_1_1,
215222
kKernelPreamble kBasicTypes
216-
"%v = OpSpecConstantOp %uint UConvert %uint_0",
223+
"%v = OpSpecConstantOp %uint UConvert %uint64_0",
217224
true, ""},
218225
{SPV_ENV_UNIVERSAL_1_3,
219226
kKernelPreamble kBasicTypes
220-
"%v = OpSpecConstantOp %uint UConvert %uint_0",
227+
"%v = OpSpecConstantOp %uint UConvert %uint64_0",
221228
true, ""},
222229
{SPV_ENV_UNIVERSAL_1_3,
223230
kKernelPreamble kBasicTypes
224-
"%v = OpSpecConstantOp %uint UConvert %uint_0",
231+
"%v = OpSpecConstantOp %uint UConvert %uint64_0",
225232
true, ""},
226233
{SPV_ENV_UNIVERSAL_1_4,
227234
kKernelPreamble kBasicTypes
228-
"%v = OpSpecConstantOp %uint UConvert %uint_0",
235+
"%v = OpSpecConstantOp %uint UConvert %uint64_0",
229236
true, ""},
230237
{SPV_ENV_UNIVERSAL_1_0,
231238
kShaderPreamble kBasicTypes
232-
"%v = OpSpecConstantOp %uint UConvert %uint_0",
239+
"%v = OpSpecConstantOp %uint UConvert %uint64_0",
233240
false,
234241
"Prior to SPIR-V 1.4, specialization constant operation "
235242
"UConvert requires Kernel capability"},
236243
{SPV_ENV_UNIVERSAL_1_1,
237244
kShaderPreamble kBasicTypes
238-
"%v = OpSpecConstantOp %uint UConvert %uint_0",
245+
"%v = OpSpecConstantOp %uint UConvert %uint64_0",
239246
false,
240247
"Prior to SPIR-V 1.4, specialization constant operation "
241248
"UConvert requires Kernel capability"},
242249
{SPV_ENV_UNIVERSAL_1_3,
243250
kShaderPreamble kBasicTypes
244-
"%v = OpSpecConstantOp %uint UConvert %uint_0",
251+
"%v = OpSpecConstantOp %uint UConvert %uint64_0",
245252
false,
246253
"Prior to SPIR-V 1.4, specialization constant operation "
247254
"UConvert requires Kernel capability"},
248255
{SPV_ENV_UNIVERSAL_1_3,
249256
kShaderPreamble kBasicTypes
250-
"%v = OpSpecConstantOp %uint UConvert %uint_0",
257+
"%v = OpSpecConstantOp %uint UConvert %uint64_0",
251258
false,
252259
"Prior to SPIR-V 1.4, specialization constant operation "
253260
"UConvert requires Kernel capability"},
254261
{SPV_ENV_UNIVERSAL_1_4,
255262
kShaderPreamble kBasicTypes
256-
"%v = OpSpecConstantOp %uint UConvert %uint_0",
263+
"%v = OpSpecConstantOp %uint UConvert %uint64_0",
257264
true, ""},
258265
}));
259266

260267
INSTANTIATE_TEST_SUITE_P(
261268
KernelInKernel, ValidateConstantOp,
262269
ValuesIn(std::vector<ConstantOpCase>{
263-
// TODO(dneto): Conversions must change width.
264270
GOOD_KERNEL_10("%v = OpSpecConstantOp %uint ConvertFToS %float_0"),
265271
GOOD_KERNEL_10("%v = OpSpecConstantOp %float ConvertSToF %uint_0"),
266272
GOOD_KERNEL_10("%v = OpSpecConstantOp %uint ConvertFToU %float_0"),
267273
GOOD_KERNEL_10("%v = OpSpecConstantOp %float ConvertUToF %uint_0"),
268-
GOOD_KERNEL_10("%v = OpSpecConstantOp %uint UConvert %uint_0"),
274+
GOOD_KERNEL_10("%v = OpSpecConstantOp %uint UConvert %uint64_0"),
269275
GOOD_KERNEL_10(
270276
"%v = OpSpecConstantOp %_ptr_uint GenericCastToPtr %null"),
271277
GOOD_KERNEL_10(
@@ -297,7 +303,8 @@ INSTANTIATE_TEST_SUITE_P(
297303
INSTANTIATE_TEST_SUITE_P(
298304
KernelInShader, ValidateConstantOp,
299305
ValuesIn(std::vector<ConstantOpCase>{
300-
// TODO(dneto): Conversions must change width.
306+
// Don't need to test GenericCastToPtr or PtrCastToGeneric as a valid
307+
// module can't have a Generic storage class with Shader capability
301308
BAD_SHADER_10("%v = OpSpecConstantOp %uint ConvertFToS %float_0",
302309
"ConvertFToS"),
303310
BAD_SHADER_10("%v = OpSpecConstantOp %float ConvertSToF %uint_0",
@@ -306,10 +313,6 @@ INSTANTIATE_TEST_SUITE_P(
306313
"ConvertFToU"),
307314
BAD_SHADER_10("%v = OpSpecConstantOp %float ConvertUToF %uint_0",
308315
"ConvertUToF"),
309-
BAD_SHADER_10("%v = OpSpecConstantOp %_ptr_uint GenericCastToPtr %null",
310-
"GenericCastToPtr"),
311-
BAD_SHADER_10("%v = OpSpecConstantOp %_ptr_uint PtrCastToGeneric %null",
312-
"PtrCastToGeneric"),
313316
BAD_SHADER_10("%v = OpSpecConstantOp %uint Bitcast %uint_0", "Bitcast"),
314317
BAD_SHADER_10("%v = OpSpecConstantOp %float FNegate %float_0",
315318
"FNegate"),
@@ -346,12 +349,14 @@ INSTANTIATE_TEST_SUITE_P(
346349
// https://github.com/KhronosGroup/glslang/issues/848
347350
{SPV_ENV_UNIVERSAL_1_0,
348351
"OpCapability Shader\n"
352+
"OpCapability Int64\n"
353+
"OpCapability Float64\n"
349354
"OpCapability VariablePointers\n"
350355
"OpCapability Linkage ; So we don't need to define a function\n"
351356
"OpExtension \"SPV_AMD_gpu_shader_int16\"\n"
352357
"OpExtension \"SPV_KHR_variable_pointers\"\n"
353358
"OpMemoryModel Logical Simple " kBasicTypes
354-
"%v = OpSpecConstantOp %uint UConvert %uint_0",
359+
"%v = OpSpecConstantOp %uint UConvert %uint64_0",
355360
true, ""},
356361
}));
357362

@@ -494,10 +499,42 @@ TEST_F(ValidateConstant, VectorMismatchedConstituents) {
494499
EXPECT_THAT(
495500
getDiagnosticString(),
496501
HasSubstr(
497-
"OpConstantComposite Constituent <id> '13[%13]'s type "
498-
"does not match Result Type <id> '3[%v2uint]'s vector element type"));
502+
"OpConstantComposite Constituent <id> '17[%17]'s type "
503+
"does not match Result Type <id> '4[%v2uint]'s vector element type"));
499504
}
500505

506+
#define BAD_KERNEL_OPERANDS(STR, ERR) \
507+
{ \
508+
SPV_ENV_UNIVERSAL_1_0, kKernelPreamble kBasicTypes STR, false, ERR, \
509+
SPV_ERROR_INVALID_DATA \
510+
}
511+
512+
// 2 of each, first has bad return type, second has bad operand
513+
INSTANTIATE_TEST_SUITE_P(
514+
BadOperandsKernel, ValidateConstantOp,
515+
ValuesIn(std::vector<ConstantOpCase>{
516+
BAD_KERNEL_OPERANDS(
517+
"%v = OpSpecConstantOp %float SConvert %uint_0",
518+
"Expected int scalar or vector type as Result Type"),
519+
BAD_KERNEL_OPERANDS(
520+
"%v = OpSpecConstantOp %uint SConvert %uint_0",
521+
"Expected input to have different bit width from Result Type"),
522+
523+
BAD_KERNEL_OPERANDS(
524+
"%v = OpSpecConstantOp %uint FConvert %float_0",
525+
"Expected float scalar or vector type as Result Type"),
526+
BAD_KERNEL_OPERANDS("%v = OpSpecConstantOp %float FConvert %float_0",
527+
"Expected component type of Value to be different "
528+
"from component type of Result Type"),
529+
530+
BAD_KERNEL_OPERANDS(
531+
"%v = OpSpecConstantOp %float UConvert %uint_0",
532+
"Expected unsigned int scalar or vector type as Result Type"),
533+
BAD_KERNEL_OPERANDS(
534+
"%v = OpSpecConstantOp %uint UConvert %uint_0",
535+
"Expected input to have different bit width from Result Type"),
536+
}));
537+
501538
} // namespace
502539
} // namespace val
503540
} // namespace spvtools

0 commit comments

Comments
 (0)