Hi Lars,
Thankd for working on this, comments below.
On Sun, 2016-04-17 at 13:18 -0400, Lars Hamre wrote:
> Return statements in conditional blocks were not having their
> output varyings lowered correctly.
>
> This patch fixes the following piglit tests:
> /spec/glsl-1.10/execution/vs-float-main-return
> /spec/glsl-1.10/execution/vs-vec2-main-return
> /spec/glsl-1.10/execution/vs-vec3-main-return
>
> Signed-off-by: Lars Hamre <cheme...@gmail.com>
>
> ---
>
> NOTE: Someone with access will need to commit this after the review
> process
>
> src/compiler/glsl/lower_packed_varyings.cpp | 58
> +++++++++++++++++++++++++++--
> 1 file changed, 54 insertions(+), 4 deletions(-)
>
> diff --git a/src/compiler/glsl/lower_packed_varyings.cpp
> b/src/compiler/glsl/lower_packed_varyings.cpp
> index 825cc9e..41edada 100644
> --- a/src/compiler/glsl/lower_packed_varyings.cpp
> +++ b/src/compiler/glsl/lower_packed_varyings.cpp
> @@ -724,6 +724,45 @@
> lower_packed_varyings_gs_splicer::visit_leave(ir_emit_vertex *ev)
> return visit_continue;
> }
>
> +/**
> + * Visitor that splices varying packing code before every return.
> + */
> +class lower_packed_varyings_return_splicer : public
> ir_hierarchical_visitor
> +{
> +public:
> + explicit lower_packed_varyings_return_splicer(void *mem_ctx,
> + const exec_list
> *instructions);
> +
> + virtual ir_visitor_status visit_leave(ir_return *ret);
> +
> +private:
> + /**
> + * Memory context used to allocate new instructions for the
> shader.
> + */
> + void * const mem_ctx;
> +
> + /**
> + * Instructions that should be spliced into place before each
> return.
> + */
> + const exec_list *instructions;
> +};
> +
> +
> +lower_packed_varyings_return_splicer::lower_packed_varyings_return_s
> plicer(
> + void *mem_ctx, const exec_list *instructions)
> + : mem_ctx(mem_ctx), instructions(instructions)
> +{
> +}
> +
> +
> +ir_visitor_status
> +lower_packed_varyings_return_splicer::visit_leave(ir_return *ret)
> +{
> + foreach_in_list(ir_instruction, ir, this->instructions) {
> + ret->insert_before(ir->clone(this->mem_ctx, NULL));
> + }
> + return visit_continue;
> +}
>
> void
> lower_packed_varyings(void *mem_ctx, unsigned locations_used,
> @@ -757,11 +796,22 @@ lower_packed_varyings(void *mem_ctx, unsigned
> locations_used,
> /* Now update all the EmitVertex instances */
> splicer.run(instructions);
> } else {
> - /* For other shader types, outputs need to be lowered at
> the end of
> - * main()
> + /* For other shader types, outputs need to be lowered
> before each
> + * return statement and at the end of main()
> */
> - main_func_sig->body.append_list(&new_variables);
> - main_func_sig->body.append_list(&new_instructions);
> +
> + lower_packed_varyings_return_splicer splicer(mem_ctx,
> &new_instructions);
> +
> + main_func_sig->body.head->insert_before(&new_variables);
> +
> + splicer.run(instructions);
This will walk over everything. I don't think you want that you only
want to walk over the instructions in main right?
For example if you have another function with a return then you would
end up inserting the instructions before that functions return too. You
could probably create a piglit test to detect this.
> +
> + /* Lower outputs at the end of main() if the last
> instruction is not
> + * a return statement
> + */
> + if (((ir_instruction*)instructions->get_tail())->ir_type !=
> ir_type_return) {
> + main_func_sig->body.append_list(&new_instructions);
> + }
Will this work for:
foo1 = vec2(0.5);
if (early_return != 0) {
foo1 = vec2(0.2);
return;
}
I assume it will be fine, but would be good to have a piglit test for
this also.
> }
> } else {
> /* Shader inputs need to be lowered at the beginning of main()
> */
> --
> 2.5.5
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
