From 28c9be8a2376d3df2db33e55bd1399df59de4c52 Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Tue, 17 Apr 2018 13:56:10 +0200 Subject: [PATCH 1/3] Unsigned integers are disallowed on legacy targets. There is no sensible way to map this that would work in all scenarios. --- spirv_glsl.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp index d63b7f0..6b69656 100644 --- a/spirv_glsl.cpp +++ b/spirv_glsl.cpp @@ -322,6 +322,9 @@ void CompilerGLSL::find_static_extensions() if (type.basetype == SPIRType::Half) require_extension_internal("GL_AMD_gpu_shader_half_float"); + + if (type.basetype == SPIRType::UInt && is_legacy()) + SPIRV_CROSS_THROW("Unsigned integers are not supported on legacy targets."); } } From e930f79e2e23a29686fa40469503a908306c2bdd Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Tue, 17 Apr 2018 14:56:49 +0200 Subject: [PATCH 2/3] Be a bit smarter about uint on legacy targets. Allow constants (array sizes for example), but using unsigned opcodes, and unsigned-specific opcodes is a problem. --- spirv_glsl.cpp | 73 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 66 insertions(+), 7 deletions(-) diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp index 6b69656..626f5dc 100644 --- a/spirv_glsl.cpp +++ b/spirv_glsl.cpp @@ -26,6 +26,45 @@ using namespace spv; using namespace spirv_cross; using namespace std; +static bool is_unsigned_opcode(Op op) +{ + // Don't have to be exhaustive, only relevant for legacy target checking ... + switch (op) + { + case OpShiftRightLogical: + case OpUGreaterThan: + case OpUGreaterThanEqual: + case OpULessThan: + case OpULessThanEqual: + case OpUConvert: + case OpUDiv: + case OpUMod: + case OpUMulExtended: + case OpConvertUToF: + case OpConvertFToU: + return true; + + default: + return false; + } +} + +static bool is_unsigned_glsl_opcode(GLSLstd450 op) +{ + // Don't have to be exhaustive, only relevant for legacy target checking ... + switch (op) + { + case GLSLstd450UClamp: + case GLSLstd450UMin: + case GLSLstd450UMax: + case GLSLstd450FindUMsb: + return true; + + default: + return false; + } +} + static bool packing_is_vec4_padded(BufferPackingStandard packing) { switch (packing) @@ -322,9 +361,6 @@ void CompilerGLSL::find_static_extensions() if (type.basetype == SPIRType::Half) require_extension_internal("GL_AMD_gpu_shader_half_float"); - - if (type.basetype == SPIRType::UInt && is_legacy()) - SPIRV_CROSS_THROW("Unsigned integers are not supported on legacy targets."); } } @@ -2441,6 +2477,9 @@ string CompilerGLSL::constant_op_expression(const SPIRConstantOp &cop) bool unary = false; string op; + if (is_legacy() && is_unsigned_opcode(cop.opcode)) + SPIRV_CROSS_THROW("Unsigned integers are not supported on legacy targets."); + // TODO: Find a clean way to reuse emit_instruction. switch (cop.opcode) { @@ -3012,7 +3051,14 @@ string CompilerGLSL::constant_expression_vector(const SPIRConstant &c, uint32_t if (splat) { res += convert_to_string(c.scalar(vector, 0)); - if (backend.uint32_t_literal_suffix) + if (is_legacy()) + { + // Fake unsigned constant literals with signed ones if possible. + // Things like array sizes, etc, tend to be unsigned even though they could just as easily be signed. + if (c.scalar_i32(vector, 0) < 0) + SPIRV_CROSS_THROW("Tried to convert uint literal into int, but this made the literal negative."); + } + else if (backend.uint32_t_literal_suffix) res += "u"; } else @@ -3024,7 +3070,14 @@ string CompilerGLSL::constant_expression_vector(const SPIRConstant &c, uint32_t else { res += convert_to_string(c.scalar(vector, i)); - if (backend.uint32_t_literal_suffix) + if (is_legacy()) + { + // Fake unsigned constant literals with signed ones if possible. + // Things like array sizes, etc, tend to be unsigned even though they could just as easily be signed. + if (c.scalar_i32(vector, i) < 0) + SPIRV_CROSS_THROW("Tried to convert uint literal into int, but this made the literal negative."); + } + else if (backend.uint32_t_literal_suffix) res += "u"; } @@ -4043,7 +4096,10 @@ string CompilerGLSL::to_function_args(uint32_t img, const SPIRType &imgtype, boo void CompilerGLSL::emit_glsl_op(uint32_t result_type, uint32_t id, uint32_t eop, const uint32_t *args, uint32_t) { - GLSLstd450 op = static_cast(eop); + auto op = static_cast(eop); + + if (is_legacy() && is_unsigned_glsl_opcode(op)) + SPIRV_CROSS_THROW("Unsigned integers are not supported on legacy GLSL targets."); switch (op) { @@ -4108,8 +4164,8 @@ void CompilerGLSL::emit_glsl_op(uint32_t result_type, uint32_t id, uint32_t eop, } // Minmax - case GLSLstd450FMin: case GLSLstd450UMin: + case GLSLstd450FMin: case GLSLstd450SMin: emit_binary_func_op(result_type, id, args[0], args[1], "min"); break; @@ -8317,6 +8373,9 @@ string CompilerGLSL::type_to_glsl(const SPIRType &type, uint32_t id) break; } + if (type.basetype == SPIRType::UInt && is_legacy()) + SPIRV_CROSS_THROW("Unsigned integers are not supported on legacy targets."); + if (type.vecsize == 1 && type.columns == 1) // Scalar builtin { switch (type.basetype) From b9cd3dcd7fdd303fa1c58039f9fc9d98f2059f39 Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Tue, 17 Apr 2018 15:01:31 +0200 Subject: [PATCH 3/3] Run format_all.sh. --- spirv_glsl.cpp | 12 +++++++----- spirv_hlsl.cpp | 8 ++++++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp index 626f5dc..805c88f 100644 --- a/spirv_glsl.cpp +++ b/spirv_glsl.cpp @@ -3075,7 +3075,8 @@ string CompilerGLSL::constant_expression_vector(const SPIRConstant &c, uint32_t // Fake unsigned constant literals with signed ones if possible. // Things like array sizes, etc, tend to be unsigned even though they could just as easily be signed. if (c.scalar_i32(vector, i) < 0) - SPIRV_CROSS_THROW("Tried to convert uint literal into int, but this made the literal negative."); + SPIRV_CROSS_THROW( + "Tried to convert uint literal into int, but this made the literal negative."); } else if (backend.uint32_t_literal_suffix) res += "u"; @@ -4585,8 +4586,7 @@ void CompilerGLSL::emit_subgroup_op(const Instruction &i) { require_extension_internal("GL_KHR_shader_subgroup_clustered"); } - else if (operation == GroupOperationExclusiveScan || - operation == GroupOperationInclusiveScan || + else if (operation == GroupOperationExclusiveScan || operation == GroupOperationInclusiveScan || operation == GroupOperationReduce) { require_extension_internal("GL_KHR_shader_subgroup_arithmetic"); @@ -4688,6 +4688,7 @@ void CompilerGLSL::emit_subgroup_op(const Instruction &i) emit_unary_func_op(result_type, id, ops[3], "subgroupAllEqual"); break; + // clang-format off #define GROUP_OP(op, glsl_op) \ case OpGroupNonUniform##op: \ { \ @@ -4718,6 +4719,7 @@ case OpGroupNonUniform##op: \ GROUP_OP(BitwiseOr, Or) GROUP_OP(BitwiseXor, Xor) #undef GROUP_OP + // clang-format on case OpGroupNonUniformQuadSwap: { @@ -7573,8 +7575,8 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction) } else if (memory == ScopeSubgroup) { - const uint32_t all_barriers = MemorySemanticsWorkgroupMemoryMask | MemorySemanticsUniformMemoryMask | - MemorySemanticsImageMemoryMask; + const uint32_t all_barriers = + MemorySemanticsWorkgroupMemoryMask | MemorySemanticsUniformMemoryMask | MemorySemanticsImageMemoryMask; if (semantics & (MemorySemanticsCrossWorkgroupMemoryMask | MemorySemanticsSubgroupMemoryMask)) { diff --git a/spirv_hlsl.cpp b/spirv_hlsl.cpp index 1704dae..f10686e 100644 --- a/spirv_hlsl.cpp +++ b/spirv_hlsl.cpp @@ -3696,8 +3696,10 @@ void CompilerHLSL::emit_subgroup_op(const Instruction &i) if (operation == GroupOperationReduce) { bool forward = should_forward(ops[4]); - auto left = join("countbits(", to_enclosed_expression(ops[4]), ".x) + countbits(", to_enclosed_expression(ops[4]), ".y)"); - auto right = join("countbits(", to_enclosed_expression(ops[4]), ".z) + countbits(", to_enclosed_expression(ops[4]), ".w)"); + auto left = join("countbits(", to_enclosed_expression(ops[4]), ".x) + countbits(", + to_enclosed_expression(ops[4]), ".y)"); + auto right = join("countbits(", to_enclosed_expression(ops[4]), ".z) + countbits(", + to_enclosed_expression(ops[4]), ".w)"); emit_op(result_type, id, join(left, " + ", right), forward); inherit_expression_dependencies(id, ops[4]); } @@ -3735,6 +3737,7 @@ void CompilerHLSL::emit_subgroup_op(const Instruction &i) break; } + // clang-format off #define GROUP_OP(op, hlsl_op, supports_scan) \ case OpGroupNonUniform##op: \ { \ @@ -3769,6 +3772,7 @@ case OpGroupNonUniform##op: \ GROUP_OP(BitwiseOr, BitOr, false) GROUP_OP(BitwiseXor, BitXor, false) #undef GROUP_OP + // clang-format on case OpGroupNonUniformQuadSwap: {