Skip to content

Conversation

@jmmartinez
Copy link
Contributor

@jmmartinez jmmartinez commented Jan 6, 2025

If the code in the "then" block is modifying the exec mask, we must retain the s_cbranch_execz branch.

Consider this example:

    s_cbranch_execz after
    s_or_b32 exec_lo, exec_lo, -1
after:
    ...

If the branch is removed, when we reach after exec is never zero, while before it would have been zero.

I stumbled upon this bug by accident. I'm trying to see if the bug is more general than this (this should be a problem for any SALU operation writing to a non-SSA register) and how to test it. Maybe something like

    bool DontMoveAcrossStore = true;
    if (??? && !MI.isSafeToMove(DontMoveAcrossStore))
      return true;

Edit: I saw an example where there was a mov to m0

 ; GFX8-NEXT:    s_xor_b64 s[0:1], exec, s[0:1]
+; GFX8-NEXT:    s_cbranch_execz .LBB29_8
 ; GFX8-NEXT:  ; %bb.7:
 ; GFX8-NEXT:    v_mov_b32_e32 v2, s2
 ; GFX8-NEXT:    s_mov_b32 m0, -1
 ; GFX8-NEXT:    ds_add_rtn_f32 v2, v2, v1
-; GFX8-NEXT:  ; %bb.8:
+; GFX8-NEXT:  .LBB29_8:

Edit++: I just realized that this pass is run after the virtual register are lowered to physical registers, so any scalar operation writing to a physical register would pose a problem. Am I right ?

If the code in the "then" block is modifying the exec mask, we must
retain the s_cbranch_execz branch.

Consider this example:

        s_cbranch_execz after
        s_or_b32 exec_lo, exec_lo, -1
    after:
        ...

If the branch is removed, when we reach after exec is never zero, while
before it would have been zero.
@jmmartinez jmmartinez requested review from arsenm and jayfoad January 6, 2025 16:05
@jmmartinez jmmartinez self-assigned this Jan 6, 2025
@jmmartinez jmmartinez closed this Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant