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

Death to baked expressions #4511

Open
jimblandy opened this issue Oct 8, 2023 · 3 comments
Open

Death to baked expressions #4511

jimblandy opened this issue Oct 8, 2023 · 3 comments
Labels
kind: refactor Making existing function faster or nicer naga Shader Translator

Comments

@jimblandy
Copy link
Member

jimblandy commented Oct 8, 2023

Naga IR should be changed to eliminate the need to bake expressions.

Baked expressions are a Naga-specific piece of ugly magic: subexpressions that we identify as needing to be evaluated in isolation, not as part of the Statement that actually refers to them. Baking an expression means that it's emitted as an initialized temporary variable. References to the expression are then rendered as uses of that variable.

Examples would be Load expressions, which need to be ordered properly with respect to Call statements that could assign to the variables they read. But it also includes stuff like the operands of MathFunction::Dot applications on Metal, which open-codes the dot product operation, and wants its operands to be cached in locals in advance to avoid repeated evaluation.

We flag expressions for baking pessimistically. For example, most Loads do not need to be separated out from the rest of the expression, but we produce (in WGSL output) a separate let declaration for every load. For the GLSL input:

    vec4 color = Color;

we generate the WGSL:

    let _e4 = global.Color;
    color = _e4;

That _e4 temporary is the result of baking.

The semantics of Naga IR should be such that backends don't need to filter through the expressions covered by an Emit and play strange games with reference count thresholds to decide what to generate.

A better approach would not only simplify Emit statement processing in backends, but also accommodate backend-specific needs like inlined Dot calls (or the broad selection of functions in the HLSL backend).

@jimblandy
Copy link
Member Author

jimblandy commented Oct 8, 2023

My preference:

  • Eliminate the distinction between needs_pre_emit and ordinary Expressions. Just emit expressions where you need them. (This would let us eliminate back::Emitter.)
  • Replace the operand of Emit with a single expression handle. (This is the only use of arena::Range, btw.)
  • Forbid multiply-referenced expressions unless they are the operands of Emit statements.
  • Provide infrastructure in back/mod.rs to make generating polyfill function definitions cleaner. In other words, suggestions like this one would be a thing of the past. This would serve instead of the backend-specific baking we do in the Metal backend, the GLSL backend, and elsewhere.

@jimblandy
Copy link
Member Author

jimblandy commented Oct 9, 2023

Thinking aloud: the nice thing about Emit as it stands is when you're generating IR for input like this:

a < b + c * function_that_assigns_to_everything() * d + e

This parenthesizes as:

a < (b + ((c * function_that_assigns_to_everything()) * d) + e)

So at the point that we need to generate the call to function_that_assigns_to_everything, we actually have three subexpressions a, b, and c that we must evaluate before the call, and two subexpressions d and e that we must evaluate after the call.

What's nice about today's Emit is that the front end can recurse down into this expression, appending expressions for a, b, and c as it goes, and then when it hits the call, deep in the tree, it can just blorp out an Emit statement whose range covers everything it's written out so far. This is what Emitter is designed to do. The code generating the call doesn't need to have a list of exactly which subexpressions are supposed to have been evaluated before the call; all it has to track is the starting arena index. Or, looking at it differently, the subrange of the expression arena is the list of expressions that must be evaluated before the call.

@teoxoy teoxoy added the kind: refactor Making existing function faster or nicer label Oct 12, 2023
@cwfitzgerald cwfitzgerald transferred this issue from gfx-rs/naga Oct 25, 2023
@cwfitzgerald cwfitzgerald added the naga Shader Translator label Oct 25, 2023
@LegNeato
Copy link
Contributor

FWIW when working on my rust-gpu PR this confused me and made the generated code funky.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: refactor Making existing function faster or nicer naga Shader Translator
Projects
Status: Todo
Development

No branches or pull requests

4 participants