[Mesa-dev] [PATCH] panfrost/midgard: Verify SSA claims when pipelining

[Alyssa Rosenzweig]

The pipeline register creation algorithm is only valid for SSA indices;
NIR registers and such cannot be pipelined without more complex
analysis. However, there are the ocassional class of "liars" -- indices
that claim to be SSA but are not. This occurs in the blend shader
prologue, for example. Detect this and just bail quietly for now.

Eventually we need to rewrite the blend shader prologue to occur in NIR
anyway (which would mitigate the issue), but that's more involved and
depends on a better understanding of pixel formats in blend shaders (for
non-RGBA8888/UNORM cases).

Fixes some blend shader regressions.

Signed-off-by: Alyssa Rosenzweig <alyssa.rosenzweig at collabora.com>
---
 src/gallium/drivers/panfrost/midgard/compiler.h  |  1 +
 .../drivers/panfrost/midgard/midgard_liveness.c  | 16 ++++++++++++++++
 .../panfrost/midgard/midgard_ra_pipeline.c       |  7 +++++++
 3 files changed, 24 insertions(+)

diff --git a/src/gallium/drivers/panfrost/midgard/compiler.h b/src/gallium/drivers/panfrost/midgard/compiler.h
index 1191c5cc7b8..e15afca688e 100644
--- a/src/gallium/drivers/panfrost/midgard/compiler.h
+++ b/src/gallium/drivers/panfrost/midgard/compiler.h
@@ -429,6 +429,7 @@ struct ra_graph;
 struct ra_graph* allocate_registers(compiler_context *ctx);
 void install_registers(compiler_context *ctx, struct ra_graph *g);
 bool mir_is_live_after(compiler_context *ctx, midgard_block *block, midgard_instruction *start, int src);
+bool mir_has_multiple_writes(compiler_context *ctx, int src);
 
 void mir_create_pipeline_registers(compiler_context *ctx);
 
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_liveness.c b/src/gallium/drivers/panfrost/midgard/midgard_liveness.c
index e4c89556162..a18d8b9f8ad 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_liveness.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_liveness.c
@@ -96,3 +96,19 @@ mir_is_live_after(compiler_context *ctx, midgard_block *block, midgard_instructi
 
         return succ;
 }
+
+/* Just a quick check -- is it written more than once? (I.e. are we definitely
+ * not SSA?) */
+
+bool
+mir_has_multiple_writes(compiler_context *ctx, int dest)
+{
+        unsigned write_count = 0;
+
+        mir_foreach_instr_global(ctx, ins) {
+                if (ins->ssa_args.dest == dest)
+                        write_count++;
+        }
+
+        return write_count > 1;
+}
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_ra_pipeline.c b/src/gallium/drivers/panfrost/midgard/midgard_ra_pipeline.c
index 07952b63ffc..d4187edc0c8 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_ra_pipeline.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_ra_pipeline.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2019 Alyssa Rosenzweig <alyssa at rosenzweig.io>
+ * Copyright (C) 2019 Collabora
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
@@ -57,6 +58,12 @@ mir_pipeline_ins(
         if ((dest < 0) || (dest >= ctx->func->impl->ssa_alloc))
                 return false;
 
+        /* Make sure they're not lying to us. Blend shaders lie. TODO: Fix your
+         * bad code Alyssa */
+
+        if (mir_has_multiple_writes(ctx, dest))
+                return false;
+
         /* We want to know if we live after this bundle, so check if
          * we're live after the last instruction of the bundle */
 
-- 
2.20.1