Skip to content

Commit fc4ff0e

Browse files
antiagainstdneto0
antiagainst
authored and
dneto0
committed
Pass environment version into SPIRV-Tools properly
Otherwise the assembling/disassembling/optimizing may error out because of target environment version mismatch. Fixes google#475
1 parent 5804d8c commit fc4ff0e

File tree

9 files changed

+184
-72
lines changed

9 files changed

+184
-72
lines changed

‎libshaderc/src/common_shaders_for_test.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,16 @@ const char kHlslShaderWithCounterBuffer[] =
310310
return float4(Ainc.IncrementCounter(), 0, 0, 0);
311311
})";
312312

313+
const char kHlslWaveActiveSumeComputeShader[] =
314+
R"(struct S { uint val; uint result; };
315+
316+
RWStructuredBuffer<S> MyBuffer;
317+
318+
[numthreads(32, 1, 1)]
319+
void main(uint3 id : SV_DispatchThreadID) {
320+
MyBuffer[id.x].result = WaveActiveSum(MyBuffer[id.x].val);
321+
})";
322+
313323
#ifdef __cplusplus
314324
}
315325
#endif // __cplusplus

‎libshaderc/src/shaderc.cc‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,26 @@ shaderc_util::Compiler::TargetEnv GetCompilerTargetEnv(shaderc_target_env env) {
294294
return shaderc_util::Compiler::TargetEnv::Vulkan;
295295
}
296296

297+
shaderc_util::Compiler::TargetEnvVersion GetCompilerTargetEnvVersion(
298+
uint32_t version_number) {
299+
using namespace shaderc_util;
300+
301+
if (static_cast<uint32_t>(Compiler::TargetEnvVersion::Vulkan_1_0) ==
302+
version_number) {
303+
return Compiler::TargetEnvVersion::Vulkan_1_0;
304+
}
305+
if (static_cast<uint32_t>(Compiler::TargetEnvVersion::Vulkan_1_1) ==
306+
version_number) {
307+
return Compiler::TargetEnvVersion::Vulkan_1_1;
308+
}
309+
if (static_cast<uint32_t>(Compiler::TargetEnvVersion::OpenGL_4_5) ==
310+
version_number) {
311+
return Compiler::TargetEnvVersion::OpenGL_4_5;
312+
}
313+
314+
return Compiler::TargetEnvVersion::Default;
315+
}
316+
297317
// Returns the Compiler::Limit enum for the given shaderc_limit enum.
298318
shaderc_util::Compiler::Limit CompilerLimit(shaderc_limit limit) {
299319
switch (limit) {
@@ -452,7 +472,8 @@ void shaderc_compile_options_set_target_env(shaderc_compile_options_t options,
452472
shaderc_target_env target,
453473
uint32_t version) {
454474
options->target_env = target;
455-
options->compiler.SetTargetEnv(GetCompilerTargetEnv(target), version);
475+
options->compiler.SetTargetEnv(GetCompilerTargetEnv(target),
476+
GetCompilerTargetEnvVersion(version));
456477
}
457478

458479
void shaderc_compile_options_set_warnings_as_errors(
@@ -652,8 +673,11 @@ shaderc_compilation_result_t shaderc_assemble_into_spv(
652673
std::string errors;
653674
const auto target_env = additional_options ? additional_options->target_env
654675
: shaderc_target_env_default;
676+
const uint32_t target_env_version =
677+
additional_options ? additional_options->target_env_version : 0;
655678
const bool assembling_succeeded = shaderc_util::SpirvToolsAssemble(
656679
GetCompilerTargetEnv(target_env),
680+
GetCompilerTargetEnvVersion(target_env_version),
657681
{source_assembly, source_assembly + source_assembly_size},
658682
&assembling_output_data, &errors);
659683
result->num_errors = !assembling_succeeded;

‎libshaderc/src/shaderc_cpp_test.cc‎

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,31 @@ TEST_F(CppInterface, CompileAndOptimizeWithLevelSize) {
526526
EXPECT_THAT(disassembly_text, Not(HasSubstr("OpSource")));
527527
}
528528

529+
TEST_F(CppInterface, CompileAndOptimizeForVulkan10Failure) {
530+
options_.SetSourceLanguage(shaderc_source_language_hlsl);
531+
options_.SetTargetEnvironment(shaderc_target_env_vulkan,
532+
shaderc_env_version_vulkan_1_0);
533+
options_.SetOptimizationLevel(shaderc_optimization_level_performance);
534+
535+
EXPECT_THAT(CompilationErrors(kHlslWaveActiveSumeComputeShader,
536+
shaderc_compute_shader, options_),
537+
// TODO(antiagainst): the error message can be improved to be more
538+
// explicit regarding Vulkan 1.1
539+
HasSubstr("compilation succeeded but failed to optimize: "
540+
"Invalid capability operand"));
541+
}
542+
543+
TEST_F(CppInterface, CompileAndOptimizeForVulkan11Success) {
544+
options_.SetSourceLanguage(shaderc_source_language_hlsl);
545+
options_.SetTargetEnvironment(shaderc_target_env_vulkan,
546+
shaderc_env_version_vulkan_1_1);
547+
options_.SetOptimizationLevel(shaderc_optimization_level_performance);
548+
549+
const std::string disassembly_text = AssemblyOutput(
550+
kHlslWaveActiveSumeComputeShader, shaderc_compute_shader, options_);
551+
EXPECT_THAT(disassembly_text, HasSubstr("OpGroupNonUniformIAdd"));
552+
}
553+
529554
TEST_F(CppInterface, FollowingOptLevelOverridesPreviousOne) {
530555
options_.SetOptimizationLevel(shaderc_optimization_level_size);
531556
// Optimization level settings overridden by
@@ -1197,37 +1222,29 @@ std::string ShaderWithTexOffset(int offset) {
11971222
// two particular limits.
11981223
TEST_F(CppInterface, LimitsTexelOffsetDefault) {
11991224
EXPECT_FALSE(CompilationSuccess(ShaderWithTexOffset(-9).c_str(),
1200-
shaderc_glsl_fragment_shader,
1201-
options_));
1225+
shaderc_glsl_fragment_shader, options_));
12021226
EXPECT_TRUE(CompilationSuccess(ShaderWithTexOffset(-8).c_str(),
1203-
shaderc_glsl_fragment_shader,
1204-
options_));
1227+
shaderc_glsl_fragment_shader, options_));
12051228
EXPECT_TRUE(CompilationSuccess(ShaderWithTexOffset(7).c_str(),
1206-
shaderc_glsl_fragment_shader,
1207-
options_));
1229+
shaderc_glsl_fragment_shader, options_));
12081230
EXPECT_FALSE(CompilationSuccess(ShaderWithTexOffset(8).c_str(),
1209-
shaderc_glsl_fragment_shader,
1210-
options_));
1231+
shaderc_glsl_fragment_shader, options_));
12111232
}
12121233

