AuroraMiddleware/SPIRV-Cross
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix edge case where opaque types can be declared on stack.

Browse Source 
In the bizarre case where the ID of a loaded opaque type aliased with a
literal which was used as part of another texturing instruction, we
could end up with a case where domination analysis assumed the loaded
opaque type needed to be moved to a different scope.

Fix the issue by never doing dominance analysis for opaque temporaries,
and be more robust when analyzing texturing instructions.

Also make sure reflection output is deterministic.
This patch slightly alterered output for some unknown reason, but it came from an
unordered_map, so it's fine.
This commit is contained in:
Hans-Kristian Arntzen 2019-02-19 17:00:49 +01:00
parent ced1637987
commit d2cc43e667
8 changed files with 202 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
reference/shaders-no-opt/asm/frag/opaque-id-literal-alias.preserve.asm.frag Normal file
View File

@ -0,0 +1,20 @@
#version 450
layout(binding = 0) uniform sampler2DMS uSampled;
layout(location = 0) out vec4 FragColor;
layout(location = 0) in vec2 vUV;
void main()
{
FragColor = vec4(0.0);
if (gl_FragCoord.x < 10.0)
{
FragColor += texelFetch(uSampled, ivec2(vUV), 0);
}
else
{
FragColor += texelFetch(uSampled, ivec2(vUV), 1);
}
}

12
reference/shaders-reflection/asm/aliased-entry-point-names.asm.multi.json
View File

