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 781f8f124b62bb6119d195b276022f9202f2dba8..ac8c812707cc450b07c3cbb988b1053f7e11bc6b 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 4dc542db0954d9905eac413877e710afdb96da07..0098657357bfa8cc4e4c208625023ffdfec07927 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 864d80fecb79425a7569c8ff9fac8be2f09c6252..05969faab73286bfb10fbb369e0d0cf2913ce077 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 e198f8f92d8c153246bfac296786c04c3daa7a06..c4b79ea0e247b11040a5d20bf5abb1535eafb59c 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 3134f959e39d66401ea9c2fec6c396859f6194d1..b07e556ecc91d7559d9a219b77c3745f1251b064 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 58073f3a7f6f982decfcf2e2f6d77c28afb259c4..05dc33ef4e282c10ae59c8e171f63bf54a33ac35 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 88f87f9dcbd0446fb8b5aba0f20d916a34b779f7..6b3798e4d7a408dc5ac355a4467e5342515e8d03 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 a3e2a7ed7e4860d6fdd9f2aa60266acb619efd1c..550062ffcfce3664057f7654480f518459d8b086 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 dcdd9b3624086a8c1f21e699fc293469b26dcfe4..be6d648cd537a830259a5aadc54343812d5af96c 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;
       }
    }