On 13/02/18 17:57, srol...@vmware.com wrote:
From: Roland Scheidegger <srol...@vmware.com>

Shaders coming from dx10 state trackers have a RET before the END.
And the epilog needs to be placed before the RET (otherwise it will
get ignored).
Hence figure out if a RET is in main, in this case we'll place
the epilog there rather than before the END.
(At a closer look, there actually seem to be problems with control
flow in general with output redirection, that would need another
look. It's enough however to fix draw's aa line emulation in some
internal bug - lines tend to be drawn with trivial shaders, moving
either a constant color or a vertex color directly to the output).

v2: add assert so buggy handling of RET in main is detected
---
  src/gallium/auxiliary/tgsi/tgsi_transform.c | 62 +++++++++++++++++++++++++----
  1 file changed, 55 insertions(+), 7 deletions(-)

diff --git a/src/gallium/auxiliary/tgsi/tgsi_transform.c 
b/src/gallium/auxiliary/tgsi/tgsi_transform.c
index ffdad13..a13cf90 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_transform.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_transform.c
@@ -110,6 +110,9 @@ tgsi_transform_shader(const struct tgsi_token *tokens_in,
  {
     uint procType;
     boolean first_instruction = TRUE;
+   boolean epilog_emitted = FALSE;
+   int cond_stack = 0;
+   int call_stack = 0;
/* input shader */
     struct tgsi_parse_context parse;
@@ -166,22 +169,66 @@ tgsi_transform_shader(const struct tgsi_token *tokens_in,
           {
              struct tgsi_full_instruction *fullinst
                 = &parse.FullToken.FullInstruction;
+            unsigned opcode = fullinst->Instruction.Opcode;
if (first_instruction && ctx->prolog) {
                 ctx->prolog(ctx);
              }
- /* XXX Note: we may also want to look for a main/top-level
-             * TGSI_OPCODE_RET instruction in the future.
+            /*
+             * XXX Note: we handle the case of ret in main.
+             * However, the output redirections done by transform
+             * have their limits with control flow and will generally
+             * not work correctly. e.g.
+             * if (cond) {
+             *    oColor = x;
+             *    ret;
+             * }
+             * oColor = y;
+             * end;
+             * If the color output is redirected to a temp and modified
+             * by a transform, this will not work (the oColor assignment
+             * in the conditional will never make it to the actual output).
               */
-            if (fullinst->Instruction.Opcode == TGSI_OPCODE_END
-                && ctx->epilog) {
-               /* Emit caller's epilog */
-               ctx->epilog(ctx);
-               /* Emit END */
+            if ((opcode == TGSI_OPCODE_END || opcode == TGSI_OPCODE_RET) &&
+                 call_stack == 0 && ctx->epilog && !epilog_emitted) {
+               if (opcode == TGSI_OPCODE_RET && cond_stack != 0) {
+                  assert(!"transform ignoring RET in main");
+               } else {
+                  assert(cond_stack == 0);
+                  /* Emit caller's epilog */
+                  ctx->epilog(ctx);
+                  epilog_emitted = TRUE;
+               }
+               /* Emit END (or RET) */
                 ctx->emit_instruction(ctx, fullinst);
              }
              else {
+               switch (opcode) {
+               case TGSI_OPCODE_IF:
+               case TGSI_OPCODE_UIF:
+               case TGSI_OPCODE_SWITCH:
+               case TGSI_OPCODE_BGNLOOP:
+                  cond_stack++;
+                  break;
+               case TGSI_OPCODE_CAL:
+                  call_stack++;
+                  break;
+               case TGSI_OPCODE_ENDIF:
+               case TGSI_OPCODE_ENDSWITCH:
+               case TGSI_OPCODE_ENDLOOP:
+                  assert(cond_stack > 0);
+                  cond_stack--;
+                  break;
+               case TGSI_OPCODE_ENDSUB:
+                  assert(call_stack > 0);
+                  call_stack--;
+                  break;
+               case TGSI_OPCODE_BGNSUB:
+               case TGSI_OPCODE_RET:
+               default:
+                  break;
+               }
                 if (ctx->transform_instruction)
                    ctx->transform_instruction(ctx, fullinst);
                 else
@@ -231,6 +278,7 @@ tgsi_transform_shader(const struct tgsi_token *tokens_in,
           assert( 0 );
        }
     }
+   assert(call_stack == 0);
tgsi_parse_free (&parse);

LGTM.  Thanks.

Reviewed-by: Jose Fonseca <jfons...@vmware.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to