panfrost/midgard: Verify SSA claims when pipelining

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@collabora.com>

diff --git a/src/gallium/drivers/panfrost/midgard/compiler.h b/src/gallium/drivers/panfrost/midgard/compiler.h
index 1191c5cc7b856a9abcbc613004f48ba79d8f8f3a..e15afca688e1c5798eaf2089f627399188633110 100644 (file)
--- 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 e4c89556162b3a010004889b36907afcb9c92e7a..a18d8b9f8ad45a34b7f00c6e711f4ff54ae2d216 100644 (file)
--- 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 07952b63ffc526271aafba09c0b6f4620c1ca3e5..d4187edc0c87a3b8aae08147ee3704b5607d7bd1 100644 (file)
--- 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@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 */