aco/insert_nops: Consider s_setpc target susceptible to VALUReadSGPRHazard

Some GPU hangs witnessed in the wild on RDNA4 in Control and Arc Raiders
seem to point towards closest-hit shaders reading a stale value for the
SGPR pair containing the currently-executing shader's address.

This SGPR pair was read by VALU in the preceding traversal shader,
making it susceptible to VALUReadSGPRHazard. Inserting
VALUReadSGPRHazard mitigations before accessing the s_setpc target seems
to fix the hang. We don't have conclusive proof that this is hazardous,
but given that all signs point towards it and we have a reasonably
simple workaround, let's roll with this for now to mitigate the hangs.

Cc: mesa-stable
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/38290>

src/amd/compiler/aco_insert_NOPs.cpp

@@ -273,6 +273,8 @@ struct NOP_ctx_gfx11 {
    std::bitset<m0.reg() / 2> sgpr_read_by_valu; /* SGPR pairs, excluding null, exec, m0 and scc */
    std::bitset<m0.reg()> sgpr_read_by_valu_then_wr_by_valu;
    RegCounterMap<11> sgpr_read_by_valu_then_wr_by_salu;
+   /* Force emitting a wait mitigating VALUReadSGPRHazard before the next ALU instruction. */
+   bool force_valu_read_sgpr_wait = false;
 
    void join(const NOP_ctx_gfx11& other)
    {
@@ -293,6 +295,7 @@ struct NOP_ctx_gfx11 {
       sgpr_read_by_valu |= other.sgpr_read_by_valu;
       sgpr_read_by_valu_then_wr_by_valu |= other.sgpr_read_by_valu_then_wr_by_valu;
       sgpr_read_by_valu_then_wr_by_salu.join_min(other.sgpr_read_by_valu_then_wr_by_salu);
+      force_valu_read_sgpr_wait |= other.force_valu_read_sgpr_wait;
    }
 
    bool operator==(const NOP_ctx_gfx11& other) const
@@ -312,7 +315,8 @@ struct NOP_ctx_gfx11 {
                 other.sgpr_read_by_valu_as_lanemask_then_wr_by_valu &&
              vgpr_written_by_wmma == other.vgpr_written_by_wmma &&
              sgpr_read_by_valu == other.sgpr_read_by_valu &&
-             sgpr_read_by_valu_then_wr_by_salu == other.sgpr_read_by_valu_then_wr_by_salu;
+             sgpr_read_by_valu_then_wr_by_salu == other.sgpr_read_by_valu_then_wr_by_salu &&
+             force_valu_read_sgpr_wait == other.force_valu_read_sgpr_wait;
    }
 };
 
@@ -1604,6 +1608,12 @@ handle_instruction_gfx11(State& state, NOP_ctx_gfx11& ctx, aco_ptr<Instruction>&
          unsigned expiry_count = instr->isSALU() ? 10 : 11;
          uint16_t imm = 0xffff;
 
+         if (ctx.force_valu_read_sgpr_wait) {
+            imm &= 0xfffe;
+            wait.sa_sdst = 0;
+            ctx.force_valu_read_sgpr_wait = false;
+         }
+
          for (Operand& op : instr->operands) {
             if (op.physReg() >= m0)
                continue;
@@ -2050,6 +2060,15 @@ insert_NOPs(Program* program)
           * SGPR might have been read by VALU if there was a previous shader part.
           */
          initial_ctx.sgpr_read_by_valu.flip();
+         /* We cannot assume the s_setpc source has not been read by VALU in the preceding shader/
+          * shader part, and there are GPU hangs in the wild suggesting that the s_setpc source may
+          * be susceptible to VALUReadSGPRHazard. It is impossible for the previous part to mitigate
+          * this, and it is not always known which register the s_setpc source was in, so force a
+          * wait to be emitted at the start of this part.
+          *
+          * TODO: This hypothesis is not yet conclusively proven. More testing is needed.
+          */
+         initial_ctx.force_valu_read_sgpr_wait = true;
       }
 
       mitigate_hazards<NOP_ctx_gfx11, handle_instruction_gfx11, resolve_all_gfx11>(program,

src/amd/compiler/tests/test_insert_nops.cpp

@@ -1764,8 +1764,12 @@ BEGIN_TEST(insert_nops.valu_read_sgpr.previous_part)
    /* Raytracing shaders have a prolog and may also be split into several parts. */
    program->stage = raytracing_cs;
 
-   /* Despite the SGPR never being read by a VALU in this shader, a sa_sdst(0) is needed. */
+   /* Despite the SGPR never being read by a VALU in this shader, a sa_sdst(0) is needed.
+    * The first instruction is also a sa_sdst(0) in RT shaders to protect against reads of the
+    * setpc target.
+    */
    //>> p_unit_test 0
+   //! s_waitcnt_depctr sa_sdst(0)
    //! s1: %0:s[4] = s_mov_b32 0
    //! s_waitcnt_depctr sa_sdst(0)
    //! s1: %0:s[64] = s_mov_b32 %0:s[4]