Add option to ADCE to remove output variables from interface. #4994
Conversation
|
@Keenuts PTAL. This is the last functional PR in the cross-shader optimization suite. I will submit the EDIC cleanup PR after this and will be done. Thanks for your help with all this code! |
| // | ||
| // If |remove_outputs| is true, allow outputs to be removed from the interface. | ||
| // This is only safe if the caller knows that there is no corresponding input | ||
| // variable in the following shader. It is false by default. |
There was a problem hiding this comment.
it's false by default on the pass constructor, here we still have is mandatory, is that expeted?
There was a problem hiding this comment.
...here we still have is mandatory...
Not sure what you mean when you say it is mandatory here. It is ok if the following shader had an unused interface variable removed, but we do not remove it in this shader. The Vulkan spec says that is ok. It is strictly an optimization to remove it. It is not mandatory.
There was a problem hiding this comment.
I meant the comment says it's false by default, but you require the parameter without default value. (the underlying pass constructor takes has the default value set to false, not this one)
There was a problem hiding this comment.
Ah. I see. Yeah. Not sure why I did it this way. Technically, there were two overloads so it wasn't really mandatory, but I changed to a more standard and simpler way to do this.
Fixed.
|
@Keenuts I believe I have addressed all your concerns. PTAL. |
This can cause interface incompatibility and should only be done if ADCE has been applied to the following shader in the pipeline. For this reason this capability is not available through the CLI but rather only non-default through the API. This functionality is intended as part of a larger cross-shader dead code elimination sequence.
|
@Keenuts I believe I have addressed all your concerns. PTAL. |
|
This broke the ABI on Linux again. Can we stop doing that? I fixed this up once before in #4653 I'll take another look at fixing it again. |
|
Is there a rule to keep ABI stable across versions? For the API I imagine it's implicit, as having a non-stable API makes it hard to use this, but not sure about the ABI. (@s-perron ?) |
This can cause interface incompatibility and should only be done if ADCE has been applied to the following shader in the pipeline. For this reason this capability is not available through the CLI but rather only non-default through the API. This functionality is intended as part of a larger cross-shader dead code elimination sequence and not a standalone optimization.