From 44f27f928916f760314381c5a26924408653aca9 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Sun, 7 Jan 2018 10:50:01 -0500 Subject: [PATCH] Allow relaxing validation of pointers in logical addressing mode In HLSL structured buffer legalization, pointer to pointer types are emitted to indicate a structured buffer variable should be treated as an alias of some other variable. We need an option to relax the check of pointer types in logical addressing mode to catch other validation errors. --- include/spirv-tools/libspirv.h | 9 +++++++ include/spirv-tools/libspirv.hpp | 10 +++++++ source/spirv_validator_options.cpp | 5 ++++ source/spirv_validator_options.h | 6 ++++- source/validate_id.cpp | 6 ++++- test/val/val_id_test.cpp | 42 ++++++++++++++++++++++++++++++ tools/val/val.cpp | 4 +++ 7 files changed, 80 insertions(+), 2 deletions(-) diff --git a/include/spirv-tools/libspirv.h b/include/spirv-tools/libspirv.h index ed7b820b2..e8ff4a5ce 100644 --- a/include/spirv-tools/libspirv.h +++ b/include/spirv-tools/libspirv.h @@ -441,6 +441,15 @@ void spvValidatorOptionsSetUniversalLimit(spv_validator_options options, void spvValidatorOptionsSetRelaxStoreStruct(spv_validator_options options, bool val); +// Records whether or not the validator should relax the rules on pointer usage +// in logical addressing mode. +// +// When relaxed, it will allow the following usage cases of pointers: +// 1) OpVariable allocating an object whose type is a pointer type +// 2) OpReturnValue returning a pointer value +void spvValidatorOptionsSetRelaxLogicalPointer(spv_validator_options options, + bool val); + // Encodes the given SPIR-V assembly text to its binary representation. The // length parameter specifies the number of bytes for text. Encoded binary will // be stored into *binary. Any error will be written into *diagnostic if diff --git a/include/spirv-tools/libspirv.hpp b/include/spirv-tools/libspirv.hpp index 6b9ad279f..2e4aa628d 100644 --- a/include/spirv-tools/libspirv.hpp +++ b/include/spirv-tools/libspirv.hpp @@ -81,6 +81,16 @@ class ValidatorOptions { spvValidatorOptionsSetRelaxStoreStruct(options_, val); } + // Records whether or not the validator should relax the rules on pointer + // usage in logical addressing mode. + // + // When relaxed, it will allow the following usage cases of pointers: + // 1) OpVariable allocating an object whose type is a pointer type + // 2) OpReturnValue returning a pointer value + void SetRelaxLogicalPointer(bool val) { + spvValidatorOptionsSetRelaxLogicalPointer(options_, val); + } + private: spv_validator_options options_; }; diff --git a/source/spirv_validator_options.cpp b/source/spirv_validator_options.cpp index e676c5e22..144a16827 100644 --- a/source/spirv_validator_options.cpp +++ b/source/spirv_validator_options.cpp @@ -81,3 +81,8 @@ void spvValidatorOptionsSetRelaxStoreStruct(spv_validator_options options, bool val) { options->relax_struct_store = val; } + +void spvValidatorOptionsSetRelaxLogicalPointer(spv_validator_options options, + bool val) { + options->relax_logcial_pointer = val; +} diff --git a/source/spirv_validator_options.h b/source/spirv_validator_options.h index 4a67812ab..d15b63bb0 100644 --- a/source/spirv_validator_options.h +++ b/source/spirv_validator_options.h @@ -37,10 +37,14 @@ struct validator_universal_limits_t { // Manages command line options passed to the SPIR-V Validator. New struct // members may be added for any new option. struct spv_validator_options_t { - spv_validator_options_t() : universal_limits_(), relax_struct_store(false) {} + spv_validator_options_t() + : universal_limits_(), + relax_struct_store(false), + relax_logcial_pointer(false) {} validator_universal_limits_t universal_limits_; bool relax_struct_store; + bool relax_logcial_pointer; }; #endif // LIBSPIRV_SPIRV_VALIDATOR_OPTIONS_H_ diff --git a/source/validate_id.cpp b/source/validate_id.cpp index 8b2eee0b8..2267f382e 100644 --- a/source/validate_id.cpp +++ b/source/validate_id.cpp @@ -1879,16 +1879,20 @@ bool idUsage::isValid(const spv_instruction_t* inst, << "' is missing or void."; return false; } + const bool uses_variable_pointer = module_.features().variable_pointers || module_.features().variable_pointers_storage_buffer; + if (addressingModel == SpvAddressingModelLogical && - SpvOpTypePointer == valueType->opcode() && !uses_variable_pointer) { + SpvOpTypePointer == valueType->opcode() && !uses_variable_pointer && + !module_.options()->relax_logcial_pointer) { DIAG(valueIndex) << "OpReturnValue value's type '" << value->type_id() << "' is a pointer, which is invalid in the Logical addressing model."; return false; } + // NOTE: Find OpFunction const spv_instruction_t* function = inst - 1; while (firstInst != function) { diff --git a/test/val/val_id_test.cpp b/test/val/val_id_test.cpp index 3bf11a42d..8278b16f7 100644 --- a/test/val/val_id_test.cpp +++ b/test/val/val_id_test.cpp @@ -2426,6 +2426,48 @@ TEST_F(ValidateIdWithMessage, OpStoreTypeBadRelaxedStruct2) { " '16's layout.")); } +TEST_F(ValidateIdWithMessage, OpStoreTypeRelaxedLogicalPointerReturnPointer) { + const string spirv = R"( + OpCapability Shader + OpCapability Linkage + OpMemoryModel Logical GLSL450 +%1 = OpTypeInt 32 1 +%2 = OpTypePointer Function %1 +%3 = OpTypeFunction %2 %2 +%4 = OpFunction %2 None %3 +%5 = OpFunctionParameter %2 +%6 = OpLabel + OpReturnValue %5 + OpFunctionEnd)"; + + spvValidatorOptionsSetRelaxLogicalPointer(options_, true); + CompileSuccessfully(spirv.c_str()); + EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +TEST_F(ValidateIdWithMessage, OpStoreTypeRelaxedLogicalPointerAllocPointer) { + const string spirv = R"( + OpCapability Shader + OpCapability Linkage + OpMemoryModel Logical GLSL450 + %1 = OpTypeVoid + %2 = OpTypeInt 32 1 + %3 = OpTypeFunction %1 ; void(void) + %4 = OpTypePointer Uniform %2 ; int* + %5 = OpTypePointer Private %4 ; int** (Private) + %6 = OpTypePointer Function %4 ; int** (Function) + %7 = OpVariable %5 Private + %8 = OpFunction %1 None %3 + %9 = OpLabel +%10 = OpVariable %6 Function + OpReturn + OpFunctionEnd)"; + + spvValidatorOptionsSetRelaxLogicalPointer(options_, true); + CompileSuccessfully(spirv.c_str()); + EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + TEST_F(ValidateIdWithMessage, OpStoreVoid) { string spirv = kGLSL450MemoryModel + R"( %1 = OpTypeVoid diff --git a/tools/val/val.cpp b/tools/val/val.cpp index 050e7d68c..633d2f6e4 100644 --- a/tools/val/val.cpp +++ b/tools/val/val.cpp @@ -44,6 +44,8 @@ Options: --max-function-args --max-control-flow-nesting-depth --max-access-chain-indexes + --relax-logcial-pointer Allow allocating an object of a pointer type and returning + a pointer value from a function in logical addressing mode --relax-struct-store Allow store from one struct type to a different type with compatible layout and members. @@ -112,6 +114,8 @@ int main(int argc, char** argv) { continue_processing = false; return_code = 1; } + } else if (0 == strcmp(cur_arg, "--relax-logical-pointer")) { + options.SetRelaxLogicalPointer(true); } else if (0 == strcmp(cur_arg, "--relax-struct-store")) { options.SetRelaxStructStore(true); } else if (0 == cur_arg[1]) {