stride3d / SDSL

Parser and compiler for the Stride shader language into SPIR-V
MIT License
8 stars 1 forks source link

[RFC] Drop support for preprocessor macros in SDSL #9

Open ykafia opened 8 months ago

ykafia commented 8 months ago

SDSL uses preprocessor macros for platform specific code and other uses. It offers a similar user experience for both C# and SDSL.

Since macros can be defined through C# code and used by the preprocessor, this makes SDSL compilation impredictible.

Definitions

The shader compiler can compile shader snippets into sdspv and cache them for future use. This has the advantage of removing the need to parse and compile a shader snippet many times.

In an ideal scenario, if we have a shader snippet composed of many other snippets that were already compiled as sdspv, only our snippet need to be compiled, then the assembler will mix all those sdspv binaries together.

Here's a graph of how the compiler should work

flowchart TD
    subgraph Spirv Compilation
        A[Compile user shader A] --> B{Is shader \nalready \ncompiled ?}
        B --> |yes| Spirv[Return spirv binaries]
        B --> |no| C[Fetch shader code\n from asset system]
        C --> ParseShader[Parse Shader]
        ParseShader --> D{Does shader A \nhave compositions\nor inherited classes\nnot compiled \nto sdspv?}
        D --> |no| E[Compile and mix sdspv A with \nall inherited and composed sdspv]
        E --> F[Assemble shader A \nwith other snippet\n from the inherited and \ncomposed spirv shaders]
        D --> |yes| G[Fetch and compile \ninherited/composed shader \nsnippets to sdspv]
        G --> E
        F --> Spirv
    end
    subgraph Parsing
        Load[Load shader text \nfrom asset system] --> CommentParsing[Parse shader comments]
        CommentParsing --> RemovalQ{Does shader\nhave comments ?}
        RemovalQ --> |yes| Removal[build a list of ReadOnlyMemory \n of char for non-comment \ncode and use StringBuilder to \nconcatenate it into a new string]
        RemovalQ --> |no| AllowedMacros{Are\nmacros\nallowed?}
        AllowedMacros --> |yes| MacroRetrieve
        AllowedMacros --> |no| StartCompilation
        Removal --> AllowedMacros
        subgraph MacroParsing
            MacroRetrieve[Retrieve macro values] --> MacroObject[Replace macro objects by\ntheir defined values]
            MacroObject --> MacroFunc[Replace macro function \nwith their expansions]
            MacroFunc --> BuildString
            BuildString[Build a string \nusing String builder] --> StartCompilation
        end
    end

The issue with macros

The fact that branching through shader macros can become very complex and that a user can change these shader macro values at runtime make caching shader complicated. It also breaks the type system since a single shader can be reused with different definition in different context.

Ideally a shader type would be defined once, with a specific definition across all shaders. Everything that the preprocessor can do can be done through the inheritance and composition system. Since we would have one definition per shader type, we can cache sdspv with just the identifier of the shader type. We cut short of all re-parsing of the same type, saving some precious CPU at the cost of a different UX for users.

Impacts

Both Stride and VL.Stride use macros extensively, this will need some work to rewrite things.

manio143 commented 8 months ago

Overall I agree with your motivation. Manipulating the shader source at runtime sounds like a flexible but costly approach. I suppose it would make sense to leave the choice of using macros (less efficient) to the user if they wish to do so, while recommending to not use it, but the question then becomes how complicated is the preprocessor code.

Can you provide some examples of rewriting a shader that uses macros to one that doesn't? I'm wondering how big the impact would be on existing shaders and if the translation could potentially be automated, so that users upgrading to new version of Stride will have their shaders auto-updated.

Eideren commented 8 months ago

Sounds like a big breaking change but I can see this being highly beneficial and I don't think it's that big an issue. VVVV is likely the only real project that may have to dedicate more time that necessary to adapt. Can you quickly chime in on this @tebjan ? There are a couple of usage that would be trivial to replace, but can you give an example of how you would resolve those two: https://github.com/stride3d/stride/blob/master/sources/engine/Stride.Rendering/Rendering/ComputeEffect/ComputeShaderBase.sdsl#L11-L19 https://github.com/stride3d/stride/blob/master/sources/engine/Stride.Particles/Shaders/ParticleBase.sdsl#L17-L21

ykafia commented 8 months ago

Overall I agree with your motivation. Manipulating the shader source at runtime sounds like a flexible but costly approach. I suppose it would make sense to leave the choice of using macros (less efficient) to the user if they wish to do so, while recommending to not use it, but the question then becomes how complicated is the preprocessor code.

I completely agree on this one, i'm going to implement a new one based on the parser library i made, i think it would be a better idea to remove the use of macros little by little instead of the whole code base.

On Stride's repo we use a binary dll of CppNet, i've been using it on this sdsl repo but the code is imo not easy to maintain, not efficient at all (lots of needless allocations) and generates a lot of warnings that would pollute the compilation of Stride if we included it as a referenced project instead of a binary dll.

ykafia commented 8 months ago

As for rewriting some stuff :

There are a couple of usage that would be trivial to replace, but can you give an example of how you would resolve those two: https://github.com/stride3d/stride/blob/master/sources/engine/Stride.Rendering/Rendering/ComputeEffect/ComputeShaderBase.sdsl#L11-L19 https://github.com/stride3d/stride/blob/master/sources/engine/Stride.Particles/Shaders/ParticleBase.sdsl#L17-L21

This can be done post compilation, on the spirv itself. There are two methods we can use :

For the compute shader, this can be modified through the OpExecutionMode which would be compiled like :
OpExecutionMode %CSMain LocalSize 1 1 1 It would be trivial to change any of the 3 integers.

The other case where we create a stream variable only for specific graphic profiles, i would assume can be done through the composition/inheritance system, this would be either :

shader ComputeSomethingBase
{
    abstract void Foo();
}

shader ComputeSomethingDesktop : ComputeSomethingBase
{
    override void Foo(){...}
}
shader ComputeSomethingAndroid : ComputeSomethingBase
{
    override void Foo(){...}
}

shader ComputeSomething
{
    compose ComputeSomethingBase computeSomething;
    void DoSomething()
    {
        computeSomething.Foo();
    }
}

After encapsulating all different methods in different shader objects, the user would have to define which to use through C#.

For function macros, it would be best to write a function explicitely and have an inline marker usable in spirv. It would fall on the GPU driver or spirv-cross to optimize the code or not.

Something akin to :

#define Add(x,y) x + y
void Foo()
{
    int x = Add(1,2);
}
// Would be re written into
[Inline]
int Add(int x, int y) => x + y;

void Foo()
{
    int x = Add(1,2);
}