diff mbox

[Bug,82139] New: [r600g, bisected] multiple ubo piglit regressions

Message ID bug-82139-502@http.bugs.freedesktop.org/ (mailing list archive)
State New, archived
Headers show

Commit Message

bugzilla-daemon@freedesktop.org Aug. 4, 2014, 4:29 p.m. UTC
https://bugs.freedesktop.org/show_bug.cgi?id=82139

          Priority: medium
            Bug ID: 82139
          Assignee: dri-devel@lists.freedesktop.org
           Summary: [r600g, bisected] multiple ubo piglit regressions
          Severity: normal
    Classification: Unclassified
                OS: All
          Reporter: andreas.boll.dev@gmail.com
          Hardware: Other
            Status: NEW
           Version: git
         Component: Drivers/Gallium/r600
           Product: Mesa

I noticed multiple ubo piglit regressions on 10.3-devel / r600g / BARTS
since this commit:

commit 01bf8bb87565ed3677e43c6b6deeb90378d88647
Author: Brian Paul <brianp@vmware.com>
Date:   Tue Jul 1 07:57:43 2014 -0600

    st/mesa: don't use address register for constant-indexed ir_binop_ubo_load

    Before, we were always using the address register and indirect addressing
    to index into a UBO constant buffer.  With this change we only do that
    when necessary.

    Using the piglit bin/arb_uniform_buffer_object-rendering test as an
    example:

    Shader code:
      uniform ub_rot {float rotation; };
      ...
      m[1][1] = cos(rotation);

    Before:
      IMM[1] INT32 {0, 1, 0, 0}
      1: UARL ADDR[0].x, IMM[1].xxxx
      2: MOV TEMP[0].x, CONST[3][ADDR[0].x].xxxx
      3: COS TEMP[1].x, TEMP[0].xxxx

    After:
      0: COS TEMP[0].x, CONST[3][0].xxxx

    Reviewed-by: Roland Scheidegger <sroland@vmware.com>

       cbuf.swizzle = swizzle_for_size(ir->type->vector_elements);
@@ -1982,9 +1990,6 @@ glsl_to_tgsi_visitor::visit(ir_expression *ir)
                                     const_offset % 16 / 4,
                                     const_offset % 16 / 4);

-      cbuf.reladdr = ralloc(mem_ctx, st_src_reg);
-      memcpy(cbuf.reladdr, &index_reg, sizeof(index_reg));
-
       if (ir->type->base_type == GLSL_TYPE_BOOL) {
          emit(ir, TGSI_OPCODE_USNE, result_dst, cbuf, st_src_reg_for_int(0));
       } else {


I guess it unhides a driver bug, since softpipe and llvmpipe are not affected.

Regressed tests:

spec/ARB_uniform_buffer_object/bufferstorage
spec/ARB_uniform_buffer_object/maxblocks
spec/ARB_uniform_buffer_object/rendering

spec/glsl-1.40/uniform_buffer/fs-mat4
spec/glsl-1.40/uniform_buffer/fs-two-members
spec/glsl-1.40/uniform_buffer/vs-mat4
spec/glsl-1.40/uniform_buffer/vs-two-members

spec/glsl-1.50/uniform_buffer/gs-mat4
spec/glsl-1.50/uniform_buffer/gs-two-members


Thanks,
Andreas.

Comments

bugzilla-daemon@freedesktop.org Aug. 4, 2014, 4:33 p.m. UTC | #1
https://bugs.freedesktop.org/show_bug.cgi?id=82139

--- Comment #1 from Andreas Boll <andreas.boll.dev@gmail.com> ---
Created attachment 104008
  --> https://bugs.freedesktop.org/attachment.cgi?id=104008&action=edit
arb_uniform_buffer_object-rendering dump before 01bf8bb

R600_DEBUG=fs,vs,gs,ps bin/arb_uniform_buffer_object-rendering -fbo
bugzilla-daemon@freedesktop.org Aug. 4, 2014, 4:34 p.m. UTC | #2
https://bugs.freedesktop.org/show_bug.cgi?id=82139

--- Comment #2 from Andreas Boll <andreas.boll.dev@gmail.com> ---
Created attachment 104009
  --> https://bugs.freedesktop.org/attachment.cgi?id=104009&action=edit
arb_uniform_buffer_object-rendering dump after 01bf8bb

R600_DEBUG=fs,vs,gs,ps bin/arb_uniform_buffer_object-rendering -fbo
bugzilla-daemon@freedesktop.org Aug. 14, 2014, 6:47 p.m. UTC | #3
https://bugs.freedesktop.org/show_bug.cgi?id=82139

Marek Olšák <maraeo@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #3 from Marek Olšák <maraeo@gmail.com> ---
Fixed by da9c3ed304be5d08ff989d61c6e2d1be8a845767. Closing.
diff mbox

Patch

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index d660a6b..69d67e1 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -1971,9 +1971,17 @@  glsl_to_tgsi_visitor::visit(ir_expression *ir)
       assert(ir->type->is_vector() || ir->type->is_scalar());

       if (const_offset_ir) {
-         index_reg = st_src_reg_for_int(const_offset / 16);
-      } else {
-         emit(ir, TGSI_OPCODE_USHR, st_dst_reg(index_reg), op[1],
st_src_reg_for_int(4));
+         /* Constant index into constant buffer */
+         cbuf.reladdr = NULL;
+         cbuf.index = const_offset / 16;
+         cbuf.has_index2 = true;
+      }
+      else {
+         /* Relative/variable index into constant buffer */
+         emit(ir, TGSI_OPCODE_USHR, st_dst_reg(index_reg), op[1],
+              st_src_reg_for_int(4));
+         cbuf.reladdr = ralloc(mem_ctx, st_src_reg);
+         memcpy(cbuf.reladdr, &index_reg, sizeof(index_reg));
       }