Bug in build_combined_image_samplers

[redorav]

Hi, I have the most recent glslang producing a shader that trips up spirvcross when doing the combined image samplers for opengl.

In short, the code that renames the combined image samplers grabs a name that doesn't correspond to the separate image and sampler it's sourced from. In my shader, for an image texture0 and sampler texture0_ss, it ends up producing the name SPIRV_Cross_Combinedparamparam, where it should be SPIRV_Cross_Combinedtexture0texture0_ss.

compiler->set_name(remap.combined_id, join("SPIRV_Cross_Combined", 
compiler->get_name(remap.image_id),
compiler->get_name(remap.sampler_id)));

I've sent you the spv to your inbox.

[redorav]

It seems like the glslang revision where this starts breaking is

Revision: bed4e4f7e4487a5b9a7d0452fa69cda18ee82f30
Author: John Kessenich <cepheus@frii.com>
Date: 08/09/2017 09:38:07
Message:
HLSL: Pass opaques by local copy, instead of by interface original.

[HansKristian-Work]

uh ... that is not legal SPIR-V afaik. IIRC, you'll need to run an optimization pass to make HLSL legal SPIR-V? I can see how that would break the combined image/sampler pass as-is. Might be doable to fix though ...

Guess I'll have a look at the SPIR-V you sent over. Won't have time to look at it today though.

[redorav]

When you say it's not legal SPIR-V, what do you mean? I disabled the optimization passes I have because it was returning "" as the name in the remapping, without the optimizations I at least get param which suggested the ids were somehow mixed up.

[HansKristian-Work]

Right, if I recall correctly HLSL requires syntax which cannot map directly to SPIR-V because the language allows you to freely copy around opaque handles to variables and such, but only as long as the compiler is able to optimize it away afterwards.

Therefore, the fix I've heard is that glslang will be very lenient and emit kinda out-of-spec SPIR-V, and have SPIRV-Tools optimize it to something "legal" again ... Might be wrong, but that's what I suspect is going on here without digging into it further.

[redorav]

I see... the shader indeed passes a texture/sampler through a function which explains what you say. Looking at the cmake script now I can see the following "spirv-tools not linked - illegal SPIRV may be generated for HLSL"

I'll try this again, perhaps I can figure out which opt pass legalizes this back and never disable this opt pass in our code.

[HansKristian-Work]

If you can reproduce this with SPIRV-Tools enabled in glslang, I'll be happy to take a look, but I'll wait for confirmation before I start digging though.

[redorav]

I've just rebuilt glslang with SPIRV-Tools and I can confirm the issue goes away if optimization is on (which looking at the source, it's on by default for hlsl with a specific comment about these opaque types). It's a bit weird that it relies on optimization to get correct SPIR-V but I'm happy to upgrade the whole lot (I was using outdated sources) if it works :)

I think you can close this, thanks a lot for your help

[HansKristian-Work]

No problem :) Good to know this for future reference.