KhronosGroup / SPIRV-Cross

SPIRV-Cross is a practical tool and library for performing reflection on SPIR-V and disassembling SPIR-V back to high level languages.
Apache License 2.0
2.03k stars 559 forks source link

spirv-cross bad code and abort error #2285

Closed mihaly-sisak closed 6 months ago

mihaly-sisak commented 7 months ago

I try to ship optimized OpenGL code in my game EvoLife. Without this steps the shader compilation takes longer on first launch. My optimizing pipeline:

glslang --quiet -G -S <stage> -o <filename>.spv <filename>.glsl
spirv-opt -Oconfig=glsl_opt_config.cfg -o <filename>_opt.spv <filename>.spv
spirv-cross --version 430 --no-es --output <filename>_opt.<stage>.glsl <filename>_opt.spv

My problem is that I can not run spirv-opt with -O, I have to use -Oconfig with the file attached. My starting point was the -O list and I bisected which options cause issues. glsl_opt_config.txt Line 3 --merge-return: causes glsl shaders to behave differently than unoptimized versions Line 4 --inline-entry-points-exhaustive: can not run without merge-return Line 40 and 43 --merge-blocks: causes spirv-cross to abort on some shaders if line 3 --merge-return and line 4 --inline-entry-points-exhaustive are enabled, here assert(loop_header_block.merge == SPIRBlock::MergeLoop);

Enabling line 3 merge-return adds an uninitialized bool variable that is used as the condition for breaking out of the do {} while(false) loop that is replacing the body of the function. Please let me know how can I help to resolve this problem. In the meantime maybe others will find this and can use the same workaround.

HansKristian-Work commented 6 months ago

Please attach a reproducing SPIR-V file. It is impossible for me to understand what the problem is just from this description. Are you sure the SPIR-V files pass validation in the first place?

mihaly-sisak commented 6 months ago

The SPIR-V file passes the spirv-val --target-env opengl4.3 test. This spirv snippet:

%reserve_joint_to_circle_u1_ = OpFunction %uint None %13
        %cid = OpFunctionParameter %_ptr_Function_uint
         %91 = OpLabel
               OpSelectionMerge %3281 None
               OpSwitch %uint_0 %3284
       %3284 = OpLabel
               OpBranch %729
        %729 = OpLabel
       %3298 = OpPhi %uint %uint_0 %3284 %3306 %732
       %3297 = OpPhi %uint %uint_0 %3284 %774 %732
               OpLoopMerge %731 %732 None
               OpBranch %733
        %733 = OpLabel
        %735 = OpLoad %uint %max_dp_per_c
        %737 = OpAccessChain %_ptr_Uniform_uint %si %int_32
        %738 = OpLoad %uint %737
        %739 = OpIAdd %uint %735 %738
        %740 = OpULessThan %bool %3297 %739
               OpBranchConditional %740 %730 %731
        %730 = OpLabel
        %743 = OpShiftLeftLogical %uint %uint_1 %3297
        %746 = OpBitwiseAnd %uint %3298 %743
        %747 = OpINotEqual %bool %746 %uint_0
               OpSelectionMerge %749 None
               OpBranchConditional %747 %748 %749
        %748 = OpLabel
               OpBranch %732
        %749 = OpLabel
        %751 = OpLoad %uint %grid_size
        %752 = OpIAdd %uint %uint_1024 %751
        %753 = OpLoad %uint %circle_size
        %754 = OpIMul %uint %753 %uint_5
        %755 = OpIAdd %uint %752 %754
        %756 = OpLoad %uint %cid
        %757 = OpIAdd %uint %755 %756
        %758 = OpAccessChain %_ptr_Uniform_int %si %int_31
        %759 = OpLoad %int %758
        %760 = OpBitcast %uint %759
        %761 = OpIAdd %uint %757 %760
        %762 = OpAccessChain %_ptr_Uniform_uint %su %int_0 %761
        %764 = OpAtomicOr %uint %762 %uint_1 %uint_0 %743
        %767 = OpBitwiseAnd %uint %764 %743
        %768 = OpIEqual %bool %767 %uint_0
               OpSelectionMerge %770 None
               OpBranchConditional %768 %769 %770
        %769 = OpLabel
               OpBranch %731
        %770 = OpLabel
               OpBranch %732
        %732 = OpLabel
       %3306 = OpPhi %uint %3298 %748 %764 %770
        %774 = OpIAdd %uint %3297 %int_1
               OpBranch %729
        %731 = OpLabel
       %3302 = OpPhi %uint %3304 %733 %3297 %769
       %3299 = OpPhi %bool %false %733 %true %769
               OpSelectionMerge %3286 None
               OpBranchConditional %3299 %3281 %3286
       %3286 = OpLabel
               OpBranch %3281
       %3281 = OpLabel
       %3301 = OpPhi %uint %3302 %731 %uint_4294967295 %3286
               OpReturnValue %3301
               OpFunctionEnd

Becomes this OpenGL 4.3 code:

uint reserve_joint_to_circle(uint cid)
{
    uint _3301;
    do
    {
        bool _3299;
        uint _3302;
        uint _3306;
        for (uint _3297 = 0u, _3298 = 0u; _3297 < (max_dp_per_c + si.s_uzero); _3298 = _3306, _3297++)
        {
            uint _743 = 1u << _3297;
            if ((_3298 & _743) != 0u)
            {
                _3306 = _3298;
                continue;
            }
            uint _764 = atomicOr(su.d[(((1024u + grid_size) + (circle_size * 5u)) + cid) + uint(si.s_izero)], _743);
            if ((_764 & _743) == 0u)
            {
                _3302 = _3297;
                _3299 = true;
                break;
            }
            _3306 = _764;
        }
        if (_3299)
        {
            _3301 = _3302;
            break;
        }
        _3301 = 4294967295u;
        break;
    } while(false);
    return _3301;
}

Sadly bool _3299 is uninitialized.

mihaly-sisak commented 6 months ago

Thank you so much! 🙏