From 1346dd5de119d603686e260daf08f36958909a23 Mon Sep 17 00:00:00 2001 From: alan-baker Date: Mon, 23 Mar 2020 16:59:37 -0400 Subject: [PATCH] Disallow phis of images, samplers and sampled images (#3246) * Disallow phis of sampled images always * Disallow phis of samplers and images for shaders * Add tests * Gate check to only occur post-legalization --- source/val/validate_cfg.cpp | 9 ++ test/val/val_cfg_test.cpp | 207 ++++++++++++++++++++++++++++++++++++ 2 files changed, 216 insertions(+) diff --git a/source/val/validate_cfg.cpp b/source/val/validate_cfg.cpp index f3019d17f..1c279f654 100644 --- a/source/val/validate_cfg.cpp +++ b/source/val/validate_cfg.cpp @@ -62,6 +62,15 @@ spv_result_t ValidatePhi(ValidationState_t& _, const Instruction* inst) { } } + if (!_.options()->before_hlsl_legalization) { + if (type_opcode == SpvOpTypeSampledImage || + (_.HasCapability(SpvCapabilityShader) && + (type_opcode == SpvOpTypeImage || type_opcode == SpvOpTypeSampler))) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "Result type cannot be Op" << spvOpcodeString(type_opcode); + } + } + // Create a uniqued vector of predecessor ids for comparison against // incoming values. OpBranchConditional %cond %label %label produces two // predecessors in the CFG. diff --git a/test/val/val_cfg_test.cpp b/test/val/val_cfg_test.cpp index 4cf402967..0d09642a3 100644 --- a/test/val/val_cfg_test.cpp +++ b/test/val/val_cfg_test.cpp @@ -4296,6 +4296,213 @@ TEST_F(ValidateCFG, ExitFromConstructWhoseHeaderIsAMerge2) { EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()); } +TEST_F(ValidateCFG, PhiResultInvalidSampler) { + const std::string text = R"( +OpCapability Shader +OpCapability Linkage +OpMemoryModel Logical GLSL450 +%void = OpTypeVoid +%bool = OpTypeBool +%f32 = OpTypeFloat 32 +%sampler = OpTypeSampler +%ptr_uc_sampler = OpTypePointer UniformConstant %sampler +%sampler_var = OpVariable %ptr_uc_sampler UniformConstant +%undef_bool = OpUndef %bool +%undef_sampler = OpUndef %sampler +%void_fn = OpTypeFunction %void +%fn = OpFunction %void None %void_fn +%entry = OpLabel +%ld_sampler = OpLoad %sampler %sampler_var +OpBranch %loop +%loop = OpLabel +%phi = OpPhi %sampler %undef_sampler %entry %ld_sampler %loop +OpLoopMerge %exit %loop None +OpBranchConditional %undef_bool %exit %loop +%exit = OpLabel +OpReturn +OpFunctionEnd +)"; + + CompileSuccessfully(text); + ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("Result type cannot be OpTypeSampler")); +} + +TEST_F(ValidateCFG, PhiResultInvalidImage) { + const std::string text = R"( +OpCapability Shader +OpCapability Linkage +OpMemoryModel Logical GLSL450 +%void = OpTypeVoid +%bool = OpTypeBool +%f32 = OpTypeFloat 32 +%image = OpTypeImage %f32 2D 0 0 0 1 Rgba32f +%ptr_uc_image = OpTypePointer UniformConstant %image +%image_var = OpVariable %ptr_uc_image UniformConstant +%undef_bool = OpUndef %bool +%undef_image = OpUndef %image +%void_fn = OpTypeFunction %void +%fn = OpFunction %void None %void_fn +%entry = OpLabel +%ld_image = OpLoad %image %image_var +OpBranch %loop +%loop = OpLabel +%phi = OpPhi %image %undef_image %entry %ld_image %loop +OpLoopMerge %exit %loop None +OpBranchConditional %undef_bool %exit %loop +%exit = OpLabel +OpReturn +OpFunctionEnd +)"; + + CompileSuccessfully(text); + ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("Result type cannot be OpTypeImage")); +} + +TEST_F(ValidateCFG, PhiResultInvalidSampledImage) { + const std::string text = R"( +OpCapability Shader +OpCapability Linkage +OpMemoryModel Logical GLSL450 +%void = OpTypeVoid +%bool = OpTypeBool +%f32 = OpTypeFloat 32 +%sampler = OpTypeSampler +%ptr_uc_sampler = OpTypePointer UniformConstant %sampler +%sampler_var = OpVariable %ptr_uc_sampler UniformConstant +%image = OpTypeImage %f32 2D 0 0 0 1 Rgba32f +%ptr_uc_image = OpTypePointer UniformConstant %image +%image_var = OpVariable %ptr_uc_image UniformConstant +%sampled_image = OpTypeSampledImage %image +%undef_bool = OpUndef %bool +%undef_sampled_image = OpUndef %sampled_image +%void_fn = OpTypeFunction %void +%fn = OpFunction %void None %void_fn +%entry = OpLabel +%ld_image = OpLoad %image %image_var +%ld_sampler = OpLoad %sampler %sampler_var +OpBranch %loop +%loop = OpLabel +%phi = OpPhi %sampled_image %undef_sampled_image %entry %sample %loop +%sample = OpSampledImage %sampled_image %ld_image %ld_sampler +OpLoopMerge %exit %loop None +OpBranchConditional %undef_bool %exit %loop +%exit = OpLabel +OpReturn +OpFunctionEnd +)"; + + CompileSuccessfully(text); + ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("Result type cannot be OpTypeSampledImage")); +} + +TEST_F(ValidateCFG, PhiResultValidPreLegalizationSampler) { + const std::string text = R"( +OpCapability Shader +OpCapability Linkage +OpMemoryModel Logical GLSL450 +%void = OpTypeVoid +%bool = OpTypeBool +%f32 = OpTypeFloat 32 +%sampler = OpTypeSampler +%ptr_uc_sampler = OpTypePointer UniformConstant %sampler +%sampler_var = OpVariable %ptr_uc_sampler UniformConstant +%undef_bool = OpUndef %bool +%undef_sampler = OpUndef %sampler +%void_fn = OpTypeFunction %void +%fn = OpFunction %void None %void_fn +%entry = OpLabel +%ld_sampler = OpLoad %sampler %sampler_var +OpBranch %loop +%loop = OpLabel +%phi = OpPhi %sampler %undef_sampler %entry %ld_sampler %loop +OpLoopMerge %exit %loop None +OpBranchConditional %undef_bool %exit %loop +%exit = OpLabel +OpReturn +OpFunctionEnd +)"; + + options_->before_hlsl_legalization = true; + CompileSuccessfully(text); + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +TEST_F(ValidateCFG, PhiResultValidPreLegalizationImage) { + const std::string text = R"( +OpCapability Shader +OpCapability Linkage +OpMemoryModel Logical GLSL450 +%void = OpTypeVoid +%bool = OpTypeBool +%f32 = OpTypeFloat 32 +%image = OpTypeImage %f32 2D 0 0 0 1 Rgba32f +%ptr_uc_image = OpTypePointer UniformConstant %image +%image_var = OpVariable %ptr_uc_image UniformConstant +%undef_bool = OpUndef %bool +%undef_image = OpUndef %image +%void_fn = OpTypeFunction %void +%fn = OpFunction %void None %void_fn +%entry = OpLabel +%ld_image = OpLoad %image %image_var +OpBranch %loop +%loop = OpLabel +%phi = OpPhi %image %undef_image %entry %ld_image %loop +OpLoopMerge %exit %loop None +OpBranchConditional %undef_bool %exit %loop +%exit = OpLabel +OpReturn +OpFunctionEnd +)"; + + options_->before_hlsl_legalization = true; + CompileSuccessfully(text); + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +TEST_F(ValidateCFG, PhiResultValidPreLegalizationSampledImage) { + const std::string text = R"( +OpCapability Shader +OpCapability Linkage +OpMemoryModel Logical GLSL450 +%void = OpTypeVoid +%bool = OpTypeBool +%f32 = OpTypeFloat 32 +%sampler = OpTypeSampler +%ptr_uc_sampler = OpTypePointer UniformConstant %sampler +%sampler_var = OpVariable %ptr_uc_sampler UniformConstant +%image = OpTypeImage %f32 2D 0 0 0 1 Rgba32f +%ptr_uc_image = OpTypePointer UniformConstant %image +%image_var = OpVariable %ptr_uc_image UniformConstant +%sampled_image = OpTypeSampledImage %image +%undef_bool = OpUndef %bool +%undef_sampled_image = OpUndef %sampled_image +%void_fn = OpTypeFunction %void +%fn = OpFunction %void None %void_fn +%entry = OpLabel +%ld_image = OpLoad %image %image_var +%ld_sampler = OpLoad %sampler %sampler_var +OpBranch %loop +%loop = OpLabel +%phi = OpPhi %sampled_image %undef_sampled_image %entry %sample %loop +%sample = OpSampledImage %sampled_image %ld_image %ld_sampler +OpLoopMerge %exit %loop None +OpBranchConditional %undef_bool %exit %loop +%exit = OpLabel +OpReturn +OpFunctionEnd +)"; + + options_->before_hlsl_legalization = true; + CompileSuccessfully(text); + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + } // namespace } // namespace val } // namespace spvtools