[Mesa-dev] [PATCH V2 1/3] i965: Add lowering pass to fold offset into unnormalized coords

[Kenneth Graunke]

On 10/14/2013 01:36 AM, Chris Forbes wrote:
> It turns out that nonzero offsets with gsampler2DRect don't work -- they
> just return garbage. Work around this by folding the offset into the
> coord.
> 
> Done as an IR pass rather than yet another hack in the visitors because
> it's clear what's going on this way. Can possibly reuse this to replace
> the existing txf coord+offset hacks.

I like this - doing this at the IR level is much simpler.  A few
simplification suggestions below...

[snip]
> new file mode 100644
> index 0000000..733c289
> --- /dev/null
> +++ b/src/mesa/drivers/dri/i965/brw_lower_unnormalized_offset.cpp
> @@ -0,0 +1,84 @@
> +/*
> + * Copyright © 2013 Intel Corporation
> + *
> + * 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.
> + */
> +
> +/**
> + * \file brw_lower_unnormalized_offset.cpp
> + *
> + * IR lower pass to convert a texture offset into an adjusted coordinate,
> + * for use with unnormalized coordinates. At least the gather4* messages
> + * on Ivybridge and Haswell make a mess with nonzero offsets.
> + *
> + * \author Chris Forbes <chrisf at ijw.co.nz>
> + */
> +
> +#include "glsl/glsl_types.h"
> +#include "glsl/ir.h"

#include "glsl/ir_builder.h"

> +#include "program/prog_instruction.h" /* For WRITEMASK_* */

using namespace ir_builder;

> +
> +class brw_lower_unnormalized_offset_visitor : public ir_hierarchical_visitor {
> +public:
> +   brw_lower_unnormalized_offset_visitor()
> +   {
> +      progress = false;
> +   }
> +
> +   ir_visitor_status visit_leave(ir_texture *ir);
> +
> +   bool progress;
> +};
> +
> +ir_visitor_status
> +brw_lower_unnormalized_offset_visitor::visit_leave(ir_texture *ir)
> +{
> +   if (ir->sampler->type->sampler_dimensionality != GLSL_SAMPLER_DIM_RECT ||
> +       !ir->offset || ir->op != ir_tg4)
> +      return visit_continue;
> +
> +   void *mem_ctx = ralloc_parent(ir);
> +
> +   ir->coordinate = new (mem_ctx) ir_expression(
> +         ir_binop_add,
> +         ir->coordinate,
> +         new (mem_ctx) ir_expression(
> +            ir_unop_i2f,
> +            ir->offset));
> +   ir->offset = NULL;

ir->coordinate = add(ir->coordinate, i2f(ir->offset));

(and you don't need the mem_ctx temporary either)

> +
> +
> +   progress = true;
> +   return visit_continue;
> +}
> +
> +extern "C" {
> +
> +bool
> +brw_do_lower_unnormalized_offset(exec_list *instructions)
> +{
> +   brw_lower_unnormalized_offset_visitor v;
> +
> +   visit_list_elements(&v, instructions);
> +
> +   return v.progress;
> +}
> +
> +}
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp
> index 5090d22..8816c24 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -169,6 +169,7 @@ brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg)
>        lower_vector_insert(shader->ir, true);
>        brw_do_cubemap_normalize(shader->ir);
>        brw_do_lower_offset_arrays(shader->ir);
> +      brw_do_lower_unnormalized_offset(shader->ir);
>        lower_noise(shader->ir);
>        lower_quadop_vector(shader->ir, false);
>  
>