[Mesa-dev] [PATCH v4 029/129] nir: Support deref instructions in lower_wpos_ytransform
Kenneth Graunke
kenneth at whitecape.org
Tue Jun 12 19:50:55 UTC 2018
On Thursday, May 31, 2018 10:02:12 PM PDT Jason Ekstrand wrote:
> Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira at intel.com>
> ---
> src/compiler/nir/nir_lower_wpos_ytransform.c | 53 ++++++++++++++++++++++------
> 1 file changed, 42 insertions(+), 11 deletions(-)
>
> diff --git a/src/compiler/nir/nir_lower_wpos_ytransform.c b/src/compiler/nir/nir_lower_wpos_ytransform.c
> index 9cb5c71..6212702 100644
> --- a/src/compiler/nir/nir_lower_wpos_ytransform.c
> +++ b/src/compiler/nir/nir_lower_wpos_ytransform.c
> @@ -77,11 +77,10 @@ nir_cmp(nir_builder *b, nir_ssa_def *src0, nir_ssa_def *src1, nir_ssa_def *src2)
> /* see emit_wpos_adjustment() in st_mesa_to_tgsi.c */
> static void
> emit_wpos_adjustment(lower_wpos_ytransform_state *state,
> - nir_intrinsic_instr *intr,
> + nir_intrinsic_instr *intr, nir_variable *fragcoord,
> bool invert, float adjX, float adjY[2])
> {
> nir_builder *b = &state->b;
> - nir_variable *fragcoord = intr->variables[0]->var;
> nir_ssa_def *wpostrans, *wpos_temp, *wpos_temp_y, *wpos_input;
>
> assert(intr->dest.is_ssa);
> @@ -144,10 +143,10 @@ emit_wpos_adjustment(lower_wpos_ytransform_state *state,
> }
>
> static void
> -lower_fragcoord(lower_wpos_ytransform_state *state, nir_intrinsic_instr *intr)
> +lower_fragcoord(lower_wpos_ytransform_state *state,
> + nir_intrinsic_instr *intr, nir_variable *fragcoord)
> {
> const nir_lower_wpos_ytransform_options *options = state->options;
> - nir_variable *fragcoord = intr->variables[0]->var;
> float adjX = 0.0f;
> float adjY[2] = { 0.0f, 0.0f };
> bool invert = false;
> @@ -229,7 +228,7 @@ lower_fragcoord(lower_wpos_ytransform_state *state, nir_intrinsic_instr *intr)
> }
> }
>
> - emit_wpos_adjustment(state, intr, invert, adjX, adjY);
> + emit_wpos_adjustment(state, intr, fragcoord, invert, adjX, adjY);
> }
>
> /* turns 'fddy(p)' into 'fddy(fmul(p, transform.x))' */
> @@ -253,7 +252,25 @@ lower_fddy(lower_wpos_ytransform_state *state, nir_alu_instr *fddy)
> fddy->src[0].swizzle[i] = MIN2(i, pt->num_components - 1);
> }
>
> -/* Multiply interp_var_at_offset's offset by transform.x to flip it. */
> +/* Multiply interp_deref_at_offset's offset by transform.x to flip it. */
> +static void
> +lower_interp_deref_at_offset(lower_wpos_ytransform_state *state,
> + nir_intrinsic_instr *interp)
> +{
> + nir_builder *b = &state->b;
> + nir_ssa_def *offset;
> + nir_ssa_def *flip_y;
> +
> + b->cursor = nir_before_instr(&interp->instr);
> +
> + offset = nir_ssa_for_src(b, interp->src[1], 2);
> + flip_y = nir_fmul(b, nir_channel(b, offset, 1),
> + nir_channel(b, get_transform(state), 0));
> + nir_instr_rewrite_src(&interp->instr, &interp->src[1],
> + nir_src_for_ssa(nir_vec2(b, nir_channel(b, offset, 0),
> + flip_y)));
> +}
> +
> static void
> lower_interp_var_at_offset(lower_wpos_ytransform_state *state,
> nir_intrinsic_instr *interp)
> @@ -298,7 +315,21 @@ lower_wpos_ytransform_block(lower_wpos_ytransform_state *state, nir_block *block
> nir_foreach_instr_safe(instr, block) {
> if (instr->type == nir_instr_type_intrinsic) {
> nir_intrinsic_instr *intr = nir_instr_as_intrinsic(instr);
> - if (intr->intrinsic == nir_intrinsic_load_var) {
> + if (intr->intrinsic == nir_intrinsic_load_deref) {
> + nir_deref_instr *deref = nir_src_as_deref(intr->src[0]);
> + nir_variable *var = nir_deref_instr_get_variable(deref);
> +
> + if ((var->data.mode == nir_var_shader_in &&
> + var->data.location == VARYING_SLOT_POS) ||
> + (var->data.mode == nir_var_system_value &&
> + var->data.location == SYSTEM_VALUE_FRAG_COORD)) {
> + /* gl_FragCoord should not have array/struct derefs: */
> + lower_fragcoord(state, intr, var);
> + } else if (var->data.mode == nir_var_system_value &&
> + var->data.location == SYSTEM_VALUE_SAMPLE_POS) {
> + lower_load_sample_pos(state, intr);
> + }
Lots of duplication here :( But I suppose the other case is going away
at the end of the series, so no real point in tidying...
> + } else if (intr->intrinsic == nir_intrinsic_load_var) {
> nir_deref_var *dvar = intr->variables[0];
> nir_variable *var = dvar->var;
>
> @@ -308,16 +339,18 @@ lower_wpos_ytransform_block(lower_wpos_ytransform_state *state, nir_block *block
> var->data.location == SYSTEM_VALUE_FRAG_COORD)) {
> /* gl_FragCoord should not have array/struct derefs: */
> assert(dvar->deref.child == NULL);
> - lower_fragcoord(state, intr);
> + lower_fragcoord(state, intr, var);
> } 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_load_frag_coord) {
> - lower_fragcoord(state, intr);
> + lower_fragcoord(state, intr, NULL);
This can't possibly work. The function immediately dereferences
fragcoord, so it can't legally be NULL. Which then begs the question,
how on earth did it work before? It would just read a non-existent
variable out of the intrinsic and use that (i.e. NULL anyway).
So, I did a bit of poking around, and noticed that everybody calls this
before nir_lower_system_values, so I think this case could simply be
deleted. Not sure whether to do that before or after your patch.
It looks fine other than that pre-existing bit of broken code, so
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
We should really delete it at some point.
> } else if (intr->intrinsic == nir_intrinsic_load_sample_pos) {
> lower_load_sample_pos(state, intr);
> + } else if (intr->intrinsic == nir_intrinsic_interp_deref_at_offset) {
> + lower_interp_deref_at_offset(state, intr);
> } else if (intr->intrinsic == nir_intrinsic_interp_var_at_offset) {
> lower_interp_var_at_offset(state, intr);
> }
> @@ -352,8 +385,6 @@ nir_lower_wpos_ytransform(nir_shader *shader,
> .shader = shader,
> };
>
> - nir_assert_lowered_derefs(shader, nir_lower_load_store_derefs);
> -
> assert(shader->info.stage == MESA_SHADER_FRAGMENT);
>
> nir_foreach_function(function, shader) {
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180612/fd5028a0/attachment.sig>
More information about the mesa-dev
mailing list