Hi Ivan,

On 25.10.2015 02:00, Ivan Kalvachev wrote:
Some constants (like 1.0 and 0.5) could be inlined as immediate inputs
without using their literal value. The r600_bytecode_special_constants()
function emulates the negative of these constants by using NEG modifier.

However some shaders define -1.0 constant and want to use it as 1.0.
They do so by using ABS modifier. But r600_bytecode_special_constants()
set NEG in addition to ABS. Since NEG modifier have priority over ABS one,
we get -|1.0| as result, instead of |1.0|.

The patch simply prevents the additional switching of NEG when ABS is set.

Nice catch. Is there a simple test case (e.g. in piglit) that exposes the incorrect behavior?

Signed-off-by: Ivan Kalvachev <ikalvac...@gmail.com>
---
  src/gallium/drivers/r600/r600_asm.c    | 9 +++++----
  src/gallium/drivers/r600/r600_shader.c | 2 +-
  2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/gallium/drivers/r600/r600_asm.c
b/src/gallium/drivers/r600/r600_asm.c
index bc69806..8fc622c 100644
--- a/src/gallium/drivers/r600/r600_asm.c
+++ b/src/gallium/drivers/r600/r600_asm.c
@@ -635,8 +635,9 @@ static int replace_gpr_with_pv_ps(struct r600_bytecode *bc,
         return 0;
  }

-void r600_bytecode_special_constants(uint32_t value, unsigned *sel,
unsigned *neg)
+void r600_bytecode_special_constants(uint32_t value, unsigned *sel,
unsigned *neg, unsigned abs)
  {
+

Please remove the extra whitespace line.

Cheers,
Nicolai

         switch(value) {
         case 0:
                 *sel = V_SQ_ALU_SRC_0;
@@ -655,11 +656,11 @@ void r600_bytecode_special_constants(uint32_t
value, unsigned *sel, unsigned *ne
                 break;
         case 0xBF800000: /* -1.0f */
                 *sel = V_SQ_ALU_SRC_1;
-               *neg ^= 1;
+               *neg ^= !abs;
                 break;
         case 0xBF000000: /* -0.5f */
                 *sel = V_SQ_ALU_SRC_0_5;
-               *neg ^= 1;
+               *neg ^= !abs;
                 break;
         default:
                 *sel = V_SQ_ALU_SRC_LITERAL;
@@ -1208,7 +1209,7 @@ int r600_bytecode_add_alu_type(struct r600_bytecode *bc,
                 }
                 if (nalu->src[i].sel == V_SQ_ALU_SRC_LITERAL)
                         r600_bytecode_special_constants(nalu->src[i].value,
-                               &nalu->src[i].sel, &nalu->src[i].neg);
+                               &nalu->src[i].sel, &nalu->src[i].neg,
nalu->src[i].abs);
         }
         if (nalu->dst.sel >= bc->ngpr) {
                 bc->ngpr = nalu->dst.sel + 1;
diff --git a/src/gallium/drivers/r600/r600_shader.c
b/src/gallium/drivers/r600/r600_shader.c
index 8efe902..50c0329 100644
--- a/src/gallium/drivers/r600/r600_shader.c
+++ b/src/gallium/drivers/r600/r600_shader.c
@@ -1008,7 +1008,7 @@ static void tgsi_src(struct r600_shader_ctx *ctx,
                         (tgsi_src->Register.SwizzleX ==
tgsi_src->Register.SwizzleW)) {

                         index = tgsi_src->Register.Index * 4 +
tgsi_src->Register.SwizzleX;
-
r600_bytecode_special_constants(ctx->literals[index], &r600_src->sel,
&r600_src->neg);
+
r600_bytecode_special_constants(ctx->literals[index], &r600_src->sel,
&r600_src->neg, r600_src->abs);
                         if (r600_src->sel != V_SQ_ALU_SRC_LITERAL)
                                 return;
                 }



_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to