From 905d914cb694b90dddd5206eed436d1aae108e8b Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 5 Jun 2019 18:51:16 +0000 Subject: [PATCH] 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 --- 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 + * 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.30.2