On 10/26/2016 02:25 AM, Iago Toral wrote:
> 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?
I have applied both suggestions locally. Thanks!
> 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
