On Tue, 2016-10-25 at 17:59 -0700, Ian Romanick wrote:
> From: Ian Romanick <ian.d.roman...@intel.com>
>
> If the user did not request full linking, link the shader with the
> built-in functions, inline them, and eliminate them. Previous to
> this
> you'd see all these calls to "dot" and "max" in the output. This
> prevented a lot of expected optimizations and cluttered the output.
> This gives it some chance of being useful.
>
> v2: Rebase on top of Ken's "built-ins now" work.
>
> v3: Don't do_common_optimizations if par-linking fails. Update
> expected
> output of warnings tests to prevent 'make check' regressions.
>
> v4: Optimize harder. Most important, do function
> inlining. Otherwise
> it's quite impractical for one function in a file to call another
> function in the same file.
>
> Signed-off-by: Ian Romanick <ian.d.roman...@intel.com>
> ---
> src/compiler/glsl/standalone.cpp | 43
> +++++++++++++++++++++-
> ...-out-function-parameter-shaderout.vert.expected | 2 +
> ...nout-function-parameter-shaderout.vert.expected | 2 +
> .../030-array-as-function-parameter.vert.expected | 2 +
> 4 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/src/compiler/glsl/standalone.cpp
> b/src/compiler/glsl/standalone.cpp
> index 6a4432f..ecf162e 100644
> --- a/src/compiler/glsl/standalone.cpp
> +++ b/src/compiler/glsl/standalone.cpp
> @@ -38,6 +38,8 @@
> #include "standalone.h"
> #include "util/string_to_uint_map.h"
> #include "util/set.h"
> +#include "linker.h"
> +#include "glsl_parser_extras.h"
>
> class add_neg_to_sub_visitor : public ir_hierarchical_visitor {
> public:
> @@ -508,10 +510,47 @@ standalone_compile_shader(const struct
> standalone_options *_options,
> }
> }
>
> - if ((status == EXIT_SUCCESS) && options->do_link) {
> + if (status == EXIT_SUCCESS) {
> _mesa_clear_shader_program_data(whole_program);
>
> - link_shaders(ctx, whole_program);
> + if (options->do_link) {
> + link_shaders(ctx, whole_program);
> + } else {
> + struct gl_shader *const shader = whole_program->Shaders[0];
>
It seems like you only need 'shader' to know to access 'shader->Stage'
in the code below, so maybe just copy the stage here?
> + whole_program->LinkStatus = GL_TRUE;
> + whole_program->_LinkedShaders[shader->Stage] =
> + link_intrastage_shaders(whole_program /* mem_ctx */,
> + ctx,
> + whole_program,
> + whole_program->Shaders,
> + 1,
> + true);
> +
> + /* Par-linking can fail, for example, if there are
> undefined external
> + * references.
> + */
> + if (whole_program->_LinkedShaders[shader->Stage] != NULL) {
The expectation in this case would be that
whole_program->LinkStatus is also GL_TRUE. Maybe worth adding an assert
here to catch inconsistent error handling bugs in the linker?
Either way, this patch is:
Reviewed-by: Iago Toral Quiroga <ito...@igalia.com>
> + struct gl_shader_compiler_options *const
> compiler_options =
> + &ctx->Const.ShaderCompilerOptions[shader->Stage];
> +
> + exec_list *const ir =
> + whole_program->_LinkedShaders[shader->Stage]->ir;
> +
> + bool progress;
> + do {
> + progress = do_function_inlining(ir);
> +
> + progress = do_common_optimization(ir,
> + false,
> + false,
> + compiler_options,
> + true)
> + && progress;
> + } while(progress);
> + }
> + }
> +
> status = (whole_program->LinkStatus) ? EXIT_SUCCESS :
> EXIT_FAILURE;
>
> if (strlen(whole_program->InfoLog) > 0) {
> diff --git a/src/compiler/glsl/tests/warnings/026-out-function-
> parameter-shaderout.vert.expected
> b/src/compiler/glsl/tests/warnings/026-out-function-parameter-
> shaderout.vert.expected
> index e69de29..60d3a8a 100644
> --- a/src/compiler/glsl/tests/warnings/026-out-function-parameter-
> shaderout.vert.expected
> +++ b/src/compiler/glsl/tests/warnings/026-out-function-parameter-
> shaderout.vert.expected
> @@ -0,0 +1,2 @@
> +
> +error: unresolved reference to function `fooFunction'
> diff --git a/src/compiler/glsl/tests/warnings/027-inout-function-
> parameter-shaderout.vert.expected
> b/src/compiler/glsl/tests/warnings/027-inout-function-parameter-
> shaderout.vert.expected
> index 1724975..651818d 100644
> --- a/src/compiler/glsl/tests/warnings/027-inout-function-parameter-
> shaderout.vert.expected
> +++ b/src/compiler/glsl/tests/warnings/027-inout-function-parameter-
> shaderout.vert.expected
> @@ -1 +1,3 @@
> 0:11(14): warning: `willBeDefined' used uninitialized
> +
> +error: unresolved reference to function `fooFunction'
> diff --git a/src/compiler/glsl/tests/warnings/030-array-as-function-
> parameter.vert.expected b/src/compiler/glsl/tests/warnings/030-array-
> as-function-parameter.vert.expected
> index 21cb2c5..b1355d3 100644
> --- a/src/compiler/glsl/tests/warnings/030-array-as-function-
> parameter.vert.expected
> +++ b/src/compiler/glsl/tests/warnings/030-array-as-function-
> parameter.vert.expected
> @@ -5,3 +5,5 @@
> 0:14(20): warning: `undefinedIndex' used uninitialized
> 0:14(51): warning: `undefinedIndex' used uninitialized
> 0:14(82): warning: `undefinedIndex' used uninitialized
> +
> +error: unresolved reference to function `foo'
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
