From bd3fde40681171d91b058e016847f4b69470ba8e Mon Sep 17 00:00:00 2001 From: Timothy Arceri Date: Wed, 2 Nov 2016 09:49:58 +1100 Subject: mesa/glsl: delete previously linked shaders earlier when linking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This moves the delete linked shaders call to _mesa_clear_shader_program_data() which makes sure we delete them before returning due to any validation problems. It also reduces some code duplication. From the OpenGL 4.5 Core spec: "If LinkProgram failed, any information about a previous link of that program object is lost. Thus, a failed link does not restore the old state of program. ... If one of these commands is called with a program for which LinkProgram failed, no error is generated unless otherwise noted. Implementations may return information on variables and interface blocks that would have been active had the program been linked successfully. In cases where the link failed because the program required too many resources, these commands may help applications determine why limits were exceeded." Therefore it's expected that we shouldn't be able to query the program that failed to link and retrieve information about a previously successful link. Before this change the linker was doing validation before freeing the previously linked shaders and therefore could exit on failure before they were freed. This change also fixes an issue in compat profile where a program with no shaders attached is expect to fall back to fixed function but was instead trying to relink IR from a previous link. Reviewed-by: Tapani Pälli Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97715 Cc: "13.0" (cherry picked from commit d2861d682a235993844989f7742c9539c3e10245) --- src/compiler/glsl/linker.cpp | 8 -------- src/compiler/glsl/standalone.cpp | 2 +- src/compiler/glsl/standalone_scaffolding.cpp | 10 +++++++++- src/compiler/glsl/standalone_scaffolding.h | 3 ++- 4 files changed, 12 insertions(+), 11 deletions(-) (limited to 'src/compiler') diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index f5c177e..5220488 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -4785,14 +4785,6 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog) "type of shader\n"); } - for (unsigned int i = 0; i < MESA_SHADER_STAGES; i++) { - if (prog->_LinkedShaders[i] != NULL) { - _mesa_delete_linked_shader(ctx, prog->_LinkedShaders[i]); - } - - prog->_LinkedShaders[i] = NULL; - } - /* Link all shaders for a particular stage and validate the result. */ for (int stage = 0; stage < MESA_SHADER_STAGES; stage++) { diff --git a/src/compiler/glsl/standalone.cpp b/src/compiler/glsl/standalone.cpp index 055c433..f096490 100644 --- a/src/compiler/glsl/standalone.cpp +++ b/src/compiler/glsl/standalone.cpp @@ -421,7 +421,7 @@ standalone_compile_shader(const struct standalone_options *_options, } if ((status == EXIT_SUCCESS) && options->do_link) { - _mesa_clear_shader_program_data(whole_program); + _mesa_clear_shader_program_data(ctx, whole_program); link_shaders(ctx, whole_program); status = (whole_program->LinkStatus) ? EXIT_SUCCESS : EXIT_FAILURE; diff --git a/src/compiler/glsl/standalone_scaffolding.cpp b/src/compiler/glsl/standalone_scaffolding.cpp index 35e40d6..d229368 100644 --- a/src/compiler/glsl/standalone_scaffolding.cpp +++ b/src/compiler/glsl/standalone_scaffolding.cpp @@ -123,8 +123,16 @@ _mesa_delete_linked_shader(struct gl_context *ctx, } void -_mesa_clear_shader_program_data(struct gl_shader_program *shProg) +_mesa_clear_shader_program_data(struct gl_context *ctx, + struct gl_shader_program *shProg) { + for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) { + if (shProg->_LinkedShaders[i] != NULL) { + _mesa_delete_linked_shader(ctx, shProg->_LinkedShaders[i]); + shProg->_LinkedShaders[i] = NULL; + } + } + shProg->NumUniformStorage = 0; shProg->UniformStorage = NULL; shProg->NumUniformRemapTable = 0; diff --git a/src/compiler/glsl/standalone_scaffolding.h b/src/compiler/glsl/standalone_scaffolding.h index b56dd3e..2c04548 100644 --- a/src/compiler/glsl/standalone_scaffolding.h +++ b/src/compiler/glsl/standalone_scaffolding.h @@ -56,7 +56,8 @@ _mesa_delete_linked_shader(struct gl_context *ctx, struct gl_linked_shader *sh); extern "C" void -_mesa_clear_shader_program_data(struct gl_shader_program *); +_mesa_clear_shader_program_data(struct gl_context *ctx, + struct gl_shader_program *); extern "C" void _mesa_shader_debug(struct gl_context *ctx, GLenum type, GLuint *id, -- cgit v1.1 From 9397899aedeec92ec678377785dd78441a74a5fb Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Wed, 2 Nov 2016 13:35:30 -0700 Subject: glsl: Update deref types when resizing implicitly sized arrays. At link time, we resolve the size of implicitly sized arrays. When doing so, we update the type of the ir_variables. However, we neglected to update the type of ir_dereference nodes which reference those variables. It turns out array_resize_visitor (for GS/TCS/TES interface array handling) already did 2/3 of the cases for this, so we can simply refactor the code and reuse it. This fixes: GL45-CTS.shader_storage_buffer_object.basic-syntax GL45-CTS.shader_storage_buffer_object.basic-syntaxSSO which have an SSBO containing an implicitly sized array, followed by some other members. setup_buffer_access uses the dereference types to compute offsets to fields, and it had a stale type where the implicitly sized array's length was still 0 instead of the actual length. While we're here, we can also fix update_array_sizes to properly update deref types as well, fixing a FINISHME from 2010. Cc: mesa-stable@lists.freedesktop.org Signed-off-by: Kenneth Graunke Reviewed-by: Timothy Arceri (cherry picked from commit 8df4aebc94337983194cc72c817c08ee938117a1) --- src/compiler/glsl/linker.cpp | 70 +++++++++++++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 23 deletions(-) (limited to 'src/compiler') diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index 5220488..f62a848 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -181,7 +181,43 @@ private: }; -class array_resize_visitor : public ir_hierarchical_visitor { +/** + * A visitor helper that provides methods for updating the types of + * ir_dereferences. Classes that update variable types (say, updating + * array sizes) will want to use this so that dereference types stay in sync. + */ +class deref_type_updater : public ir_hierarchical_visitor { +public: + virtual ir_visitor_status visit(ir_dereference_variable *ir) + { + ir->type = ir->var->type; + return visit_continue; + } + + virtual ir_visitor_status visit_leave(ir_dereference_array *ir) + { + const glsl_type *const vt = ir->array->type; + if (vt->is_array()) + ir->type = vt->fields.array; + return visit_continue; + } + + virtual ir_visitor_status visit_leave(ir_dereference_record *ir) + { + for (unsigned i = 0; i < ir->record->type->length; i++) { + const struct glsl_struct_field *field = + &ir->record->type->fields.structure[i]; + if (strcmp(field->name, ir->field) == 0) { + ir->type = field->type; + break; + } + } + return visit_continue; + } +}; + + +class array_resize_visitor : public deref_type_updater { public: unsigned num_vertices; gl_shader_program *prog; @@ -240,24 +276,6 @@ public: return visit_continue; } - - /* Dereferences of input variables need to be updated so that their type - * matches the newly assigned type of the variable they are accessing. */ - virtual ir_visitor_status visit(ir_dereference_variable *ir) - { - ir->type = ir->var->type; - return visit_continue; - } - - /* Dereferences of 2D input arrays need to be updated so that their type - * matches the newly assigned type of the array they are accessing. */ - virtual ir_visitor_status visit_leave(ir_dereference_array *ir) - { - const glsl_type *const vt = ir->array->type; - if (vt->is_array()) - ir->type = vt->fields.array; - return visit_continue; - } }; /** @@ -1353,7 +1371,7 @@ move_non_declarations(exec_list *instructions, exec_node *last, * it inside that function leads to compiler warnings with some versions of * gcc. */ -class array_sizing_visitor : public ir_hierarchical_visitor { +class array_sizing_visitor : public deref_type_updater { public: array_sizing_visitor() : mem_ctx(ralloc_context(NULL)), @@ -2273,6 +2291,8 @@ update_array_sizes(struct gl_shader_program *prog) if (prog->_LinkedShaders[i] == NULL) continue; + bool types_were_updated = false; + foreach_in_list(ir_instruction, node, prog->_LinkedShaders[i]->ir) { ir_variable *const var = node->as_variable(); @@ -2328,11 +2348,15 @@ update_array_sizes(struct gl_shader_program *prog) var->type = glsl_type::get_array_instance(var->type->fields.array, size + 1); - /* FINISHME: We should update the types of array - * dereferences of this variable now. - */ + types_were_updated = true; } } + + /* Update the types of dereferences in case we changed any. */ + if (types_were_updated) { + deref_type_updater v; + v.run(prog->_LinkedShaders[i]->ir); + } } } -- cgit v1.1 From 996c20208fdbf50e64480ebe9d87c256b939be4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolai=20H=C3=A4hnle?= Date: Mon, 31 Oct 2016 14:03:10 +0100 Subject: glsl: fix lowering of UBO references of named blocks When a UBO reference has the form block_name.foo where block_name refers to a block where the first member has a non-zero offset, the base offset was incorrectly added to the reference. Fixes an assertion triggered in debug builds by GL45-CTS.enhanced_layouts.uniform_block_layout_qualifier_conflict. That test doesn't properly check for correct execution in this case, so I am also going to send out a piglit test. Cc: 13.0 Reviewed-by: Iago Toral Quiroga (cherry picked from commit 37d646c1b3626ad54ed93a784824af7b5abe8a99) --- src/compiler/glsl/lower_ubo_reference.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'src/compiler') diff --git a/src/compiler/glsl/lower_ubo_reference.cpp b/src/compiler/glsl/lower_ubo_reference.cpp index 37134a9..eafa1dd 100644 --- a/src/compiler/glsl/lower_ubo_reference.cpp +++ b/src/compiler/glsl/lower_ubo_reference.cpp @@ -107,7 +107,6 @@ public: struct gl_linked_shader *shader; bool clamp_block_indices; - struct gl_uniform_buffer_variable *ubo_var; const struct glsl_struct_field *struct_field; ir_variable *variable; ir_rvalue *uniform_block; @@ -308,8 +307,11 @@ lower_ubo_reference_visitor::setup_for_load_or_store(void *mem_ctx, this->uniform_block = index; } - this->ubo_var = var->is_interface_instance() - ? &blocks[i]->Uniforms[0] : &blocks[i]->Uniforms[var->data.location]; + if (var->is_interface_instance()) { + *const_offset = 0; + } else { + *const_offset = blocks[i]->Uniforms[var->data.location].Offset; + } break; } @@ -317,8 +319,6 @@ lower_ubo_reference_visitor::setup_for_load_or_store(void *mem_ctx, assert(this->uniform_block); - *const_offset = ubo_var->Offset; - this->struct_field = NULL; setup_buffer_access(mem_ctx, deref, offset, const_offset, row_major, matrix_columns, &this->struct_field, packing); -- cgit v1.1 From 2789bfdbb59c4528a248dd8c02124dd384caefe6 Mon Sep 17 00:00:00 2001 From: Francisco Jerez Date: Tue, 1 Nov 2016 11:56:13 -0700 Subject: nir: Flip gl_SamplePosition in nir_lower_wpos_ytransform(). Assuming the hardware is set up to use a screen coordinate system flipped vertically with respect to the GL's window coordinate system, the SYSTEM_VALUE_SAMPLE_POS vector will also be flipped vertically with respect to the value expected by the GL, so we need to give it the same treatment as gl_FragCoord. Fixes the following CTS tests on i965: ES31-CTS.functional.shaders.multisample_interpolation.interpolate_at_offset.at_sample_position.default_framebuffer ES31-CTS.functional.shaders.sample_variables.sample_pos.correctness.default_framebuffer when run with any multisample configuration, e.g. rgba8888d24s8ms4. Cc: Reviewed-by: Kenneth Graunke Reviewed-by: Anuj Phogat (cherry picked from commit f3d387867f74ae758b41168f23992671f7dce254) --- src/compiler/nir/nir_lower_wpos_ytransform.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'src/compiler') diff --git a/src/compiler/nir/nir_lower_wpos_ytransform.c b/src/compiler/nir/nir_lower_wpos_ytransform.c index 173f058..f211c73 100644 --- a/src/compiler/nir/nir_lower_wpos_ytransform.c +++ b/src/compiler/nir/nir_lower_wpos_ytransform.c @@ -273,6 +273,26 @@ lower_interp_var_at_offset(lower_wpos_ytransform_state *state, } static void +lower_load_sample_pos(lower_wpos_ytransform_state *state, + nir_intrinsic_instr *intr) +{ + nir_builder *b = &state->b; + b->cursor = nir_after_instr(&intr->instr); + + nir_ssa_def *pos = &intr->dest.ssa; + nir_ssa_def *scale = nir_channel(b, get_transform(state), 0); + nir_ssa_def *neg_scale = nir_channel(b, get_transform(state), 2); + /* Either y or 1-y for scale equal to 1 or -1 respectively. */ + nir_ssa_def *flipped_y = + nir_fadd(b, nir_fmax(b, neg_scale, nir_imm_float(b, 0.0)), + nir_fmul(b, nir_channel(b, pos, 1), scale)); + nir_ssa_def *flipped_pos = nir_vec2(b, nir_channel(b, pos, 0), flipped_y); + + nir_ssa_def_rewrite_uses_after(&intr->dest.ssa, nir_src_for_ssa(flipped_pos), + flipped_pos->parent_instr); +} + +static void lower_wpos_ytransform_block(lower_wpos_ytransform_state *state, nir_block *block) { nir_foreach_instr_safe(instr, block) { @@ -287,6 +307,10 @@ lower_wpos_ytransform_block(lower_wpos_ytransform_state *state, nir_block *block /* gl_FragCoord should not have array/struct deref's: */ assert(dvar->deref.child == NULL); lower_fragcoord(state, intr); + } else if (var->data.mode == nir_var_system_value && + var->data.location == SYSTEM_VALUE_SAMPLE_POS) { + assert(dvar->deref.child == NULL); + lower_load_sample_pos(state, intr); } } else if (intr->intrinsic == nir_intrinsic_interp_var_at_offset) { lower_interp_var_at_offset(state, intr); -- cgit v1.1 From fa6c02787e1c4d650b7aaa528a7c4e5e129e2906 Mon Sep 17 00:00:00 2001 From: Dave Airlie Date: Wed, 2 Nov 2016 01:22:07 +0000 Subject: nir: add conditional discard optimisation (v4) This is ported from GLSL and converts if (cond) discard; into discard_if(cond); This removes a block, but also is needed by radv to workaround a bug in the LLVM backend. v2: handle if (a) discard_if(b) (nha) cleanup and drop pointless loop (Matt) make sure there are no dependent phis (Eric) v3: make sure only one instruction in the then block. v4: remove sneaky tabs, add cursor init (Eric) Reviewed-by: Eric Anholt Cc: "13.0" Signed-off-by: Dave Airlie (cherry picked from commit b16dff2d88302e5113598a818d2f92f8af02cd79) --- src/compiler/Makefile.sources | 1 + src/compiler/nir/nir.h | 2 + src/compiler/nir/nir_opt_conditional_discard.c | 125 +++++++++++++++++++++++++ 3 files changed, 128 insertions(+) create mode 100644 src/compiler/nir/nir_opt_conditional_discard.c (limited to 'src/compiler') diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources index a30443d..2bb4868 100644 --- a/src/compiler/Makefile.sources +++ b/src/compiler/Makefile.sources @@ -227,6 +227,7 @@ NIR_FILES = \ nir/nir_metadata.c \ nir/nir_move_vec_src_uses_to_dest.c \ nir/nir_normalize_cubemap_coords.c \ + nir/nir_opt_conditional_discard.c \ nir/nir_opt_constant_folding.c \ nir/nir_opt_copy_propagate.c \ nir/nir_opt_cse.c \ diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h index d6c8efa..0369700 100644 --- a/src/compiler/nir/nir.h +++ b/src/compiler/nir/nir.h @@ -2625,6 +2625,8 @@ bool nir_opt_remove_phis(nir_shader *shader); bool nir_opt_undef(nir_shader *shader); +bool nir_opt_conditional_discard(nir_shader *shader); + void nir_sweep(nir_shader *shader); nir_intrinsic_op nir_intrinsic_from_system_value(gl_system_value val); diff --git a/src/compiler/nir/nir_opt_conditional_discard.c b/src/compiler/nir/nir_opt_conditional_discard.c new file mode 100644 index 0000000..2fde179 --- /dev/null +++ b/src/compiler/nir/nir_opt_conditional_discard.c @@ -0,0 +1,125 @@ +/* + * Copyright © 2016 Red Hat + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + */ + +#include "nir.h" +#include "nir_builder.h" + +/** @file nir_opt_conditional_discard.c + * + * Handles optimization of lowering if (cond) discard to discard_if(cond). + */ + +static bool +nir_opt_conditional_discard_block(nir_block *block, void *mem_ctx) +{ + nir_builder bld; + + if (nir_cf_node_is_first(&block->cf_node)) + return false; + + nir_cf_node *prev_node = nir_cf_node_prev(&block->cf_node); + if (prev_node->type != nir_cf_node_if) + return false; + + nir_if *if_stmt = nir_cf_node_as_if(prev_node); + nir_block *then_block = nir_if_first_then_block(if_stmt); + nir_block *else_block = nir_if_first_else_block(if_stmt); + + /* check there is only one else block and it is empty */ + if (nir_if_last_else_block(if_stmt) != else_block) + return false; + if (!exec_list_is_empty(&else_block->instr_list)) + return false; + + /* check there is only one then block and it has only one instruction in it */ + if (nir_if_last_then_block(if_stmt) != then_block) + return false; + if (exec_list_is_empty(&then_block->instr_list)) + return false; + if (exec_list_length(&then_block->instr_list) > 1) + return false; + /* + * make sure no subsequent phi nodes point at this if. + */ + nir_block *after = nir_cf_node_as_block(nir_cf_node_next(&if_stmt->cf_node)); + nir_foreach_instr_safe(instr, after) { + if (instr->type != nir_instr_type_phi) + break; + nir_phi_instr *phi = nir_instr_as_phi(instr); + + nir_foreach_phi_src(phi_src, phi) { + if (phi_src->pred == then_block || + phi_src->pred == else_block) + return false; + } + } + + /* Get the first instruction in the then block and confirm it is + * a discard or a discard_if + */ + nir_instr *instr = nir_block_first_instr(then_block); + if (instr->type != nir_instr_type_intrinsic) + return false; + + nir_intrinsic_instr *intrin = nir_instr_as_intrinsic(instr); + if (intrin->intrinsic != nir_intrinsic_discard && + intrin->intrinsic != nir_intrinsic_discard_if) + return false; + + nir_src cond; + + nir_builder_init(&bld, mem_ctx); + bld.cursor = nir_before_cf_node(prev_node); + if (intrin->intrinsic == nir_intrinsic_discard) + cond = if_stmt->condition; + else + cond = nir_src_for_ssa(nir_iand(&bld, + nir_ssa_for_src(&bld, if_stmt->condition, 1), + nir_ssa_for_src(&bld, intrin->src[0], 1))); + + nir_intrinsic_instr *discard_if = + nir_intrinsic_instr_create(mem_ctx, nir_intrinsic_discard_if); + nir_src_copy(&discard_if->src[0], &cond, discard_if); + + nir_instr_insert_before_cf(prev_node, &discard_if->instr); + nir_instr_remove(&intrin->instr); + nir_cf_node_remove(&if_stmt->cf_node); + + return true; +} + +bool +nir_opt_conditional_discard(nir_shader *shader) +{ + bool progress = false; + + nir_foreach_function(function, shader) { + if (function->impl) { + void *mem_ctx = ralloc_parent(function->impl); + nir_foreach_block_safe(block, function->impl) { + progress |= nir_opt_conditional_discard_block(block, mem_ctx); + } + } + } + return progress; +} -- cgit v1.1