<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <meta name="Generator" content="Microsoft Exchange Server"> <style></style> </head> <body> <div> 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é <div class="x_gmail_quote">On Thu, Nov 3, 2016 at 10:48 AM +0000, "Nicolai Hähnle" <<a href="mailto:nhaehnle@gmail.com" target="_blank">nhaehnle@gmail.com</a>> wrote: </div> </div> <div class="PlainText">On 03.11.2016 02:15, sroland@vmware.com wrote: > From: Roland Scheidegger <sroland@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.haehnle@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); > > </div> </body> </html>