diff options
| author | Samuel Pitoiset <samuel.pitoiset@gmail.com> | 2020-02-07 16:33:35 +0100 |
|---|---|---|
| committer | Dylan Baker <dylan@pnwbakers.com> | 2020-02-10 09:05:28 -0800 |
| commit | a3bd400c1417c7a3fff8bad55297eaae5924568c (patch) | |
| tree | 89ed5efb0e55955fcd2d03c7709712e6a281dfbc | |
| parent | 32dc7fff47f2c0c88165b4cb24d9418eeafd7cea (diff) | |
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@lists.freedesktop.org>
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Daniel Schürmann <daniel@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)
| -rw-r--r-- | .pick_status.json | 2 | ||||
| -rw-r--r-- | 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. |