laurentlb / shader-minifier

Minify and obfuscate GLSL or HLSL code
https://ctrl-alt-test.fr/minifier/
Apache License 2.0
454 stars 34 forks source link

Inline void functions with no returns #469

Open therontarigo opened 2 weeks ago

therontarigo commented 2 weeks ago

If a function has void return type, takes no arguments, and does not use the return statement, it should be possible to inline it. Separating part of a larger function in a separate function in source is sometimes useful for keeping sources organized.

therontarigo commented 2 weeks ago

This is a special case of #470

therontarigo commented 2 weeks ago

In this case (functions with no return) the restriction on no arguments may be unnecessary, caveat being bug #471

eldritchconundrum commented 2 weeks ago

No arguments is a good limitation that removes many edge cases. This is not too hard, but we should still take care of things like:

therontarigo commented 2 weeks ago

Declaration conflicts: Can the whole inlined function get wrapped in { } by the inlining pass and then have braces removed by a later pass if they turn out to be unnecessary?

Local shadowing: How is it handled now? (What happens when a single return expression function references a global variable and gets inlined?)

eldritchconundrum commented 2 weeks ago

Declarations conflicts: yes.

Local shadowing: this is currently detected and prevents automatic inlining.

There's also what Laurent says in https://github.com/laurentlb/shader-minifier/issues/470#issuecomment-2461051355 : we might need to enable inlining only when the void returning function is called directly in a statement expression, as opposed to inside an arbitrary expression (e.g. through operator comma).

therontarigo commented 2 weeks ago

I'm not sure I understand why local shadowing is a challenge. If I have local scope B inside of local scope A inside of global scope G, and an expression inside of B references a var from G, the minifier already knows somehow not to shadow it when renaming vars in A. Having the expression inside of B come from an inlining pass shouldn't change that. Or are you thinking of the case that the input shader has a name reused locally?

we might need to enable inlining only when the void returning function is called directly in a statement expression

agreed. This describes exactly the case I'm interested in.

eldritchconundrum commented 2 weeks ago

The case "the function refer to a global function or variable by a name that is shadowed by a local variable that's in scope at the call site" is not really a challenge to handle here, because we can use the existing implementation of the current inlining to detect it.

However, it illustrates that with inlining, thinking of all possible problems in advance isn't easy... This is why I think it's wise to start with #469 rather than #470.

therontarigo commented 2 weeks ago

For context: the shader I'm working on has some inversion of control situations, where there is a clear separation between framework code and content code, and rather than the content function calling into framework helpers, the framework function executes the content function as needed in a loop. This design is already heavily constrained by performance optimizations, it's not only done for size reasons. Writing the content directly into the framework, or using #include for local scope code is antithetical to good organization. Right now it works for the framework to write some global variables, execute the content subroutine, then read some modified globals. Ideally that can all be done with in and out parameters.