On Monday, August 24, 2015 04:14:13 PM Jason Ekstrand wrote: > On Mon, Aug 24, 2015 at 4:03 PM, Kenneth Graunke <kenn...@whitecape.org> > wrote: > > On Wednesday, August 19, 2015 10:45:44 PM Jason Ekstrand wrote: > >> v2 (Jason Ekstrand): Fix up image uniforms > >> --- > >> src/glsl/nir/nir_lower_io.c | 9 +++++++-- > >> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 13 +------------ > >> 2 files changed, 8 insertions(+), 14 deletions(-) > >> > >> diff --git a/src/glsl/nir/nir_lower_io.c b/src/glsl/nir/nir_lower_io.c > >> index 15a4edc..70645b6 100644 > >> --- a/src/glsl/nir/nir_lower_io.c > >> +++ b/src/glsl/nir/nir_lower_io.c > >> @@ -244,9 +244,14 @@ nir_lower_io_block(nir_block *block, void *void_state) > >> nir_src indirect; > >> unsigned offset = get_io_offset(intrin->variables[0], > >> &intrin->instr, &indirect, > >> state); > >> - offset += intrin->variables[0]->var->data.driver_location; > >> > >> - load->const_index[0] = offset; > >> + unsigned location = > >> intrin->variables[0]->var->data.driver_location; > >> + if (mode == nir_var_uniform) { > >> + load->const_index[0] = location; > >> + load->const_index[1] = offset; > >> + } else { > >> + load->const_index[0] = location + offset; > >> + } > >> > >> if (has_indirect) > >> load->src[0] = indirect; > >> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > >> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > >> index 5b54b95..32a05dc 100644 > >> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > >> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > >> @@ -239,18 +239,7 @@ fs_visitor::nir_setup_uniform(nir_variable *var) > >> } > >> > >> if (storage->type->is_image()) { > >> - /* Images don't get a valid location assigned by nir_lower_io() > >> - * because their size is driver-specific, so we need to allocate > >> - * space for them here at the end of the parameter array. > >> - */ > >> - var->data.driver_location = uniforms; > >> - unsigned size = > >> - BRW_IMAGE_PARAM_SIZE * MAX2(storage->array_elements, 1); > >> - > >> - setup_image_uniform_values(uniforms, storage); > >> - > >> - param_size[uniforms] = size; > >> - uniforms += size; > >> + setup_image_uniform_values(index, storage); > >> } else { > >> unsigned slots = storage->type->component_slots(); > >> if (storage->array_elements) > >> > > > > This hunk really looks like it was squashed into the wrong patch... > > You're right. This hunk should go into the patch that starts passing > type_size into nir_lower_io.
Ahh, that makes so much more sense. Squashing this hunk into that patch (07) sounds great. Perhaps add something like this to the commit message of that patch: "In fact, this causes nir_lower_io to allocate space for our image uniforms, allowing us to drop hacks in the backend." Patches 1-13 (other than the ones I wrote) are: Reviewed-by: Kenneth Graunke <kenn...@whitecape.org> This definitely cleans up a lot of mess. Great work!
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev