llvmpipe/draw: handle constant buffer limits and robustness (v1.1)
authorDave Airlie <airlied@redhat.com>
Sun, 29 Dec 2019 23:59:28 +0000 (09:59 +1000)
committerMarge Bot <eric+marge@anholt.net>
Thu, 23 Jul 2020 00:04:49 +0000 (00:04 +0000)
TGSI expect vec4 of constants for it's current code paths, and when
doing indirect accesses it does the comparison on vec4 indexes,
however NIR does the indexing on packed float indexes.

This also align the compute path with the other shaders, and
should improve robustness (at least under Vulkan)

Fixes:
KHR-NoContext.gl43.robust_buffer_access_behavior.uniform_buffer

v1.1:
rename variable to something more meaningful (Roland)

Reviewed-by: Roland Scheidegger <sroland@vmware.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/5971>

src/gallium/auxiliary/draw/draw_context.c
src/gallium/auxiliary/draw/draw_context.h
src/gallium/auxiliary/draw/draw_private.h
src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c
src/gallium/auxiliary/gallivm/lp_bld_nir_soa.c
src/gallium/drivers/llvmpipe/lp_context.c
src/gallium/drivers/llvmpipe/lp_screen.h
src/gallium/drivers/llvmpipe/lp_setup.c
src/gallium/drivers/llvmpipe/lp_state_cs.c

