On Thursday, May 31, 2018 10:02:12 PM PDT Jason Ekstrand wrote:
> Reviewed-by: Caio Marcelo de Oliveira Filho <caio.olive...@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 <kenn...@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) {
>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
