[Mesa-dev] [PATCH 1/2] draw: fix undefined input handling some more...
Jose Fonseca
jfonseca at vmware.com
Thu Nov 3 17:44:33 UTC 2016
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" <nhaehnle at gmail.com<mailto:nhaehnle at gmail.com>> wrote:
On 03.11.2016 02:15, sroland at vmware.com wrote:
> From: Roland Scheidegger <sroland at 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 at 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);
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161103/893959f9/attachment-0001.html>
More information about the mesa-dev
mailing list