12131234
TEST_F(CppInterface, LimitsTexelOffsetLowerMinimum) {
12141235
options_.SetLimit(shaderc_limit_min_program_texel_offset, -99);
12151236
EXPECT_FALSE(CompilationSuccess(ShaderWithTexOffset(-100).c_str(),
1216-
shaderc_glsl_fragment_shader,
1217-
options_));
1237+
shaderc_glsl_fragment_shader, options_));
12181238
EXPECT_TRUE(CompilationSuccess(ShaderWithTexOffset(-99).c_str(),
1219-
shaderc_glsl_fragment_shader,
1220-
options_));
1239+
shaderc_glsl_fragment_shader, options_));
12211240
}
12221241

12231242
TEST_F(CppInterface, LimitsTexelOffsetHigherMaximum) {
12241243
options_.SetLimit(shaderc_limit_max_program_texel_offset, 10);
12251244
EXPECT_TRUE(CompilationSuccess(ShaderWithTexOffset(10).c_str(),
1226-
shaderc_glsl_fragment_shader,
1227-
options_));
1245+
shaderc_glsl_fragment_shader, options_));
12281246
EXPECT_FALSE(CompilationSuccess(ShaderWithTexOffset(11).c_str(),
1229-
shaderc_glsl_fragment_shader,
1230-
options_));
1247+
shaderc_glsl_fragment_shader, options_));
12311248
}
12321249