index 781f8f1..ac8c812 100644 (file)
@@ -95,6 +95,7 @@ draw_create_context(struct pipe_context *pipe, void *context,
 #endif
 
    draw->pipe = pipe;
+   draw->constant_buffer_stride = (sizeof(float) * 4);
 
    if (!draw_init(draw))
       goto err_destroy;
@@ -1350,5 +1351,9 @@ draw_set_disk_cache_callbacks(struct draw_context *draw,
    draw->disk_cache_find_shader = find_shader;
    draw->disk_cache_insert_shader = insert_shader;
    draw->disk_cache_cookie = data_cookie;
+}
 
+void draw_set_constant_buffer_stride(struct draw_context *draw, unsigned num_bytes)
+{
+   draw->constant_buffer_stride = num_bytes;
 }
index 4dc542d..0098657 100644 (file)
@@ -122,6 +122,9 @@ void draw_enable_point_sprites(struct draw_context *draw, boolean enable);
 
 void draw_set_zs_format(struct draw_context *draw, enum pipe_format format);
 
+/* for TGSI constants are 4 * sizeof(float), but for NIR they need to be sizeof(float); */
+void draw_set_constant_buffer_stride(struct draw_context *draw, unsigned num_bytes);
+
 boolean
 draw_install_aaline_stage(struct draw_context *draw, struct pipe_context *pipe);
 
index 864d80f..05969fa 100644 (file)
@@ -367,7 +367,7 @@ struct draw_context
    unsigned instance_id;
    unsigned start_instance;
    unsigned start_index;
-
+   unsigned constant_buffer_stride;
    struct draw_llvm *llvm;
 
    /** Texture sampler and sampler view state.
index e198f8f..c4b79ea 100644 (file)
@@ -421,6 +421,16 @@ llvm_middle_end_prepare( struct draw_pt_middle_end *middle,
    }
 }
 
+static unsigned
+get_num_consts_robust(struct draw_context *draw, unsigned *sizes, unsigned idx)
+{
+   unsigned const_bytes = sizes[idx];
+
+   if (const_bytes < sizeof(float))
+      return 0;
+
+   return DIV_ROUND_UP(const_bytes, draw->constant_buffer_stride);
+}
 
 /**
  * Bind/update constant buffer pointers, clip planes and viewport dims.
@@ -443,8 +453,7 @@ llvm_middle_end_bind_parameters(struct draw_pt_middle_end *middle)
        * shader expects 16-byte allocations, the fix is likely to move
        * to LOAD intrinsic in the future and remove the vec4 constraint.
        */
-      int num_consts =
-         DIV_ROUND_UP(draw->pt.user.vs_constants_size[i], (sizeof(float) * 4));
+      int num_consts = get_num_consts_robust(draw, draw->pt.user.vs_constants_size, i);
       llvm->jit_context.vs_constants[i] = draw->pt.user.vs_constants[i];
       llvm->jit_context.num_vs_constants[i] = num_consts;
       if (num_consts == 0) {
@@ -461,8 +470,7 @@ llvm_middle_end_bind_parameters(struct draw_pt_middle_end *middle)
    }
 
    for (i = 0; i < ARRAY_SIZE(llvm->gs_jit_context.constants); ++i) {
-      int num_consts =
-         DIV_ROUND_UP(draw->pt.user.gs_constants_size[i], (sizeof(float) * 4));
+      int num_consts = get_num_consts_robust(draw, draw->pt.user.gs_constants_size, i);
       llvm->gs_jit_context.constants[i] = draw->pt.user.gs_constants[i];
       llvm->gs_jit_context.num_constants[i] = num_consts;
       if (num_consts == 0) {
@@ -479,8 +487,7 @@ llvm_middle_end_bind_parameters(struct draw_pt_middle_end *middle)
    }
 
    for (i = 0; i < ARRAY_SIZE(llvm->tcs_jit_context.constants); ++i) {
-      int num_consts =
-         DIV_ROUND_UP(draw->pt.user.tcs_constants_size[i], (sizeof(float) * 4));
+      int num_consts = get_num_consts_robust(draw, draw->pt.user.tcs_constants_size, i);
       llvm->tcs_jit_context.constants[i] = draw->pt.user.tcs_constants[i];
       llvm->tcs_jit_context.num_constants[i] = num_consts;
       if (num_consts == 0) {
@@ -497,8 +504,7 @@ llvm_middle_end_bind_parameters(struct draw_pt_middle_end *middle)
    }
 
    for (i = 0; i < ARRAY_SIZE(llvm->tes_jit_context.constants); ++i) {
-      int num_consts =
-         DIV_ROUND_UP(draw->pt.user.tes_constants_size[i], (sizeof(float) * 4));
+      int num_consts = get_num_consts_robust(draw, draw->pt.user.tes_constants_size, i);
       llvm->tes_jit_context.constants[i] = draw->pt.user.tes_constants[i];
       llvm->tes_jit_context.num_constants[i] = num_consts;
       if (num_consts == 0) {
index 3134f95..b07e556 100644 (file)
@@ -929,13 +929,11 @@ static void emit_load_ubo(struct lp_build_nir_context *bld_base,
       LLVMValueRef overflow_mask;
       LLVMValueRef num_consts = lp_build_array_get(gallivm, bld->const_sizes_ptr, index);
 
-      num_consts = LLVMBuildShl(gallivm->builder, num_consts, lp_build_const_int32(gallivm, 4), "");
       num_consts = lp_build_broadcast_scalar(uint_bld, num_consts);
       for (unsigned c = 0; c < nc; c++) {
          LLVMValueRef this_offset = lp_build_add(uint_bld, offset, lp_build_const_int_vec(gallivm, uint_bld->type, c));
          overflow_mask = lp_build_compare(gallivm, uint_bld->type, PIPE_FUNC_GEQUAL,
                                           this_offset, num_consts);
-
          result[c] = build_gather(bld_base, bld_broad, consts_ptr, this_offset, overflow_mask, NULL);
       }
    }
index 58073f3..05dc33e 100644 (file)
@@ -47,6 +47,7 @@
 #include "lp_query.h"
 #include "lp_setup.h"
 #include "lp_screen.h"
+
 /* This is only safe if there's just one concurrent context */
 #ifdef EMBEDDED_DEVICE
 #define USE_GLOBAL_LLVM_CONTEXT
@@ -222,6 +223,9 @@ llvmpipe_create_context(struct pipe_screen *screen, void *priv,
                                  llvmpipe_screen(screen),
                                  lp_draw_disk_cache_find_shader,
                                  lp_draw_disk_cache_insert_shader);
+
+   draw_set_constant_buffer_stride(llvmpipe->draw, lp_get_constant_buffer_stride(screen));
+
    /* FIXME: devise alternative to draw_texture_samplers */
 
    llvmpipe->setup = lp_setup_create( &llvmpipe->pipe,
index 88f87f9..6b3798e 100644 (file)
@@ -82,6 +82,10 @@ llvmpipe_screen( struct pipe_screen *pipe )
    return (struct llvmpipe_screen *)pipe;
 }
 
-
+static inline unsigned lp_get_constant_buffer_stride(struct pipe_screen *_screen)
+{
+   struct llvmpipe_screen *screen = llvmpipe_screen(_screen);
+   return screen->use_tgsi ? (sizeof(float) * 4) : sizeof(float);
+}
 
 #endif /* LP_SCREEN_H */
index a3e2a7e..550062f 100644 (file)
@@ -1201,7 +1201,7 @@ try_update_scene_state( struct lp_setup_context *setup )
             current_data = (ubyte *) setup->constants[i].current.user_buffer;
          }
 
-         if (current_data) {
+         if (current_data && current_size >= sizeof(float)) {
             current_data += setup->constants[i].current.buffer_offset;
 
             /* TODO: copy only the actually used constants? */
@@ -1235,7 +1235,7 @@ try_update_scene_state( struct lp_setup_context *setup )
          }
 
          num_constants =
-            DIV_ROUND_UP(setup->constants[i].stored_size, (sizeof(float) * 4));
+            DIV_ROUND_UP(setup->constants[i].stored_size, lp_get_constant_buffer_stride(scene->pipe->screen));
          setup->fs.current.jit_context.num_constants[i] = num_constants;
          setup->dirty |= LP_SETUP_NEW_FS;
       }
index dcdd9b3..be6d648 100644 (file)
@@ -1161,7 +1161,7 @@ update_csctx_consts(struct llvmpipe_context *llvmpipe)
    for (i = 0; i < ARRAY_SIZE(csctx->constants); ++i) {
       struct pipe_resource *buffer = csctx->constants[i].current.buffer;
       const ubyte *current_data = NULL;
-
+      unsigned current_size = csctx->constants[i].current.buffer_size;
       if (buffer) {
          /* resource buffer */
          current_data = (ubyte *) llvmpipe_resource_data(buffer);
@@ -1171,13 +1171,15 @@ update_csctx_consts(struct llvmpipe_context *llvmpipe)
          current_data = (ubyte *) csctx->constants[i].current.user_buffer;
       }
 
-      if (current_data) {
+      if (current_data && current_size >= sizeof(float)) {
          current_data += csctx->constants[i].current.buffer_offset;
-
          csctx->cs.current.jit_context.constants[i] = (const float *)current_data;
-         csctx->cs.current.jit_context.num_constants[i] = csctx->constants[i].current.buffer_size;
+         csctx->cs.current.jit_context.num_constants[i] =
+            DIV_ROUND_UP(csctx->constants[i].current.buffer_size,
+                         lp_get_constant_buffer_stride(llvmpipe->pipe.screen));
       } else {
-         csctx->cs.current.jit_context.constants[i] = NULL;
+         static const float fake_const_buf[4];
+         csctx->cs.current.jit_context.constants[i] = fake_const_buf;
          csctx->cs.current.jit_context.num_constants[i] = 0;
       }
    }