stride3d / stride

Stride Game Engine (formerly Xenko)
https://stride3d.net
MIT License
6.45k stars 933 forks source link

Change SDSL language specification to better match with spirv #1542

Closed ykafia closed 1 year ago

ykafia commented 1 year ago

Related to #14

For sdsl->spirv compilation I've hit a design issue. Since SDSL is supposed to be a superset of HLSL and spirv a kind of low level vulkan GLSL, the many hsls features that we were using (semantics, packing and such) are not going to be the same in SPV, or might not even exist.

On top of that Spirv-Cross and Naga behave very differently when translating SPV to HLSL which might cause issues with shaders.

I propose we make SDSL closely match SPV features to avoid translation confusion.

It should only affect shader I/O (eg : defining layout position and built-in variables instead of using semantics), not the core of the language (everything that's inside a function.

manio143 commented 1 year ago

Can you suggest an example of how this would look in user defined shader (currently vs after changing it to match SPV)? I would also note that this would likely mean a new SDSL language version and require some process of migrating from the old one, ideally automated, but likely manual if the differences are big enough to require language changes outside of syntactic sugar.

ykafia commented 1 year ago

Sure! I'll add some explanations to make sure I understood the issue well too.

Let's look at the the ShaderBaseStream file for example.

SV_Position, SV_TargetX, TEXTURECOORDX etc are all semantics that are used by HLSL to simplify/automatically handle data passed between shader stages. Meaning If we put a semantic of AAA to a vertex shader output variable, in the pixel shader stage, creating an input variable with the AAA semantic will automatically bind the input variable of the PS to the output of the VS.

In GLSL/Vulkan this is handled by layout with locations. It also uses glsl built in values (like Position is bound to the predefined variable gl_Position). Using builtin variables with spirv makes the translation (for both naga and spirv-cross) more correct than if we don't. I've tried to work around it by using extensions of spirv but it's a pain in the neck to make spirv-cross and naga generate correct semantic names. I also started recreating the location thingy that glslangValidator does, but this might be wasteful in cpu and memory.

So for the sake of having a more coherent, simple and faster compilation and translation, i feel it would be best to make SDSL and Spirv share the same logic instead of bending spirv to work with SDSL.

So in SDSL instead of having :

shader ShaderBaseStream {
    stage stream float4 ShadingPosition : SV_Position;
    stage stream float4 ColorTarget : SV_Target0;
    stage stream float4 ColorTarget1 : SV_Target1;
}

We would have :

shader ShaderBaseStream {
    // mandatory builtin definition because this will help spirv-cross and naga enormously while saving ourselves some processing power
    stage stream float4 ShadingPosition : builtin(Position);
    // For non-builtin, either keep the semantic and waste a tidsy bits of processing power and memory to generate layout locations
    stage stream float4 ColorTarget : SV_Target0;
    // Or use the layout location directly 
    stage stream float4 ColorTarget1 : layout(1);
}

Builtin

Example of hlsl compiled to spirv here :

```hlsl float4x4 view_proj_matrix; float4x4 texture_matrix0; struct VS_OUTPUT { float4 Pos : POSITION; float3 Pshade : TEXCOORD0; }; VS_OUTPUT VSMain (float4 vPosition : POSITION) { VS_OUTPUT Out = (VS_OUTPUT) 0; // Transform position to clip space Out.Pos = mul (view_proj_matrix, vPosition); // Transform Pshade Out.Pshade = mul (texture_matrix0, vPosition); return Out; } ``` ``` ; SPIR-V ; Version: 1.0 ; Generator: Khronos Glslang Reference Front End; 10 ; Bound: 79 ; Schema: 0 OpCapability Shader %1 = OpExtInstImport "GLSL.std.450" OpMemoryModel Logical GLSL450 OpEntryPoint Vertex %VSMain "VSMain" %vPosition %_entryPointOutput_Pos %_entryPointOutput_Pshade OpSource HLSL 500 OpName %VSMain "VSMain" OpName %_Global "$Global" OpMemberName %_Global 0 "view_proj_matrix" OpMemberName %_Global 1 "texture_matrix0" OpName %_ "" OpName %vPosition "vPosition" OpName %_entryPointOutput_Pos "@entryPointOutput.Pos" OpName %_entryPointOutput_Pshade "@entryPointOutput.Pshade" OpMemberDecorate %_Global 0 RowMajor OpMemberDecorate %_Global 0 Offset 0 OpMemberDecorate %_Global 0 MatrixStride 16 OpMemberDecorate %_Global 1 RowMajor OpMemberDecorate %_Global 1 Offset 64 OpMemberDecorate %_Global 1 MatrixStride 16 OpDecorate %_Global Block OpDecorate %_ DescriptorSet 0 OpDecorate %_ Binding 0 OpDecorate %vPosition Location 0 ; Here we decorate vPosition with a Location exactly like what we would do with GLSL OpDecorate %_entryPointOutput_Pos Location 0 OpDecorate %_entryPointOutput_Pshade Location 1 %void = OpTypeVoid %3 = OpTypeFunction %void %float = OpTypeFloat 32 %v4float = OpTypeVector %float 4 %v3float = OpTypeVector %float 3 %int = OpTypeInt 32 1 %int_0 = OpConstant %int 0 %mat4v4float = OpTypeMatrix %v4float 4 %_Global = OpTypeStruct %mat4v4float %mat4v4float %_ptr_Uniform__Global = OpTypePointer Uniform %_Global %_ = OpVariable %_ptr_Uniform__Global Uniform %_ptr_Uniform_mat4v4float = OpTypePointer Uniform %mat4v4float %int_1 = OpConstant %int 1 %_ptr_Input_v4float = OpTypePointer Input %v4float %vPosition = OpVariable %_ptr_Input_v4float Input %_ptr_Output_v4float = OpTypePointer Output %v4float %_entryPointOutput_Pos = OpVariable %_ptr_Output_v4float Output %_ptr_Output_v3float = OpTypePointer Output %v3float %_entryPointOutput_Pshade = OpVariable %_ptr_Output_v3float Output %VSMain = OpFunction %void None %3 %5 = OpLabel %50 = OpLoad %v4float %vPosition %67 = OpAccessChain %_ptr_Uniform_mat4v4float %_ %int_0 %68 = OpLoad %mat4v4float %67 %69 = OpVectorTimesMatrix %v4float %50 %68 %72 = OpAccessChain %_ptr_Uniform_mat4v4float %_ %int_1 %73 = OpLoad %mat4v4float %72 %74 = OpVectorTimesMatrix %v4float %50 %73 %75 = OpCompositeExtract %float %74 0 %76 = OpCompositeExtract %float %74 1 %77 = OpCompositeExtract %float %74 2 %78 = OpCompositeConstruct %v3float %75 %76 %77 OpStore %_entryPointOutput_Pos %69 OpStore %_entryPointOutput_Pshade %78 OpReturn OpFunctionEnd ```
manio143 commented 1 year ago

I would definitely go with semantic SV_TargetX notation in order to remove the need for user to calculate layout manually. I'm curious about your comment on "waste processing power and memory to generate layout locations". If SDSL is only a frontend for SPV in this new design then we could perform the conversion at compile time and thus making it easier for the user even for a slightly higher processing cost seems like a good idea.

If it's not too much trouble, can you elaborate why

it's a pain in the neck to make spirv-cross and naga generate correct semantic names

and if there's an existing convention in SDSL for semantic names? If I understand the problem correctly, HLSL doesn't define SV_Position and instead it's just a user defined semantic nickname which could be anything. Since SPV has a set of predefined concepts we would like to define a direct mapping between certain SDSL semantic names and those SPV built-ins. So when user writes SV_Position that would be treated as builtin(Position) for example - thus we could drive enforcement of semantic convention with the transpiler doing the appropriate mapping and the language definition wouldn't have to be augmented. This still may be a breaking change but at least a smaller one IMO.

xen2 commented 1 year ago

SPV being extensible, the idea was:

ykafia commented 1 year ago

If it's not too much trouble, can you elaborate why

Edit : This is better explained here

keeping my previous answer here

GLSL/Spirv and HLSL don't have the same semantics/builtins and rules for locations, we can partially replace some semantics with their corresponding spirv builtins but for the rest we have to emulate what HLSL does. Cross and Naga understand builtins perfectly, but whenever we need an HLSL one they generate [one based on `TEXCOORD` for Cross](https://github.com/KhronosGroup/SPIRV-Cross/issues/163) and `LOCXXX` for Naga. And i can't find a way to make any generate an SV semantic. Long story short : I'm trying to focus into generating correct HLSL by going through GLSL since Stride is heavily dependent on DX11.

and if there's an existing convention in SDSL for semantic names?

SDSL semantics follow the exact same rules as HLSL, where some semantics starting with SV are predefined to make sure some shader stages get read/write access to only specific variables.

Edit : MSFT did that.

We could make our own semantic system thinking of it.

ykafia commented 1 year ago

SPV being extensible, the idea was:

  • Extend SPV with additional metadata/opcode to store whatever info required by SDSL mixing/storage, including our own format to store semantic, etc. Let's call this the SDSL-SPV.
  • Only when generating final Vulkan shaders to be loaded can this stuff be quickly processed/transformed to actual locations. Basically transform from SDSL-SPV -> SPV. (it might have been slightly different, but just to give an idea of how it could be done)

Yes, this is true, i hadn't thought about it. I assume we should define our own SDSL semantic logic for that too

ykafia commented 1 year ago

Just found that Lots of info, it's incredible, So apparently all my questions are answered in the document and on top of that msft allow some decorations for vulkan locations in case anyone wants to.

struct VSInput {
  [[vk::location(0)]] float4 pos  : POSITION;
  [[vk::location(1)]] float3 norm : NORMAL;
};

[[vk::location(1)]]
float4 VSMain(in  VSInput input,
              [[vk::location(2)]]
              in  float4  tex     : TEXCOORD,
              out float4  pos     : SV_Position) : TEXCOORD {
  pos = input.pos;
  return tex;
}
ykafia commented 1 year ago

I think ultimately i will follow the same guidelines that MSFT used for HLSL. I'll do the best i can to emulate the semantics and in bug cases then we'd have to advise users to specify locations. I'll close this issue since the DXC doc is answering most of my design questions and that means we're going to make SDSL work like HLSL as it's supposed to be.