search

KhronosGroup / glslang

Khronos-reference front end for GLSL/ESSL, partial front end for HLSL, and a SPIR-V generator.
Other
2.91k stars 816 forks source link

Preprocessed source in SPIR-V OpSource would be beneficial. #1611

Closed greg-lunarg closed 5 years ago

greg-lunarg commented 5 years ago

It would be beneficial if preprocessed source were available in SPIR-V OpSource, when possible. Currently glslang is using the original source file content in OpSource.

The main motivator here is to help bindless GPU-assisted validation (and similar tools) output the actual text of the shader source line of interest to the user.

Preprocessed source will have (at least) two benefits over the original source file.

include directives will be expanded and thus their associated file contents will be made directly available in the SPIR-V.

Also all comments are removed in preprocessed source so any such tool will not need to worry about parsing comments in order to correctly ignore #line directives that have been commented out.

If there is concern about this policy, for example, consequent SPIR-V size, this capability could be put under an option.

A .zip with sample HLSL source is attached. It should be compiled with the following command line:

glslangValidator.exe -D -e MainPs -V -g -Od -o foo4.frag.new.spv foo4.frag

greg-lunarg commented 5 years ago

i3.zip

greg-lunarg commented 5 years ago

@johnkslang I was planning to work on this. Let me know if you have concerns or suggestions.

baldurk commented 5 years ago

How would this work exactly? Is there a way currently in SPIR-V to add a second separate OpSource with different information without confusing existing tools handling OpSource? If not it might need a SPIR-V extension, perhaps with SPV_GOOGLE_decorate_string you could add a decoration onto the file's OpString with the pre-processed source.

I'm not sure I entirely understand the motivation though. I don't think it does yet, but if glslang added OpSource information for all files used then tools can run the pre-process themselves if they need to. This information will be needed anyway for tools that want the original source in compilable form, so I'm not sure there's much value in also including pre-processed source as well.

danginsburg commented 5 years ago

I'm not sure I entirely understand the motivation though.

So I think the idea here was as follows: let's say a tool like RenderDoc wants to allow shader edits or debugging based on the source code in OpSource. If that code were GLSL or HLSL that used #include's, and it wasn't pre-processed before being given to RenderDoc, then RenderDoc can't do edits or debug it - right? There is no way for it to get at the #include files that were there.

greg-lunarg commented 5 years ago

There is no way for it to get at the #include files that were there.

There are certainly no guarantees. #includes are not required to have full path names, and the include file might not even be on the same machine.

The goal is to only make life easier for tools like Renderdoc. I don't think we will be doing anything that will make things harder or impossible. We should not be doing anything that will be incompatible with existing tools. We will not be generating anything different than what can already be seen in OpSource today.

The one concern I could imagine would be that of size, but even though we are expanding #includes, preprocessed source should have size on the order of the SPIR-V generated from it. Comments will be removed, and any #if and #ifdefs that evaluate to 0 will have their content removed.

baldurk commented 5 years ago

I'd strongly prefer to have all of the original files present as an OpSource each. This is what DXBC does and it works well - each file that participates in the build gets a pair of (absolute pathname, contents).

That way you have the maximum information and you can always preprocess down if you need everything in one file, but you can have the files separately if needed. Going the other way is not possible especially if comments and inactive preprocessor code is removed.

Having only pre-processed source would be technically valid but significantly less user friendly and not desirable.

greg-lunarg commented 5 years ago

Well, this isn't quite as handy for the validation layers' purposes, but they can probably work with it. It probably makes sense to have similar behavior to DXBC. It is at least a step in the right direction for glslang. I agree there is loss of information with my first proposal.

If no one has major issues with changing directions here, I will close this issue and open another with Baldur's proposal.