Re: [Mesa-dev] [PATCH 1/2] draw: fix undefined input handling some more...

[Nicolai Hähnle]

On 03.11.2016 02:15, srol...@vmware.com wrote:
From: Roland Scheidegger <srol...@vmware.com>
Previous fixes were incomplete - some code still iterated through the number
of elements provided by velem layout instead of the number stored in the key
(which is the same as the number defined by the vs). And also actually
accessed the elements from the layout directly instead of those in the key.
This mismatch could still cause crashes.
(Besides, it is a very good idea to only use data stored in the key anyway.)

Makes sense to me. It would make sense to pull the check for 
PIPE_FORMAT_NONE from generate_fetch into draw_llvm_generate for 
consistency between the two loops. Either way,

Reviewed-by: Nicolai Hähnle <nicolai.haeh...@amd.com>

---
 src/gallium/auxiliary/draw/draw_llvm.c | 77 ++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 37 deletions(-)

diff --git a/src/gallium/auxiliary/draw/draw_llvm.c 
b/src/gallium/auxiliary/draw/draw_llvm.c
index 2f82d9d..d5fc1c2 100644
--- a/src/gallium/auxiliary/draw/draw_llvm.c
+++ b/src/gallium/auxiliary/draw/draw_llvm.c
@@ -1658,10 +1658,10 @@ draw_llvm_generate(struct draw_llvm *llvm, struct 
draw_llvm_variant *variant,
    /*
     * Pre-calculate everything which is constant per shader invocation.
     */
-   for (j = 0; j < draw->pt.nr_vertex_elements; ++j) {
+   for (j = 0; j < key->nr_vertex_elements; ++j) {
       LLVMValueRef vb_buffer_offset, buffer_size;
       LLVMValueRef vb_info, vbuffer_ptr;
-      struct pipe_vertex_element *velem = &draw->pt.vertex_element[j];
+      struct pipe_vertex_element *velem = &key->vertex_element[j];
       LLVMValueRef vb_index =
          lp_build_const_int32(gallivm, velem->vertex_buffer_index);
       LLVMValueRef bsize = lp_build_const_int32(gallivm,
@@ -1669,41 +1669,44 @@ draw_llvm_generate(struct draw_llvm *llvm, struct 
draw_llvm_variant *variant,
       LLVMValueRef src_offset = lp_build_const_int32(gallivm,
                                                      velem->src_offset);

-      vbuffer_ptr = LLVMBuildGEP(builder, vbuffers_ptr, &vb_index, 1, "");
-      vb_info = LLVMBuildGEP(builder, vb_ptr, &vb_index, 1, "");
-      vb_stride[j] = draw_jit_vbuffer_stride(gallivm, vb_info);
-      vb_buffer_offset = draw_jit_vbuffer_offset(gallivm, vb_info);
-      map_ptr[j] = draw_jit_dvbuffer_map(gallivm, vbuffer_ptr);
-      buffer_size = draw_jit_dvbuffer_size(gallivm, vbuffer_ptr);
-
-      ofbit[j] = NULL;
-      stride_fixed[j] = lp_build_uadd_overflow(gallivm, vb_buffer_offset,
-                                               src_offset, &ofbit[j]);
-      buffer_size_adj[j] = lp_build_usub_overflow(gallivm, buffer_size, bsize,
-                                                   &ofbit[j]);
-
-      if (velem->instance_divisor) {
-         /* Index is equal to the start instance plus the number of current
-          * instance divided by the divisor. In this case we compute it as:
-          * index = start_instance + (instance_id  / divisor)
-          */
-         LLVMValueRef current_instance;
-         current_instance = LLVMBuildUDiv(builder, system_values.instance_id,
-                                          lp_build_const_int32(gallivm,
-                                                               
velem->instance_divisor),
-                                          "instance_divisor");
-         instance_index[j] = lp_build_uadd_overflow(gallivm, start_instance,
-                                                    current_instance, 
&ofbit[j]);
-      }
+      if (velem->src_format != PIPE_FORMAT_NONE) {
+         vbuffer_ptr = LLVMBuildGEP(builder, vbuffers_ptr, &vb_index, 1, "");
+         vb_info = LLVMBuildGEP(builder, vb_ptr, &vb_index, 1, "");
+         vb_stride[j] = draw_jit_vbuffer_stride(gallivm, vb_info);
+         vb_buffer_offset = draw_jit_vbuffer_offset(gallivm, vb_info);
+         map_ptr[j] = draw_jit_dvbuffer_map(gallivm, vbuffer_ptr);
+         buffer_size = draw_jit_dvbuffer_size(gallivm, vbuffer_ptr);
+
+         ofbit[j] = NULL;
+         stride_fixed[j] = lp_build_uadd_overflow(gallivm, vb_buffer_offset,
+                                                  src_offset, &ofbit[j]);
+         buffer_size_adj[j] = lp_build_usub_overflow(gallivm, buffer_size, 
bsize,
+                                                     &ofbit[j]);
+
+         if (velem->instance_divisor) {
+            /* Index is equal to the start instance plus the number of current
+             * instance divided by the divisor. In this case we compute it as:
+             * index = start_instance + (instance_id  / divisor)
+             */
+            LLVMValueRef current_instance;
+            current_instance = LLVMBuildUDiv(builder, 
system_values.instance_id,
+                                             lp_build_const_int32(gallivm,
+                                                                  
velem->instance_divisor),
+                                             "instance_divisor");
+            instance_index[j] = lp_build_uadd_overflow(gallivm, start_instance,
+                                                       current_instance, 
&ofbit[j]);
+         }

-      if (0) {
-         lp_build_printf(gallivm, "vbuf index = %u, vb_stride is %u\n",
-                         vb_index, vb_stride[j]);
-         lp_build_printf(gallivm, "   vb_buffer_offset = %u, src_offset is 
%u\n",
-                         vb_buffer_offset, src_offset);
-         lp_build_print_value(gallivm, "   blocksize = ", bsize);
-         lp_build_printf(gallivm, "   instance_id = %u\n", 
system_values.instance_id);
-         lp_build_printf(gallivm, "   buffer size = %u\n", buffer_size);
+         if (0) {
+            lp_build_printf(gallivm, "vbuf index = %u, vb_stride is %u\n",
+                            vb_index, vb_stride[j]);
+            lp_build_printf(gallivm, "   vb_buffer_offset = %u, src_offset is 
%u\n",
+                            vb_buffer_offset, src_offset);
+            lp_build_print_value(gallivm, "   blocksize = ", bsize);
+            lp_build_printf(gallivm, "   instance_id = %u\n",
+                            system_values.instance_id);
+            lp_build_printf(gallivm, "   buffer size = %u\n", buffer_size);
+         }
       }
    }

@@ -1782,7 +1785,7 @@ draw_llvm_generate(struct draw_llvm *llvm, struct 
draw_llvm_variant *variant,
       }

       for (j = 0; j < key->nr_vertex_elements; ++j) {
-         struct pipe_vertex_element *velem = &draw->pt.vertex_element[j];
+         struct pipe_vertex_element *velem = &key->vertex_element[j];
          const struct util_format_description *format_desc =
             util_format_description(velem->src_format);


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev