Mesa (staging/21.3): intel/fs: Add physical fall-through CFG edge for unconditional BREAK instruction.

[GitLab Mirror]

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

Author: Francisco Jerez <currojerez at riseup.net>
Date:   Tue Dec 14 13:40:49 2021 -0800

intel/fs: Add physical fall-through CFG edge for unconditional BREAK instruction.

This adds a missing CFG edge that represents a possible physical
control flow path the EU might take under some conditions which isn't
part of the logical CFG of the program.  This possibility shouldn't
have led to problems on platforms prior to Gfx12, since the missing
control flow edge cannot possibly influence liveness intervals.
However on Gfx12+ it becomes the compiler's responsibility to resolve
data dependencies across instructions, and the missing physical
control flow paths may lead to a WaR data hazard currently not visible
to the software scoreboard pass, which could lead to data corruption.

Worse, the possibility for this path to be taken by the EU increases
on Gfx12+ due to a hardware bug affecting EU fusion -- However the
same physical path can be potentially taken on earlier platforms as
well, so this patch extends the CFG on all platforms for consistency,
even though the lack of this edge shouldn't lead to any functional
issues on platforms earlier than Gfx12.  There are no shader-db
changes on earlier platforms, so there seems to be no disadvantage
from using the same CFG representation as on later platforms.

This issue has ben reported on TGL with the following conformance
test, thanks to Ian for bringing the FULSIM dependency check warning
to my attention:

   dEQP-VK.graphicsfuzz.spv-stable-pillars-volatile-nontemporal-store

Fixes: 4d1959e69328cf ("intel/cfg: Represent divergent control flow paths caused by non-uniform loop execution.")
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/4940
Reported-by: Tapani Pälli <tapani.palli at intel.com>
Reported-by: Ian Romanick <ian.d.romanick at intel.com>
Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/14248>
(cherry picked from commit e7470a40c569025b18d4d6767aa21caaa862a5b5)

---

 .pick_status.json              | 2 +-
 src/intel/compiler/brw_cfg.cpp | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/.pick_status.json b/.pick_status.json
index 3e403257e01..4da91a926b6 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -499,7 +499,7 @@
         "description": "intel/fs: Add physical fall-through CFG edge for unconditional BREAK instruction.",
         "nominated": true,
         "nomination_type": 1,
-        "resolution": 0,
+        "resolution": 1,
         "main_sha": null,
         "because_sha": "4d1959e69328cf0d59f0ec7aeea5a2b704ef0c5f"
     },
diff --git a/src/intel/compiler/brw_cfg.cpp b/src/intel/compiler/brw_cfg.cpp
index 6cbbaaf53c2..4ee8cc82320 100644
--- a/src/intel/compiler/brw_cfg.cpp
+++ b/src/intel/compiler/brw_cfg.cpp
@@ -364,6 +364,8 @@ cfg_t::cfg_t(const backend_shader *s, exec_list *instructions) :
 	 next = new_block();
 	 if (inst->predicate)
             cur->add_successor(mem_ctx, next, bblock_link_logical);
+         else
+            cur->add_successor(mem_ctx, next, bblock_link_physical);
 
 	 set_next_block(&cur, next, ip);
 	 break;