12331250
TEST_F(CppInterface, UniformsWithoutBindingsFailCompilation) {

‎libshaderc/src/shaderc_test.cc‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,11 @@ class CompileStringTest : public testing::Test {
262262
OutputType output_type = OutputType::SpirvBinary) {
263263
const Compilation comp(compiler_.get_compiler_handle(), shader, kind,
264264
"shader", "main", options, output_type);
265-
EXPECT_TRUE(CompilationResultIsSuccess(comp.result())) << kind << '\n'
266-
<< shader;
265+
EXPECT_TRUE(CompilationResultIsSuccess(comp.result()))
266+
<< "shader kind: " << kind << "\nerror message: "
267+
<< shaderc_result_get_error_message(comp.result())
268+
<< "\nshader source code: \n"
269+
<< shader;
267270
// Use string(const char* s, size_t n) constructor instead of
268271
// string(const char* s) to make sure the string has complete binary data.
269272
// string(const char* s) assumes a null-terminated C-string, which will cut
@@ -673,6 +676,34 @@ TEST_F(CompileStringWithOptionsTest, CompileAndOptimizeWithLevelSize) {
673676
EXPECT_THAT(disassembly_text, Not(HasSubstr("OpSource")));
674677
}
675678

679+
TEST_F(CompileStringWithOptionsTest, CompileAndOptimizeForVulkan10Failure) {
680+
shaderc_compile_options_set_source_language(options_.get(),
681+
shaderc_source_language_hlsl);
682+
shaderc_compile_options_set_target_env(options_.get(),
683+
shaderc_target_env_vulkan,
684+
shaderc_env_version_vulkan_1_0);
685+
shaderc_compile_options_set_optimization_level(
686+
options_.get(), shaderc_optimization_level_performance);
687+
688+
EXPECT_FALSE(CompilesToValidSpv(compiler_, kHlslWaveActiveSumeComputeShader,
689+
shaderc_compute_shader, options_.get()));
690+
}
691+
692+
TEST_F(CompileStringWithOptionsTest, CompileAndOptimizeForVulkan11Success) {
693+
shaderc_compile_options_set_source_language(options_.get(),
694+
shaderc_source_language_hlsl);
695+
shaderc_compile_options_set_target_env(options_.get(),
696+
shaderc_target_env_vulkan,
697+
shaderc_env_version_vulkan_1_1);
698+
shaderc_compile_options_set_optimization_level(
699+
options_.get(), shaderc_optimization_level_performance);
700+
701+
const std::string disassembly_text = CompilationOutput(
702+
kHlslWaveActiveSumeComputeShader, shaderc_compute_shader, options_.get(),
703+
OutputType::SpirvAssemblyText);
704+
EXPECT_THAT(disassembly_text, HasSubstr("OpGroupNonUniformIAdd"));
705+
}
706+
676707
TEST_F(CompileStringWithOptionsTest, FollowingOptLevelOverridesPreviousOne) {
677708
shaderc_compile_options_set_optimization_level(
678709
options_.get(), shaderc_optimization_level_size);

‎libshaderc_util/include/libshaderc_util/compiler.h‎

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -106,16 +106,17 @@ class Compiler {
106106

107107
// Target environment.
108108
enum class TargetEnv {
109-
Vulkan, // Default to Vulkan 1.0
110-
OpenGL, // Default to OpenGL 4.5
111-
OpenGLCompat, // Deprecated.
109+
Vulkan, // Default to Vulkan 1.0
110+
OpenGL, // Default to OpenGL 4.5
111+
OpenGLCompat, // Deprecated.
112112
};
113113

114114
// Target environment versions. These numbers match those used by Glslang.
115115
enum class TargetEnvVersion : uint32_t {
116+
Default = 0, // Default for the corresponding target environment
116117
// For Vulkan, use numbering scheme from vulkan.h
117-
Vulkan_1_0 = ((1 << 22)), // Default to Vulkan 1.0
118-
Vulkan_1_1 = ((1 << 22) | (1 << 12)), // Default to Vulkan 1.0
118+
Vulkan_1_0 = ((1 << 22)), // Vulkan 1.0
119+
Vulkan_1_1 = ((1 << 22) | (1 << 12)), // Vulkan 1.1
119120
// For OpenGL, use the numbering from #version in shaders.
120121
OpenGL_4_5 = 450,
121122
};
@@ -133,9 +134,10 @@ class Compiler {
133134
Performance, // Optimization towards better performance.
134135
};
135136

136-
// Resource limits. These map to the "max*" fields in glslang::TBuiltInResource.
137+
// Resource limits. These map to the "max*" fields in
138+
// glslang::TBuiltInResource.
137139
enum class Limit {
138-
#define RESOURCE(NAME,FIELD,CNAME) NAME,
140+
#define RESOURCE(NAME, FIELD, CNAME) NAME,
139141
#include "resources.inc"
140142
#undef RESOURCE
141143
};
@@ -185,20 +187,24 @@ class Compiler {
185187

186188
// Returns a std::array of all the Stage values.
187189
const std::array<Stage, kNumStages>& stages() const {
188-
static std::array<Stage, kNumStages> values{
189-
{Stage::Vertex, Stage::TessEval, Stage::TessControl, Stage::Geometry,
190-
Stage::Fragment, Stage::Compute,
190+
static std::array<Stage, kNumStages> values{{
191+
Stage::Vertex,
192+
Stage::TessEval,
193+
Stage::TessControl,
194+
Stage::Geometry,
195+
Stage::Fragment,
196+
Stage::Compute,
191197
#ifdef NV_EXTENSIONS
192-
Stage::RayGenNV,
193-
Stage::IntersectNV,
194-
Stage::AnyHitNV,
195-
Stage::ClosestHitNV,
196-
Stage::MissNV,
197-
Stage::CallableNV,
198-
Stage::TaskNV,
199-
Stage::MeshNV,
198+
Stage::RayGenNV,
199+
Stage::IntersectNV,
200+
Stage::AnyHitNV,
201+
Stage::ClosestHitNV,
202+
Stage::MissNV,
203+
Stage::CallableNV,
204+
Stage::TaskNV,
205+
Stage::MeshNV,
200206
#endif
201-
}};
207+
}};
202208
return values;
203209
}
204210

@@ -215,7 +221,7 @@ class Compiler {
215221
generate_debug_info_(false),
216222
enabled_opt_passes_(),
217223
target_env_(TargetEnv::Vulkan),
218-
target_env_version_(0), // Resolve default later.
224+
target_env_version_(TargetEnvVersion::Default),
219225
source_language_(SourceLanguage::GLSL),
220226
limits_(kDefaultTBuiltInResource),
221227
auto_bind_uniforms_(false),
@@ -255,10 +261,11 @@ class Compiler {
255261
const char* definition, size_t definition_length);
256262

257263
// Sets the target environment, including version. The version value should
258-
// be 0 or one of the values from TargetEnvVersion. The 0 version value maps
264+
// be 0 or one of the values from TargetEnvVersion. The default value maps
259265
// to Vulkan 1.0 if the target environment is Vulkan, and it maps to OpenGL
260266
// 4.5 if the target environment is OpenGL.
261-
void SetTargetEnv(TargetEnv env, uint32_t version = 0);
267+
void SetTargetEnv(TargetEnv env,
268+
TargetEnvVersion version = TargetEnvVersion::Default);
262269

263270
// Sets the souce language.
264271
void SetSourceLanguage(SourceLanguage lang);
@@ -476,7 +483,7 @@ class Compiler {
476483
// particular to each target environment. If this is 0, then use a default
477484
// for that particular target environment. See libshaders/shaderc/shaderc.h
478485
// for those defaults.
479-
uint32_t target_env_version_;
486+
TargetEnvVersion target_env_version_;
480487

481488
// The source language. Defaults to GLSL.
482489
SourceLanguage source_language_;

‎libshaderc_util/include/libshaderc_util/spirv_tools_wrapper.h‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,16 @@ namespace shaderc_util {
2727
// Assembles the given assembly. On success, returns true, writes the assembled
2828
// binary to *binary, and clears *errors. Otherwise, writes the error message
2929
// into *errors.
30-
bool SpirvToolsAssemble(Compiler::TargetEnv env, const string_piece assembly,
31-
spv_binary* binary, std::string* errors);
30+
bool SpirvToolsAssemble(Compiler::TargetEnv env,
31+
Compiler::TargetEnvVersion version,
32+
const string_piece assembly, spv_binary* binary,
33+
std::string* errors);
3234

3335
// Disassembles the given binary. Returns true and writes the disassembled text
3436
// to *text_or_error if successful. Otherwise, writes the error message to
3537
// *text_or_error.
3638
bool SpirvToolsDisassemble(Compiler::TargetEnv env,
39+
Compiler::TargetEnvVersion version,
3740
const std::vector<uint32_t>& binary,
3841
std::string* text_or_error);
3942

@@ -55,6 +58,7 @@ enum class PassId {
5558
// optimized binary back to *binary if successful. Otherwise, writes errors to
5659
// *errors and the content of binary may be in an invalid state.
5760
bool SpirvToolsOptimize(Compiler::TargetEnv env,
61+
Compiler::TargetEnvVersion version,
5862
const std::vector<PassId>& enabled_passes,
5963
std::vector<uint32_t>* binary, std::string* errors);
6064

0 commit comments

Comments
 (0)