Mesa (master): llvmpipe/draw: handle constant buffer limits and robustness (v1.1)

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Thu Jul 23 00:23:34 UTC 2020


Module: Mesa
Branch: master
Commit: 12c06a0deb632728f76ceb480e86151b8f96100e
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=12c06a0deb632728f76ceb480e86151b8f96100e

Author: Dave Airlie <airlied at redhat.com>
Date:   Mon Dec 30 09:59:28 2019 +1000

llvmpipe/draw: handle constant buffer limits and robustness (v1.1)

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 at vmware.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/5971>

---

 src/gallium/auxiliary/draw/draw_context.c          |  5 +++++
 src/gallium/auxiliary/draw/draw_context.h          |  3 +++
 src/gallium/auxiliary/draw/draw_private.h          |  2 +-
 .../draw/draw_pt_fetch_shade_pipeline_llvm.c       | 22 ++++++++++++++--------
 src/gallium/auxiliary/gallivm/lp_bld_nir_soa.c     |  2 --
 src/gallium/drivers/llvmpipe/lp_context.c          |  4 ++++
 src/gallium/drivers/llvmpipe/lp_screen.h           |  6 +++++-
 src/gallium/drivers/llvmpipe/lp_setup.c            |  4 ++--
 src/gallium/drivers/llvmpipe/lp_state_cs.c         | 12 +++++++-----
 9 files changed, 41 insertions(+), 19 deletions(-)

diff --git a/src/gallium/auxiliary/draw/draw_context.c b/src/gallium/auxiliary/draw/draw_context.c
index 781f8f124b6..ac8c812707c 100644
--- a/src/gallium/auxiliary/draw/draw_context.c
+++ b/src/gallium/auxiliary/draw/draw_context.c
@@ -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;
 }
diff --git a/src/gallium/auxiliary/draw/draw_context.h b/src/gallium/auxiliary/draw/draw_context.h
index 4dc542db095..0098657357b 100644
--- a/src/gallium/auxiliary/draw/draw_context.h
+++ b/src/gallium/auxiliary/draw/draw_context.h
@@ -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);
 
diff --git a/src/gallium/auxiliary/draw/draw_private.h b/src/gallium/auxiliary/draw/draw_private.h
index 864d80fecb7..05969faab73 100644
--- a/src/gallium/auxiliary/draw/draw_private.h
+++ b/src/gallium/auxiliary/draw/draw_private.h
@@ -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.
diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c
index e198f8f92d8..c4b79ea0e24 100644
--- a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c
+++ b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c
@@ -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) {
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_nir_soa.c b/src/gallium/auxiliary/gallivm/lp_bld_nir_soa.c
index 3134f959e39..b07e556ecc9 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_nir_soa.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_nir_soa.c
@@ -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);
       }
    }
diff --git a/src/gallium/drivers/llvmpipe/lp_context.c b/src/gallium/drivers/llvmpipe/lp_context.c
index 58073f3a7f6..05dc33ef4e2 100644
--- a/src/gallium/drivers/llvmpipe/lp_context.c
+++ b/src/gallium/drivers/llvmpipe/lp_context.c
@@ -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,
diff --git a/src/gallium/drivers/llvmpipe/lp_screen.h b/src/gallium/drivers/llvmpipe/lp_screen.h
index 88f87f9dcbd..6b3798e4d7a 100644
--- a/src/gallium/drivers/llvmpipe/lp_screen.h
+++ b/src/gallium/drivers/llvmpipe/lp_screen.h
@@ -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 */
diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c b/src/gallium/drivers/llvmpipe/lp_setup.c
index a3e2a7ed7e4..550062ffcfc 100644
--- a/src/gallium/drivers/llvmpipe/lp_setup.c
+++ b/src/gallium/drivers/llvmpipe/lp_setup.c
@@ -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;
       }
diff --git a/src/gallium/drivers/llvmpipe/lp_state_cs.c b/src/gallium/drivers/llvmpipe/lp_state_cs.c
index dcdd9b36240..be6d648cd53 100644
--- a/src/gallium/drivers/llvmpipe/lp_state_cs.c
+++ b/src/gallium/drivers/llvmpipe/lp_state_cs.c
@@ -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;
       }
    }



More information about the mesa-commit mailing list