Fix invariant propagation #2427
Conversation
|
|
|
@HansKristian-Work These bits were missing from my previous pull request in order to make it work for MSL |
| if (expression_is_forwarded(expr.self) && !expression_suppresses_usage_tracking(expr.self) && | ||
| forced_invariant_temporaries.count(expr.self) == 0) | ||
| if( ( forced_invariant_temporaries.count( expr.self ) == 0 ) && | ||
| ( backend.force_invariant_temporaries || ( expression_is_forwarded( expr.self ) && !expression_suppresses_usage_tracking( expr.self ) ) ) ) |
There was a problem hiding this comment.
Can you explain what the real difference is here? The only difference I can see is that the previous code has an escape hatch for expression_suppresses_usage_tracking.
Is the fix here that for MSL, we should ignore that? Technically I suppose the suppression could be considered code variance, so I can accept that resolution, but I need to understand what you're trying to fix here.
There was a problem hiding this comment.
Without these forced_invariant_temporaries the invariant propagation (i.e., forcing temporaries) stops before reaching the end of the dependency chain.
The intent of this option is to force the whole dependency chain into temporaries, only for MSL.
GLSL/SPIRV output (i.e, android) has not shown this kind of problems so far, thus I'd leave that codepath unchanged unless you think that would also make sense.
There was a problem hiding this comment.
Does #2428 resolve the issue?
There was a problem hiding this comment.
wrt #2428:
On a first check on a single shader abusing CompilerGLSL::flags_to_qualifiers_glsl() to output INV tag it propagates nicely until the beginning of main().
Full rebuild seems good as well, no glitches.
There was a problem hiding this comment.
Thanks for confirming.
|
Please make sure to sign the CLA, otherwise I cannot merge anything. |
|
Merged as part of #2428. |
GLSL/SPIRV output seems to work fine (i.e., no problems/glitches detected when using invariant), thus only for MSL output invariant position dependencies are forced into temporaries.