intel/fs/gfx12: Ensure that prior reads have executed before barrier with acquire...
authorFrancisco Jerez <currojerez@riseup.net>
Mon, 9 Jan 2023 23:31:33 +0000 (15:31 -0800)
committerEric Engestrom <eric@engestrom.ch>
Thu, 26 Jan 2023 15:40:31 +0000 (15:40 +0000)
commit7fe1d202d52c478094a04ec61a8f0b47da59db66
tree77bbd2e15a40d04a624420db14e8caf4a31f509b
parent9098d83fb3391c3c4d80bad241d86d5f3bfadb04
intel/fs/gfx12: Ensure that prior reads have executed before barrier with acquire semantics.

This avoids a violation of the Vulkan memory model that was leading to
intermittent failures of at least 8k test-cases of the Vulkan CTS
(within the group dEQP-VK.memory_model.*) on TGL and DG2 platforms.
In theory the issue may be reproducible on earlier platforms like IVB
and ICL, but the SYNC.ALLWR instruction is not available on those
platforms so a different (likely costlier) fix will be needed.

The issue occurs within the sequence we emit for a NIR memory barrier
with acquire semantics requiring the synchronization of multiple
caches, e.g. in pseudocode for a barrier involving the TGM and UGM
caches on DG2:

 x <- load.ugm // Atomic read sequenced-before the barrier
 y <- fence.ugm
 z <- fence.tgm
 wait(y, z)
 w <- load.tgm // Read sequenced-after the barrier

In the example we must provide the guarantee that the memory load for
x is completed before the one for w, however this ordering can be
reversed with the intervention of a concurrent thread, since the UGM
fence will block on the prior UGM load and potentially take a long
time, while the TGM fence may complete and invalidate the TGM cache
immediately, so a concurrent thread could pollute the TGM cache with
stale contents for the w location *before* the UGM load has completed,
leading to an inversion of the expected memory ordering.

v2: Apply the workaround regardless of whether the NIR barrier
    intrinsic specifies multiple storage classes or a single one,
    since an acquire barrier is required to order subsequent requests
    relative to previous atomic requests of unknown storage class not
    necessarily specified by the memory scope information of the
    intrinsic.

Cc: mesa-stable
Reviewed-by: Ivan Briano <ivan.briano@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20690>
(cherry picked from commit 4a2e7306dd007a9564f9194c52d181ef24271c4e)
.pick_status.json
src/intel/compiler/brw_fs_nir.cpp