On Tue, 2017-11-28 at 12:59 -0800, Jason Ekstrand wrote:
> On Tue, Nov 14, 2017 at 10:39 PM, Samuel Iglesias Gonsálvez <
> sigles...@igalia.com> wrote:
>
> > We can write to the same output but in different components, like
> > in this example:
> >
> > layout(location = 0, component = 0) out ivec2 dEQP_FragColor_0;
> > layout(location = 0, component = 2) out ivec2 dEQP_FragColor_1;
> >
> > Therefore, they are not two different outputs but only one.
> >
> > Fixes:
> >
> > dEQP-VK.glsl.440.linkage.varying.component.frag_out.*
> >
> > Signed-off-by: Samuel Iglesias Gonsálvez <sigles...@igalia.com>
> > ---
> > src/compiler/shader_enums.h | 1 +
> > src/intel/vulkan/anv_pipeline.c | 46 +++++++++++++++++++++++++++
> > ---
> > -----------
> > 2 files changed, 32 insertions(+), 15 deletions(-)
> >
> > diff --git a/src/compiler/shader_enums.h
> > b/src/compiler/shader_enums.h
> > index 9d229d4199e..90729dbfd96 100644
> > --- a/src/compiler/shader_enums.h
> > +++ b/src/compiler/shader_enums.h
> > @@ -603,6 +603,7 @@ typedef enum
> > FRAG_RESULT_DATA5,
> > FRAG_RESULT_DATA6,
> > FRAG_RESULT_DATA7,
> > + FRAG_RESULT_MAX, /**< Number of fragment program results */
> > } gl_frag_result;
> >
> > const char *gl_frag_result_name(gl_frag_result result);
> > diff --git a/src/intel/vulkan/anv_pipeline.c
> > b/src/intel/vulkan/anv_
> > pipeline.c
> > index 907b24a758d..92cfc2898f5 100644
> > --- a/src/intel/vulkan/anv_pipeline.c
> > +++ b/src/intel/vulkan/anv_pipeline.c
> > @@ -870,30 +870,28 @@ anv_pipeline_compile_fs(struct anv_pipeline
> > *pipeline,
> > }
> >
> > unsigned num_rts = 0;
> > - struct anv_pipeline_binding rt_bindings[8];
> > + const int max_rt = FRAG_RESULT_MAX - FRAG_RESULT_DATA0;
> > + struct anv_pipeline_binding rt_bindings[max_rt];
> > nir_function_impl *impl = nir_shader_get_entrypoint(nir);
> > + int rt_to_bindings[max_rt];
> > + memset(rt_to_bindings, -1, sizeof(int) * max_rt);
> > +
> > + /* Set new, compacted, location */
> > nir_foreach_variable_safe(var, &nir->outputs) {
> > if (var->data.location < FRAG_RESULT_DATA0)
> > continue;
> >
> > unsigned rt = var->data.location - FRAG_RESULT_DATA0;
> > - if (rt >= key.nr_color_regions) {
> > - /* Out-of-bounds, throw it away */
> > - var->data.mode = nir_var_local;
> > - exec_node_remove(&var->node);
> > - exec_list_push_tail(&impl->locals, &var->node);
> > + if (rt_to_bindings[rt] != -1 || rt >=
> > key.nr_color_regions)
> > continue;
> > - }
> > -
> > - /* Give it a new, compacted, location */
> > - var->data.location = FRAG_RESULT_DATA0 + num_rts;
> > -
> > - unsigned array_len =
> > + const unsigned array_len =
> > glsl_type_is_array(var->type) ? glsl_get_length(var-
> > >type) :
> > 1;
> > - assert(num_rts + array_len <= 8);
> > + assert(num_rts + array_len <= max_rt);
> > +
> > + rt_to_bindings[rt] = num_rts;
> >
>
> But what about arrays? I'm pretty sure that in order to get this
> 100%
> correct in all cases, we need to do the three-pass I suggested
> above. In
> particular, we need rt_to_bindings to have an entry for each array
> element.
Ok, now I understand it better. I will implement it then.
> Also, if we want to handle crazy cases such as a vec2[2] at
> location 0 and a vec2 at location 1 channel 2, we basically have to
> do a
> pre-pass to figure out what all bindings are live before we can start
> assigning anything.
>
> As a side-note, it's abundantly obvious that we need more tests for
> this.
>
Right. I will modify CTS to add more tests.
Thanks!
Sam
> --Jason
>
>
> >
> > for (unsigned i = 0; i < array_len; i++) {
> > - rt_bindings[num_rts + i] = (struct
> > anv_pipeline_binding) {
> > + rt_bindings[rt_to_bindings[rt] + i] = (struct
> > anv_pipeline_binding) {
> > .set = ANV_DESCRIPTOR_SET_COLOR_ATTACHMENTS,
> > .binding = 0,
> > .index = rt + i,
> > @@ -903,6 +901,24 @@ anv_pipeline_compile_fs(struct anv_pipeline
> > *pipeline,
> > num_rts += array_len;
> > }
> >
> > + nir_foreach_variable_safe(var, &nir->outputs) {
> > + if (var->data.location < FRAG_RESULT_DATA0)
> > + continue;
> > +
> > + unsigned rt = var->data.location - FRAG_RESULT_DATA0;
> > + if (rt >= key.nr_color_regions) {
> > + /* Out-of-bounds, throw it away */
> > + var->data.mode = nir_var_local;
> > + exec_node_remove(&var->node);
> > + exec_list_push_tail(&impl->locals, &var->node);
> > + continue;
> > + }
> > +
> > + /* Give it the new location */
> > + assert(rt_to_bindings[rt] != -1);
> > + var->data.location = rt_to_bindings[rt] +
> > FRAG_RESULT_DATA0;
> > + }
> > +
> > if (num_rts == 0) {
> > /* If we have no render targets, we need a null render
> > target */
> > rt_bindings[0] = (struct anv_pipeline_binding) {
> > @@ -913,7 +929,7 @@ anv_pipeline_compile_fs(struct anv_pipeline
> > *pipeline,
> > num_rts = 1;
> > }
> >
> > - assert(num_rts <= 8);
> > + assert(num_rts <= max_rt);
> > map.surface_to_descriptor -= num_rts;
> > map.surface_count += num_rts;
> > assert(map.surface_count <= 256);
> > --
> > 2.14.3
> >
> >
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
