Patch 1 looks good to me. Yes, not using compilation key is a recipe for problems.
Regarding patch 2, I need a bit more time to review. I'd also like the new lp_build_umul... function to be added to lp_bld_arit: there is a single caller note, but that might change in the future, so might as well place it in its natural place. José On Thu, Nov 3, 2016 at 10:48 AM +0000, "Nicolai Hähnle" <nhaeh...@gmail.com<mailto:nhaeh...@gmail.com>> wrote: 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
