Mesa (staging/20.0): aco: fix waiting for scalar stores before "writing back" data on GFX8-GFX9

[GitLab Mirror]

Module: Mesa
Branch: staging/20.0
Commit: a3bd400c1417c7a3fff8bad55297eaae5924568c
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=a3bd400c1417c7a3fff8bad55297eaae5924568c

Author: Samuel Pitoiset <samuel.pitoiset at gmail.com>
Date:   Fri Feb  7 16:33:35 2020 +0100

aco: fix waiting for scalar stores before "writing back" data on GFX8-GFX9

Seems required also on GFX8-GFX9 to achieve correct behaviour. This
is an undocumented behaviour but it makes real sense to me.

pipeline-db on GFX9:
Totals from affected shaders:
SGPRS: 1018 -> 1018 (0.00 %)
VGPRS: 516 -> 516 (0.00 %)
Code Size: 40516 -> 40636 (0.30 %) bytes
Max Waves: 280 -> 280 (0.00 %)

This fixes some sort of sun flickering with Assassins Creed Origins.

Closes: https://gitlab.freedesktop.org/mesa/mesa/issues/2488
Cc: <mesa-stable at lists.freedesktop.org>
Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
Reviewed-by: Daniel Schürmann <daniel at schuermann.dev>
Tested-by: Marge Bot <https://gitlab.freedesktop.org/mesa/mesa/merge_requests/3750>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/merge_requests/3750>
(cherry picked from commit 34fd894e42ae1ec9d35bf9c4f05364b03dd4a223)

---

 .pick_status.json                       |  2 +-
 src/amd/compiler/aco_insert_waitcnt.cpp | 15 +++++++++------
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/.pick_status.json b/.pick_status.json
index 3269c5c5ba3..19b5b65ff5b 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -13,7 +13,7 @@
         "description": "aco: fix waiting for scalar stores before \"writing back\" data on GFX8-GFX9",
         "nominated": true,
         "nomination_type": 0,
-        "resolution": 0,
+        "resolution": 1,
         "master_sha": null,
         "because_sha": null
     },
diff --git a/src/amd/compiler/aco_insert_waitcnt.cpp b/src/amd/compiler/aco_insert_waitcnt.cpp
index 5ec9636752d..8d8024f5aa2 100644
--- a/src/amd/compiler/aco_insert_waitcnt.cpp
+++ b/src/amd/compiler/aco_insert_waitcnt.cpp
@@ -374,13 +374,16 @@ wait_imm kill(Instruction* instr, wait_ctx& ctx)
 
    imm.combine(parse_wait_instr(ctx, instr));
 
-   if (ctx.chip_class >= GFX10) {
-      /* Seems to be required on GFX10 to achieve correct behaviour.
-       * It shouldn't cost anything anyways since we're about to do s_endpgm.
-       */
-      if (ctx.lgkm_cnt && instr->opcode == aco_opcode::s_dcache_wb)
-         imm.lgkm = 0;
 
+   /* It's required to wait for scalar stores before "writing back" data.
+    * It shouldn't cost anything anyways since we're about to do s_endpgm.
+    */
+   if (ctx.lgkm_cnt && instr->opcode == aco_opcode::s_dcache_wb) {
+      assert(ctx.chip_class >= GFX8);
+      imm.lgkm = 0;
+   }
+
+   if (ctx.chip_class >= GFX10) {
       /* GFX10: A store followed by a load at the same address causes a problem because
        * the load doesn't load the correct values unless we wait for the store first.
        * This is NOT mitigated by an s_nop.