[Mesa-dev] [RFC] compiler: Move double_inputs to gl_program::DualSlotInputs
Jason Ekstrand
jason at jlekstrand.net
Thu Aug 30 23:48:46 UTC 2018
Previously, we had two field in shader_info: double_inputs_read and
double_inputs. Presumably, the one was for all double inputs that are
read and the other is all that exist. However, because nir_gather_info
regenerates these two values, there is a possibility, if a variable gets
deleted, that the value of double_inputs could change over time. This
is a problem because double_inputs is used to remap the input locations
to a two-slot-per-dvec3/4 scheme for i965. If that mapping were to
change between glsl_to_nir and back-end state setup, we would fall over
when trying to map the NIR outputs back onto the GL location space.
This commit changes the way slot re-mapping works. Instead of the
double_inputs field in shader_info, it adds a DualSlotInputs bitfield to
gl_program. By having it in gl_program, we more easily guarantee that
NIR passes won't touch it after it's been set. It also makes more sense
to put it in a GL data structure since it's really a mapping from GL
slots to back-end and/or NIR slots and not really a NIR shader thing.
This shouldn't affect gallium drivers or radv but I have yet to actually
test it on any of them.
Cc: Kenneth Graunke <kenneth at whitecape.org>
Cc: Timothy Arceri <tarceri at itsqueeze.com>
---
src/compiler/glsl/glsl_to_nir.cpp | 16 +++------
src/compiler/glsl/ir_set_program_inouts.cpp | 2 +-
src/compiler/glsl/serialize.cpp | 2 ++
src/compiler/nir/nir.c | 36 ++++++++++++-------
src/compiler/nir/nir.h | 5 +--
src/compiler/nir/nir_gather_info.c | 6 ----
src/compiler/shader_info.h | 3 --
src/mesa/drivers/dri/i965/brw_draw_upload.c | 19 +++++-----
src/mesa/drivers/dri/i965/genX_state_upload.c | 1 +
src/mesa/main/glspirv.c | 20 +++--------
src/mesa/main/mtypes.h | 15 ++++++++
src/mesa/state_tracker/st_glsl_to_nir.cpp | 2 +-
src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 2 +-
src/mesa/state_tracker/st_program.c | 3 +-
14 files changed, 66 insertions(+), 66 deletions(-)
diff --git a/src/compiler/glsl/glsl_to_nir.cpp b/src/compiler/glsl/glsl_to_nir.cpp
index f38d280d406..d22f4a58dd4 100644
--- a/src/compiler/glsl/glsl_to_nir.cpp
+++ b/src/compiler/glsl/glsl_to_nir.cpp
@@ -149,8 +149,11 @@ glsl_to_nir(const struct gl_shader_program *shader_prog,
* two locations. For instance, if we have in the IR code a dvec3 attr0 in
* location 0 and vec4 attr1 in location 1, in NIR attr0 will use
* locations/slots 0 and 1, and attr1 will use location/slot 2 */
- if (shader->info.stage == MESA_SHADER_VERTEX)
- nir_remap_attributes(shader, options);
+ if (shader->info.stage == MESA_SHADER_VERTEX) {
+ sh->Program->DualSlotInputs = nir_get_dual_slot_attributes(shader);
+ if (options->vs_inputs_dual_locations)
+ nir_remap_dual_slot_attributes(shader, sh->Program->DualSlotInputs);
+ }
shader->info.name = ralloc_asprintf(shader, "GLSL%d", shader_prog->Name);
if (shader_prog->Label)
@@ -344,15 +347,6 @@ nir_visitor::visit(ir_variable *ir)
var->data.compact = ir->type->without_array()->is_scalar();
}
}
-
- /* Mark all the locations that require two slots */
- if (shader->info.stage == MESA_SHADER_VERTEX &&
- glsl_type_is_dual_slot(glsl_without_array(var->type))) {
- for (unsigned i = 0; i < glsl_count_attribute_slots(var->type, true); i++) {
- uint64_t bitfield = BITFIELD64_BIT(var->data.location + i);
- shader->info.vs.double_inputs |= bitfield;
- }
- }
break;
case ir_var_shader_out:
diff --git a/src/compiler/glsl/ir_set_program_inouts.cpp b/src/compiler/glsl/ir_set_program_inouts.cpp
index ba1e44167c3..a3cb19479b8 100644
--- a/src/compiler/glsl/ir_set_program_inouts.cpp
+++ b/src/compiler/glsl/ir_set_program_inouts.cpp
@@ -118,7 +118,7 @@ mark(struct gl_program *prog, ir_variable *var, int offset, int len,
/* double inputs read is only for vertex inputs */
if (stage == MESA_SHADER_VERTEX &&
var->type->without_array()->is_dual_slot())
- prog->info.vs.double_inputs_read |= bitfield;
+ prog->DualSlotInputs |= bitfield;
if (stage == MESA_SHADER_FRAGMENT) {
prog->info.fs.uses_sample_qualifier |= var->data.sample;
diff --git a/src/compiler/glsl/serialize.cpp b/src/compiler/glsl/serialize.cpp
index 889038fb5e2..267700e7e78 100644
--- a/src/compiler/glsl/serialize.cpp
+++ b/src/compiler/glsl/serialize.cpp
@@ -1035,6 +1035,7 @@ write_shader_metadata(struct blob *metadata, gl_linked_shader *shader)
struct gl_program *glprog = shader->Program;
unsigned i;
+ blob_write_uint64(metadata, glprog->DualSlotInputs);
blob_write_bytes(metadata, glprog->TexturesUsed,
sizeof(glprog->TexturesUsed));
blob_write_uint64(metadata, glprog->SamplersUsed);
@@ -1088,6 +1089,7 @@ read_shader_metadata(struct blob_reader *metadata,
{
unsigned i;
+ glprog->DualSlotInputs = blob_read_uint64(metadata);
blob_copy_bytes(metadata, (uint8_t *) glprog->TexturesUsed,
sizeof(glprog->TexturesUsed));
glprog->SamplersUsed = blob_read_uint64(metadata);
diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
index 7ae46845191..f450720dd79 100644
--- a/src/compiler/nir/nir.c
+++ b/src/compiler/nir/nir.c
@@ -1854,23 +1854,33 @@ nir_system_value_from_intrinsic(nir_intrinsic_op intrin)
}
}
+uint64_t
+nir_get_dual_slot_attributes(nir_shader *shader)
+{
+ assert(shader->info.stage == MESA_SHADER_VERTEX);
+
+ uint64_t dual_slot = 0;
+ nir_foreach_variable(var, &shader->inputs) {
+ if (glsl_type_is_dual_slot(glsl_without_array(var->type))) {
+ unsigned slots = glsl_count_attribute_slots(var->type, true);
+ dual_slot |= BITFIELD64_MASK(slots) << var->data.location;
+ }
+ }
+
+ return dual_slot;
+}
+
/* OpenGL utility method that remaps the location attributes if they are
* doubles. Not needed for vulkan due the differences on the input location
* count for doubles on vulkan vs OpenGL
*/
void
-nir_remap_attributes(nir_shader *shader,
- const nir_shader_compiler_options *options)
-{
- if (options->vs_inputs_dual_locations) {
- nir_foreach_variable(var, &shader->inputs) {
- var->data.location +=
- _mesa_bitcount_64(shader->info.vs.double_inputs &
- BITFIELD64_MASK(var->data.location));
- }
- }
+nir_remap_dual_slot_attributes(nir_shader *shader, uint64_t dual_slot)
+{
+ assert(shader->info.stage == MESA_SHADER_VERTEX);
- /* Once the remap is done, reset double_inputs_read, so later it will have
- * which location/slots are doubles */
- shader->info.vs.double_inputs = 0;
+ nir_foreach_variable(var, &shader->inputs) {
+ var->data.location +=
+ _mesa_bitcount_64(dual_slot & BITFIELD64_MASK(var->data.location));
+ }
}
diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index 169fa1fa20d..309dda673be 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -3039,8 +3039,9 @@ bool nir_opt_conditional_discard(nir_shader *shader);
void nir_sweep(nir_shader *shader);
-void nir_remap_attributes(nir_shader *shader,
- const nir_shader_compiler_options *options);
+uint64_t nir_get_dual_slot_attributes(nir_shader *shader);
+void nir_remap_dual_slot_attributes(nir_shader *shader,
+ uint64_t dual_slot);
nir_intrinsic_op nir_intrinsic_from_system_value(gl_system_value val);
gl_system_value nir_system_value_from_intrinsic(nir_intrinsic_op intrin);
diff --git a/src/compiler/nir/nir_gather_info.c b/src/compiler/nir/nir_gather_info.c
index 4a030cb6256..de18c9bd78e 100644
--- a/src/compiler/nir/nir_gather_info.c
+++ b/src/compiler/nir/nir_gather_info.c
@@ -54,11 +54,6 @@ set_io_mask(nir_shader *shader, nir_variable *var, int offset, int len,
else
shader->info.inputs_read |= bitfield;
- /* double inputs read is only for vertex inputs */
- if (shader->info.stage == MESA_SHADER_VERTEX &&
- glsl_type_is_dual_slot(glsl_without_array(var->type)))
- shader->info.vs.double_inputs_read |= bitfield;
-
if (shader->info.stage == MESA_SHADER_FRAGMENT) {
shader->info.fs.uses_sample_qualifier |= var->data.sample;
}
@@ -417,7 +412,6 @@ nir_shader_gather_info(nir_shader *shader, nir_function_impl *entrypoint)
shader->info.system_values_read = 0;
if (shader->info.stage == MESA_SHADER_VERTEX) {
shader->info.vs.double_inputs = 0;
- shader->info.vs.double_inputs_read = 0;
}
if (shader->info.stage == MESA_SHADER_FRAGMENT) {
shader->info.fs.uses_sample_qualifier = false;
diff --git a/src/compiler/shader_info.h b/src/compiler/shader_info.h
index dab15b58894..65bc0588d67 100644
--- a/src/compiler/shader_info.h
+++ b/src/compiler/shader_info.h
@@ -134,9 +134,6 @@ typedef struct shader_info {
struct {
/* Which inputs are doubles */
uint64_t double_inputs;
-
- /* Which inputs are actually read and are double */
- uint64_t double_inputs_read;
} vs;
struct {
diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c b/src/mesa/drivers/dri/i965/brw_draw_upload.c
index bc9b2566deb..6a29d562cb2 100644
--- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
@@ -454,6 +454,8 @@ brw_prepare_vertices(struct brw_context *brw)
{
const struct gen_device_info *devinfo = &brw->screen->devinfo;
struct gl_context *ctx = &brw->ctx;
+ /* BRW_NEW_VERTEX_PROGRAM */
+ const struct gl_program *vp = brw->programs[MESA_SHADER_VERTEX];
/* BRW_NEW_VS_PROG_DATA */
const struct brw_vs_prog_data *vs_prog_data =
brw_vs_prog_data(brw->vs.base.prog_data);
@@ -485,17 +487,14 @@ brw_prepare_vertices(struct brw_context *brw)
/* Accumulate the list of enabled arrays. */
brw->vb.nr_enabled = 0;
- while (vs_inputs) {
- GLuint first = ffsll(vs_inputs) - 1;
- assert (first < 64);
- GLuint index =
- first - DIV_ROUND_UP(_mesa_bitcount_64(vs_prog_data->double_inputs_read &
- BITFIELD64_MASK(first)), 2);
+ for (unsigned index = 0; index < VERT_ATTRIB_MAX; index++) {
+ const unsigned location =
+ index + _mesa_bitcount_64(vp->DualSlotInputs & BITFIELD64_MASK(index));
+ if (!(vs_inputs & BITFIELD64_BIT(location)))
+ continue;
+
struct brw_vertex_element *input = &brw->vb.inputs[index];
- input->is_dual_slot = (vs_prog_data->double_inputs_read & BITFIELD64_BIT(first)) != 0;
- vs_inputs &= ~BITFIELD64_BIT(first);
- if (input->is_dual_slot)
- vs_inputs &= ~BITFIELD64_BIT(first + 1);
+ input->is_dual_slot = (vp->DualSlotInputs & BITFIELD64_BIT(index)) != 0;
brw->vb.enabled[brw->vb.nr_enabled++] = input;
}
diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c
index 09a42e44b08..740cb0c4d2e 100644
--- a/src/mesa/drivers/dri/i965/genX_state_upload.c
+++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
@@ -933,6 +933,7 @@ static const struct brw_tracked_state genX(vertices) = {
.mesa = _NEW_POLYGON,
.brw = BRW_NEW_BATCH |
BRW_NEW_BLORP |
+ BRW_NEW_VERTEX_PROGRAM |
BRW_NEW_VERTICES |
BRW_NEW_VS_PROG_DATA,
},
diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index 1c5b7dd17f3..c53fe0bd07c 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -182,20 +182,6 @@ _mesa_spirv_link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
prog->last_vert_prog = prog->_LinkedShaders[last_vert_stage - 1]->Program;
}
-static void
-nir_compute_double_inputs(nir_shader *shader,
- const nir_shader_compiler_options *options)
-{
- nir_foreach_variable(var, &shader->inputs) {
- if (glsl_type_is_dual_slot(glsl_without_array(var->type))) {
- for (unsigned i = 0; i < glsl_count_attribute_slots(var->type, true); i++) {
- uint64_t bitfield = BITFIELD64_BIT(var->data.location + i);
- shader->info.vs.double_inputs |= bitfield;
- }
- }
- }
-}
-
nir_shader *
_mesa_spirv_to_nir(struct gl_context *ctx,
const struct gl_shader_program *prog,
@@ -278,8 +264,10 @@ _mesa_spirv_to_nir(struct gl_context *ctx,
NIR_PASS_V(nir, nir_split_per_member_structs);
if (nir->info.stage == MESA_SHADER_VERTEX) {
- nir_compute_double_inputs(nir, options);
- nir_remap_attributes(nir, options);
+ uint64_t dual_slot_inputs = nir_get_dual_slot_attributes(nir);
+ if (options->vs_inputs_dual_locations)
+ nir_remap_dual_slot_attributes(nir, dual_slot_inputs);
+ linked_shader->Program->DualSlotInputs = dual_slot_inputs;
}
return nir;
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 5ff0d3227a8..9ed49b7ff24 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2066,6 +2066,21 @@ struct gl_program
/** Is this program written to on disk shader cache */
bool program_written_to_cache;
+ /** A bitfield indicating which vertex shader inputs consume two slots
+ *
+ * This is used for mapping from single-slot input locations in the GL API
+ * to dual-slot double input locations in the shader. This field is set
+ * once as part of linking and never updated again to ensure the mapping
+ * remains consistent.
+ *
+ * Note: There may be dual-slot variables in the original shader source
+ * which do not appear in this bitfield due to having been eliminated by
+ * the compiler prior to DualSlotInputs being calculated. There may also
+ * be bits set in this bitfield which are set but which the shader never
+ * reads due to compiler optimizations eliminating such variables after
+ * DualSlotInputs is calculated.
+ */
+ GLbitfield64 DualSlotInputs;
/** Subset of OutputsWritten outputs written with non-zero index. */
GLbitfield64 SecondaryOutputsWritten;
/** TEXTURE_x_BIT bitmask */
diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp b/src/mesa/state_tracker/st_glsl_to_nir.cpp
index ae2c49960c9..0ee9bd9fef1 100644
--- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
@@ -91,7 +91,7 @@ st_nir_assign_vs_in_locations(struct gl_program *prog, nir_shader *nir)
if ((prog->info.inputs_read & BITFIELD64_BIT(attr)) != 0) {
input_to_index[attr] = num_inputs;
num_inputs++;
- if ((prog->info.vs.double_inputs_read & BITFIELD64_BIT(attr)) != 0) {
+ if ((prog->DualSlotInputs & BITFIELD64_BIT(attr)) != 0) {
/* add placeholder for second part of a double attribute */
num_inputs++;
}
diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index 7b96947c607..a30672dbd62 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -7128,7 +7128,7 @@ get_mesa_program_tgsi(struct gl_context *ctx,
_mesa_copy_linked_program_data(shader_program, shader);
shrink_array_declarations(v->inputs, v->num_inputs,
&prog->info.inputs_read,
- prog->info.vs.double_inputs_read,
+ prog->DualSlotInputs,
&prog->info.patch_inputs_read);
shrink_array_declarations(v->outputs, v->num_outputs,
&prog->info.outputs_written, 0ULL,
diff --git a/src/mesa/state_tracker/st_program.c b/src/mesa/state_tracker/st_program.c
index 8117f4ff8db..af86c47b945 100644
--- a/src/mesa/state_tracker/st_program.c
+++ b/src/mesa/state_tracker/st_program.c
@@ -406,8 +406,7 @@ st_translate_vertex_program(struct st_context *st,
stvp->input_to_index[attr] = stvp->num_inputs;
stvp->index_to_input[stvp->num_inputs] = attr;
stvp->num_inputs++;
- if ((stvp->Base.info.vs.double_inputs_read &
- BITFIELD64_BIT(attr)) != 0) {
+ if ((stvp->Base.DualSlotInputs & BITFIELD64_BIT(attr)) != 0) {
/* add placeholder for second part of a double attribute */
stvp->index_to_input[stvp->num_inputs] = ST_DOUBLE_ATTRIB_PLACEHOLDER;
stvp->num_inputs++;
--
2.17.1
More information about the mesa-dev
mailing list