Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ykafia opened this issue Feb 25, 2024 · 4 comments
Open

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

ykafia opened this issue Feb 25, 2024 · 4 comments
Labels
rfc Request for comment sdspv Regarding the SDSL flavored SPIR-V spv SPIR-V generator from sdspv

Comments

@ykafia
Copy link
Collaborator

ykafia commented Feb 25, 2024

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

  • Shader snippet : A piece of SDSL code that cannot be assembled as a spirv module, but can be used in inheritance or composition context. The typical .sdsl file that user write
  • sdspv binary : A shader snippet that has been compiled into SDSL flavored spirv binary (invalid spirv) that is meant to be used in inheritance and composition scenarios.
  • spirv binary : A valid spirv module that can be fed to the gpu driver or spirv-cross. It can be composed of many sdspv binaries thanks to the binary mixin system.

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
Loading

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.

@ykafia ykafia added rfc Request for comment sdspv Regarding the SDSL flavored SPIR-V spv SPIR-V generator from sdspv labels Feb 25, 2024
@manio143
Copy link
Member

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
Copy link

Eideren commented Feb 26, 2024

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
Copy link
Collaborator Author

ykafia commented Feb 27, 2024

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
Copy link
Collaborator Author

ykafia commented Feb 27, 2024

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 :

  • Specialization constant for when we'll be using Vulkan or OpenGL 4.6+ with spirv directly.
  • The same as macros but on the spirv assembler level. It would work like macros but instead of modifying the text, it would modify the resulting binaries before being used thanks to the spirv reflection i've implemented.

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 :

  • For global variable declarations we would have different sdfx files that uses different trees of inheritance
  • For code blocks inside functions we need to put them into a separate methods into different shader objects and use composition. For example ComputeSomethingAndroid and ComputeSomethingDesktop would both inherit from ComputeSomethingBase with virtual methods, ComputeSomething would compose a variable of type ComputeSomethingBase that can be either ComputeSomethingAndroid or ComputeSomethingDesktop.
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);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rfc Request for comment sdspv Regarding the SDSL flavored SPIR-V spv SPIR-V generator from sdspv
Projects
None yet
Development

No branches or pull requests

3 participants