On Aug 17, 2015 4:08 PM, "Kenneth Graunke" <kenn...@whitecape.org> wrote:
>
> Previously, we used nir_lower_io with the scalar type_size function,
> which mapped VERT_ATTRIB_* locations to...some numbers. Then, in
> fs_visitor::nir_setup_inputs(), we created temporaries indexed by
> those numbers, and emitted MOVs from the actual ATTR registers to
> those temporaries.
>
> This patch reworks our input lowering to produce NIR lower_input
> intrinsics that properly index into the ATTR file, so we can access
> it directly.
>
> No changes in shader-db.
>
> Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
> ---
> src/mesa/drivers/dri/i965/brw_fs.cpp | 2 +-
> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 71
+++++++++-----------------------
> src/mesa/drivers/dri/i965/brw_nir.c | 25 ++++++++++-
> 3 files changed, 45 insertions(+), 53 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 93adbc6..efabb52 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -4527,7 +4527,7 @@ fs_visitor::dump_instruction(backend_instruction
*be_inst, FILE *file)
> fprintf(file, "***m%d***", inst->src[i].reg);
> break;
> case ATTR:
> - fprintf(file, "attr%d", inst->src[i].reg +
inst->src[i].reg_offset);
> + fprintf(file, "attr%d+%d", inst->src[i].reg,
inst->src[i].reg_offset);
> break;
> case UNIFORM:
> fprintf(file, "u%d", inst->src[i].reg +
inst->src[i].reg_offset);
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index 51ba23b..397cdcb 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -56,61 +56,25 @@ fs_visitor::emit_nir_code()
> void
> fs_visitor::nir_setup_inputs(nir_shader *shader)
> {
> + if (stage != MESA_SHADER_FRAGMENT)
> + return;
> +
> nir_inputs = bld.vgrf(BRW_REGISTER_TYPE_F, shader->num_inputs);
>
> foreach_list_typed(nir_variable, var, node, &shader->inputs) {
> - enum brw_reg_type type = brw_type_for_base_type(var->type);
> fs_reg input = offset(nir_inputs, bld, var->data.driver_location);
>
> fs_reg reg;
> - switch (stage) {
> - case MESA_SHADER_VERTEX: {
> - /* Our ATTR file is indexed by VERT_ATTRIB_*, which is the value
> - * stored in nir_variable::location.
> - *
> - * However, NIR's load_input intrinsics use a different index -
an
> - * offset into a single contiguous array containing all inputs.
> - * This index corresponds to the nir_variable::driver_location
field.
> - *
> - * So, we need to copy from fs_reg(ATTR, var->location) to
> - * offset(nir_inputs, var->data.driver_location).
> - */
> - const glsl_type *const t = var->type->without_array();
> - const unsigned components = t->components();
> - const unsigned cols = t->matrix_columns;
> - const unsigned elts = t->vector_elements;
> - unsigned array_length = var->type->is_array() ?
var->type->length : 1;
> - for (unsigned i = 0; i < array_length; i++) {
> - for (unsigned j = 0; j < cols; j++) {
> - for (unsigned k = 0; k < elts; k++) {
> - bld.MOV(offset(retype(input, type), bld,
> - components * i + elts * j + k),
> - offset(fs_reg(ATTR, var->data.location + i,
type),
> - bld, 4 * j + k));
> - }
> - }
> - }
> - break;
> - }
> - case MESA_SHADER_GEOMETRY:
> - case MESA_SHADER_COMPUTE:
> - case MESA_SHADER_TESS_CTRL:
> - case MESA_SHADER_TESS_EVAL:
> - unreachable("fs_visitor not used for these stages yet.");
> - break;
> - case MESA_SHADER_FRAGMENT:
> - if (var->data.location == VARYING_SLOT_POS) {
> - reg =
*emit_fragcoord_interpolation(var->data.pixel_center_integer,
> -
var->data.origin_upper_left);
> - emit_percomp(bld, fs_inst(BRW_OPCODE_MOV,
bld.dispatch_width(),
> - input, reg), 0xF);
> - } else {
> - emit_general_interpolation(input, var->name, var->type,
> - (glsl_interp_qualifier)
var->data.interpolation,
> - var->data.location,
var->data.centroid,
> - var->data.sample);
> - }
> - break;
> + if (var->data.location == VARYING_SLOT_POS) {
> + reg =
*emit_fragcoord_interpolation(var->data.pixel_center_integer,
> +
var->data.origin_upper_left);
> + emit_percomp(bld, fs_inst(BRW_OPCODE_MOV, bld.dispatch_width(),
> + input, reg), 0xF);
> + } else {
> + emit_general_interpolation(input, var->name, var->type,
> + (glsl_interp_qualifier)
var->data.interpolation,
> + var->data.location,
var->data.centroid,
> + var->data.sample);
> }
> }
> }
> @@ -1557,8 +1521,13 @@ fs_visitor::nir_emit_intrinsic(const fs_builder
&bld, nir_intrinsic_instr *instr
> case nir_intrinsic_load_input: {
> unsigned index = 0;
> for (unsigned j = 0; j < instr->num_components; j++) {
> - fs_reg src = offset(retype(nir_inputs, dest.type), bld,
> - instr->const_index[0] + index);
> + fs_reg src;
> + if (stage == MESA_SHADER_VERTEX) {
> + src = offset(fs_reg(ATTR, instr->const_index[0], dest.type),
bld, index);
> + } else {
> + src = offset(retype(nir_inputs, dest.type), bld,
> + instr->const_index[0] + index);
> + }
> if (has_indirect)
> src.reladdr = new(mem_ctx)
fs_reg(get_nir_src(instr->src[0]));
> index++;
> diff --git a/src/mesa/drivers/dri/i965/brw_nir.c
b/src/mesa/drivers/dri/i965/brw_nir.c
> index 224c207..2e2d02b 100644
> --- a/src/mesa/drivers/dri/i965/brw_nir.c
> +++ b/src/mesa/drivers/dri/i965/brw_nir.c
> @@ -28,14 +28,37 @@
> #include "program/prog_to_nir.h"
>
> static void
> +lower_scalar_vs_inputs(nir_shader *nir)
> +{
> + /* Start with the location of the variable's base. */
> + foreach_list_typed(nir_variable, var, node, &nir->inputs) {
> + var->data.driver_location = var->data.location;
> + }
> +
> + /* Now use nir_lower_io to walk dereference chains. Attribute arrays
> + * are loaded as one vec4 per element (or matrix column), so we use
> + * type_size_vec4 here.
> + */
> + nir_lower_io(nir, nir_var_shader_in, type_size_vec4);
> +}
I think I'd rather leave this inline and not split it out. It's easier to
see the interactions that way.
> +
> +static void
> brw_lower_nir_io_scalar(nir_shader *nir, gl_shader_stage stage)
> {
> nir_assign_var_locations_direct_first(nir, &nir->uniforms,
> &nir->num_direct_uniforms,
> &nir->num_uniforms,
> type_size_scalar);
> - nir_assign_var_locations(&nir->inputs, &nir->num_inputs,
type_size_scalar);
> nir_assign_var_locations(&nir->outputs, &nir->num_outputs,
type_size_scalar);
> +
> + switch (stage) {
> + case MESA_SHADER_VERTEX:
> + lower_scalar_vs_inputs(nir);
> + break;
> + default:
> + nir_assign_var_locations(&nir->inputs, &nir->num_inputs,
type_size_scalar);
Again, is "regular" lowering really the right default here? It seems
FS-specific...
> + break;
> + }
> nir_lower_io(nir, -1, type_size_scalar);
> }
>
> --
> 2.5.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
