search

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.02k stars 557 forks source link

A couple of tests failed to pass with one of the recent glslang commits #2366

Closed wooyoungqcom closed 2 weeks ago

wooyoungqcom commented 3 weeks ago

Hi,

I am preparing a patch to add support for GL_QCOM_image_processing2.

Most of the tests including the new tests we are trying to add have passed with

GLSLANG_REV=10ee92feb00712f444783032f5c9ee798887670d SPIRV_TOOLS_REV=9bd44d028e8ca257dde9c24d5f67adabee3275ab SPIRV_HEADERS_REV=69597bee034c4bd878490c7b3e1a1e1f6d6b8ecc

and with some reference output fixes.

However, the following two tests

shaders/asm/frag/locations-components.asm.frag shaders/vulkan/vert/small-storage.vk.vert

failed to pass with glslang commit "44fcbccd06b6e3596925d787774afe2a28d3fe16" which is included with "10ee92feb00712f444783032f5c9ee798887670d"

For shaders/asm/frag/locations-components.asm.frag, changing location from 2 to 3 for %22,

           OpDecorate %22 Location 2
           OpDecorate %22 Flat
           OpDecorate %28 Location 2
           OpDecorate %28 Component 1
           OpDecorate %28 Flat
           OpDecorate %33 Location 2
           OpDecorate %33 Component 2
           OpDecorate %33 Flat

==>

           OpDecorate %22 Location 3
           OpDecorate %22 Flat
           OpDecorate %28 Location 2
           OpDecorate %28 Component 1
           OpDecorate %28 Flat
           OpDecorate %33 Location 2
           OpDecorate %33 Component 2
           OpDecorate %33 Flat

made the location alias issue go away, but spirv-cross did not handle the following correctly,

     %v1 = OpVariable %_ptr_Private_v4float Private

and glslangValidator complains about

ERROR: 0:12: 'v1' : undeclared identifier ERROR: 0:12: '' : compilation terminated ERROR: 2 compilation errors. No code generated.

For shaders/vulkan/vert/small-storage.vk.vert, is it OK to change int16_t to uint16_t for "foo"

layout(location = 0, component = 0) in int16_t foo; layout(location = 0, component = 1) in uint16_t bar;

==>

layout(location = 0, component = 0) in uint16_t foo; layout(location = 0, component = 1) in uint16_t bar;

wooyoungqcom commented 3 weeks ago

I forgot to mention another case. shaders-msl-no-opt/asm/tesc/copy-memory-control-point.asm.tesc (and shaders-no-opt/asm/tesc/copy-memory-control-point.asm.tesc) has the same OpDecorate repeated a few times

            OpDecorate %16 Binding 0
            OpDecorate %30 BuiltIn TessLevelOuter
        |   OpDecorate %30 Patch                              
        |   OpDecorate %30 Patch                              
        |   OpDecorate %30 Patch                              
        |   OpDecorate %30 Patch                              
            OpDecorate %80 BuiltIn TessLevelInner
        |   OpDecorate %80 Patch                              
        |   OpDecorate %80 Patch                              
            OpDecorate %101 Location 0
            OpDecorate %103 Location 1
            OpDecorate %108 Location 2

Is it OK to remove the duplicates ? I think glslang commit "1e4f53ab2de355296de690583bd26818264b25ff" lets glslangValidator complain about them

HansKristian-Work commented 3 weeks ago

The commits you provided don't compile:

/home/maister/git/SPIRV-Cross/external/spirv-tools/source/val/validate_atomics.cpp:151:49: error: ‘AtomicFloat16VectorNV’ is not a member of ‘spv::Capability’
  151 |             (!(_.HasCapability(spv::Capability::AtomicFloat16VectorNV) &&
      |                                                 ^~~~~~~~~~~~~~~~~~~~~
/home/maister/git/SPIRV-Cross/external/spirv-tools/source/val/validate_atomics.cpp:165:55: error: ‘AtomicFloat16VectorNV’ is not a member of ‘spv::Capability’
  165 |                      _.HasCapability(spv::Capability::AtomicFloat16VectorNV) &&
      |                                                       ^~~~~~~~~~~~~~~~~~~~~
/home/maister/git/SPIRV-Cross/external/spirv-tools/source/val/validate_atomics.cpp:231:53: error: ‘AtomicFloat16VectorNV’ is not a member of ‘spv::Capability’
  231 |               if (!_.HasCapability(spv::Capability::AtomicFloat16VectorNV))
      |                                                     ^~~~~~~~~~~~~~~~~~~~~
/home/maister/git/SPIRV-Cross/external/spirv-tools/source/val/validate_atomics.cpp:263:53: error: ‘AtomicFloat16VectorNV’ is not a member of ‘spv::Capability’
  263 |               if (!_.HasCapability(spv::Capability::AtomicFloat16VectorNV))
wooyoungqcom commented 3 weeks ago

On my local machine, after clearing external/spirv-tools-build, with build_glslang_spirv_tools.sh , I got the following outputs.

[ 99%] Built target SPIRV-Tools-reduce Scanning dependencies of target spirv-reduce [100%] Building CXX object tools/CMakeFiles/spirv-reduce.dir/util/flags.cpp.o [100%] Building CXX object tools/CMakeFiles/spirv-reduce.dir/reduce/reduce.cpp.o [100%] Building CXX object tools/CMakeFiles/spirv-reduce.dir/util/cli_consumer.cpp.o [100%] Linking CXX executable spirv-reduce [100%] Built target spirv-reduce Install the project...

Does your external/spirv-tools/external/spirv-headers has commit "4f7b471f1a" ?