@ -1,19 +1,19 @@
{
"entryPoints" : [
{
"name" : "main",
"mode" : "vert"
},
{
"name" : "main2",
"name" : "maim",
"mode" : "vert"
},
{
"name" : "main",
"mode" : "vert"
},
{
"name" : "maim",
"mode" : "frag"
},
{
"name" : "main2",
"name" : "main",
"mode" : "frag"
}
],

78
shaders-no-opt/asm/frag/opaque-id-literal-alias.preserve.asm.frag Normal file
View File

@ -0,0 +1,78 @@
; SPIR-V
; Version: 1.0
; Generator: Khronos Glslang Reference Front End; 7
; Bound: 50
; Schema: 0
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main" %FragColor %gl_FragCoord %vUV
OpExecutionMode %main OriginUpperLeft
OpSource GLSL 450
OpName %main "main"
OpName %FragColor "FragColor"
OpName %gl_FragCoord "gl_FragCoord"
OpName %uSampled "uSampled"
OpName %vUV "vUV"
OpDecorate %FragColor Location 0
OpDecorate %gl_FragCoord BuiltIn FragCoord
OpDecorate %uSampled DescriptorSet 0
OpDecorate %uSampled Binding 0
OpDecorate %vUV Location 0
%void = OpTypeVoid
%3 = OpTypeFunction %void
%float = OpTypeFloat 32
%v4float = OpTypeVector %float 4
%_ptr_Output_v4float = OpTypePointer Output %v4float
%FragColor = OpVariable %_ptr_Output_v4float Output
%float_0 = OpConstant %float 0
%11 = OpConstantComposite %v4float %float_0 %float_0 %float_0 %float_0
%_ptr_Input_v4float = OpTypePointer Input %v4float
%gl_FragCoord = OpVariable %_ptr_Input_v4float Input
%uint = OpTypeInt 32 0
%uint_0 = OpConstant %uint 0
%_ptr_Input_float = OpTypePointer Input %float
%float_10 = OpConstant %float 10
%bool = OpTypeBool
%24 = OpTypeImage %float 2D 0 0 1 1 Unknown
%25 = OpTypeSampledImage %24
%_ptr_UniformConstant_25 = OpTypePointer UniformConstant %25
%uSampled = OpVariable %_ptr_UniformConstant_25 UniformConstant
%v2float = OpTypeVector %float 2
%_ptr_Input_v2float = OpTypePointer Input %v2float
%vUV = OpVariable %_ptr_Input_v2float Input
%int = OpTypeInt 32 1
%v2int = OpTypeVector %int 2
%int_0 = OpConstant %int 0
%int_1 = OpConstant %int 1
%main = OpFunction %void None %3
%5 = OpLabel
OpStore %FragColor %11
%17 = OpAccessChain %_ptr_Input_float %gl_FragCoord %uint_0
%18 = OpLoad %float %17
%21 = OpFOrdLessThan %bool %18 %float_10
OpSelectionMerge %23 None
OpBranchConditional %21 %22 %41
%22 = OpLabel
%28 = OpLoad %25 %uSampled
%32 = OpLoad %v2float %vUV
%35 = OpConvertFToS %v2int %32
%64 = OpImage %24 %28
%38 = OpImageFetch %v4float %64 %35 Sample %int_0
%39 = OpLoad %v4float %FragColor
%40 = OpFAdd %v4float %39 %38
OpStore %FragColor %40
OpBranch %23
%41 = OpLabel
%42 = OpLoad %25 %uSampled
%43 = OpLoad %v2float %vUV
%44 = OpConvertFToS %v2int %43
%46 = OpImage %24 %42
%47 = OpImageFetch %v4float %46 %44 Sample %int_1
%48 = OpLoad %v4float %FragColor
%49 = OpFAdd %v4float %48 %47
OpStore %FragColor %49
OpBranch %23
%23 = OpLabel
OpReturn
OpFunctionEnd

4
shaders-reflection/asm/aliased-entry-point-names.asm.multi
View File

@ -7,9 +7,9 @@
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Vertex %main "main" %_
OpEntryPoint Vertex %main2 "main2" %_
OpEntryPoint Vertex %main2 "maim" %_
OpEntryPoint Fragment %main3 "main" %FragColor
OpEntryPoint Fragment %main4 "main2" %FragColor
OpEntryPoint Fragment %main4 "maim" %FragColor
OpSource GLSL 450
OpMemberDecorate %gl_PerVertex 0 BuiltIn Position
OpMemberDecorate %gl_PerVertex 1 BuiltIn PointSize

64
spirv_cross.cpp
View File

@ -3053,7 +3053,7 @@ bool Compiler::AnalyzeVariableScopeAccessHandler::handle(spv::Op op, const uint3
}
case OpArrayLength:
// Uses literals, but cannot be a phi variable, so ignore.
// Uses literals, but cannot be a phi variable or temporary, so ignore.
break;
// Atomics shouldn't be able to access function-local variables.
@ -3072,6 +3072,55 @@ bool Compiler::AnalyzeVariableScopeAccessHandler::handle(spv::Op op, const uint3
notify_variable_access(args[i], current_block->self);
break;
case OpImageWrite:
for (uint32_t i = 0; i < length; i++)
{
// Argument 3 is a literal.
if (i != 3)
notify_variable_access(args[i], current_block->self);
}
break;
case OpImageSampleImplicitLod:
case OpImageSampleExplicitLod:
case OpImageSparseSampleImplicitLod:
case OpImageSparseSampleExplicitLod:
case OpImageSampleProjImplicitLod:
case OpImageSampleProjExplicitLod:
case OpImageSparseSampleProjImplicitLod:
case OpImageSparseSampleProjExplicitLod:
case OpImageFetch:
case OpImageSparseFetch:
case OpImageRead:
case OpImageSparseRead:
for (uint32_t i = 1; i < length; i++)
{
// Argument 4 is a literal.
if (i != 4)
notify_variable_access(args[i], current_block->self);
}
break;
case OpImageSampleDrefImplicitLod:
case OpImageSampleDrefExplicitLod:
case OpImageSparseSampleDrefImplicitLod:
case OpImageSparseSampleDrefExplicitLod:
case OpImageSampleProjDrefImplicitLod:
case OpImageSampleProjDrefExplicitLod:
case OpImageSparseSampleProjDrefImplicitLod:
case OpImageSparseSampleProjDrefExplicitLod:
case OpImageGather:
case OpImageSparseGather:
case OpImageDrefGather:
case OpImageSparseDrefGather:
for (uint32_t i = 1; i < length; i++)
{
// Argument 5 is a literal.
if (i != 5)
notify_variable_access(args[i], current_block->self);
}
break;
default:
{
// Rather dirty way of figuring out where Phi variables are used.
@ -3310,6 +3359,11 @@ void Compiler::analyze_variable_scope(SPIRFunction &entry, AnalyzeVariableScopeA
continue;
}
// There is no point in doing domination analysis for opaque types.
auto &type = get<SPIRType>(itr->second);
if (type_is_opaque_value(type))
continue;
DominatorBuilder builder(cfg);
bool force_temporary = false;
@ -3996,7 +4050,7 @@ bool Compiler::instruction_to_result_type(uint32_t &result_type, uint32_t &resul
return false;
default:
if (length > 1)
if (length > 1 && maybe_get<SPIRType>(args[0]) != nullptr)
{
result_type = args[0];
result_id = args[1];
@ -4065,3 +4119,9 @@ bool Compiler::image_is_comparison(const spirv_cross::SPIRType &type, uint32_t i
{
return type.image.depth || (comparison_ids.count(id) != 0);
}
bool Compiler::type_is_opaque_value(const spirv_cross::SPIRType &type) const
{
return !type.pointer && (type.basetype == SPIRType::SampledImage || type.basetype == SPIRType::Image ||
type.basetype == SPIRType::Sampler);
}

5
spirv_cross.hpp
View File

@ -565,7 +565,9 @@ protected:
template <typename T>
T *maybe_get(uint32_t id)
{
if (ir.ids[id].get_type() == static_cast<Types>(T::type))
if (id >= ir.ids.size())
return nullptr;
else if (ir.ids[id].get_type() == static_cast<Types>(T::type))
return &get<T>(id);
else
return nullptr;
@ -982,6 +984,7 @@ private:
void fixup_type_alias();
bool type_is_block_like(const SPIRType &type) const;
bool type_is_opaque_value(const SPIRType &type) const;
};
} // namespace spirv_cross

10
spirv_reflect.cpp
View File

@ -391,6 +391,16 @@ void CompilerReflection::emit_entry_points()
auto entries = get_entry_points_and_stages();
if (!entries.empty())
{
// Needed to make output deterministic.
sort(begin(entries), end(entries), [](const EntryPoint &a, const EntryPoint &b) -> bool {
if (a.execution_model < b.execution_model)
return true;
else if (a.execution_model > b.execution_model)
return false;
else
return a.name < b.name;
});
json_stream->emit_json_key_array("entryPoints");
for (auto &e : entries)
{

24
test_shaders.py
View File

@ -135,8 +135,12 @@ def cross_compile_msl(shader, spirv, opt):
spirv_path = create_temporary()
msl_path = create_temporary(os.path.basename(shader))
spirv_cmd = ['spirv-as', '-o', spirv_path, shader]
if '.preserve.' in shader:
spirv_cmd.append('--preserve-numeric-ids')
if spirv:
subprocess.check_call(['spirv-as', '-o', spirv_path, shader])
subprocess.check_call(spirv_cmd)
else:
subprocess.check_call(['glslangValidator', '--target-env', 'vulkan1.1', '-V', '-o', spirv_path, shader])
@ -231,8 +235,12 @@ def cross_compile_hlsl(shader, spirv, opt, force_no_external_validation):
spirv_path = create_temporary()
hlsl_path = create_temporary(os.path.basename(shader))
spirv_cmd = ['spirv-as', '-o', spirv_path, shader]
if '.preserve.' in shader:
spirv_cmd.append('--preserve-numeric-ids')
if spirv:
subprocess.check_call(['spirv-as', '-o', spirv_path, shader])
subprocess.check_call(spirv_cmd)
else:
subprocess.check_call(['glslangValidator', '--target-env', 'vulkan1.1', '-V', '-o', spirv_path, shader])
@ -255,8 +263,12 @@ def cross_compile_reflect(shader, spirv, opt):
spirv_path = create_temporary()
reflect_path = create_temporary(os.path.basename(shader))
spirv_cmd = ['spirv-as', '-o', spirv_path, shader]
if '.preserve.' in shader:
spirv_cmd.append('--preserve-numeric-ids')
if spirv:
subprocess.check_call(['spirv-as', '-o', spirv_path, shader])
subprocess.check_call(spirv_cmd)
else:
subprocess.check_call(['glslangValidator', '--target-env', 'vulkan1.1', '-V', '-o', spirv_path, shader])
@ -282,8 +294,12 @@ def cross_compile(shader, vulkan, spirv, invalid_spirv, eliminate, is_legacy, fl
if vulkan or spirv:
vulkan_glsl_path = create_temporary('vk' + os.path.basename(shader))
spirv_cmd = ['spirv-as', '-o', spirv_path, shader]
if '.preserve.' in shader:
spirv_cmd.append('--preserve-numeric-ids')
if spirv:
subprocess.check_call(['spirv-as', '-o', spirv_path, shader])
subprocess.check_call(spirv_cmd)
else:
subprocess.check_call(['glslangValidator', '--target-env', 'vulkan1.1', '-V', '-o', spirv_path, shader])