[PATCH] D71192: AMDGPU: Fix AMDGPUUnifyDivergentExitNodes with no normal returns

[Connor Abbott via Phabricator via llvm-commits]

cwabbott added a comment.

In D71192#1886101 <https://reviews.llvm.org/D71192#1886101>, @piotr wrote:

> Hi @cwabbott, 
>  This commit causes a GPU hang on amdvlk in one test, due to the missing "done" bit on the normal export. I think in the attached case the patch incorrectly classifies the export as being in an infinite loop.F11390737: before_AMDGPUUnifyDivergentExitNodes.ll <https://reviews.llvm.org/F11390737>


Hi @piotr,

I think what's happening is that the "normal" return, that already has an export before it, is uniformly reached (i.e. only reached via uniform branches). Normally this means that we don't have to do anything with it, so the pass ignores it, but then we remove the "done" bit even though the final dummy export, which is supposed to replace it, isn't reached when returning normally. There are two ways I can see to solve it:

- When `InsertExport` is true, unify *all* the return blocks, even the uniformly-reached ones. For this shader, we would still remove the done bit on the original export, but we'd replace the return with a branch to the `UnifiedReturnBlock` with the dummy export.
- Do the above, but when we can prove that all the existing exports are post-dominated by the normal, uniformly-reached returns, we can avoid clearing their done bits or unifying them.

The second option would prevent the regression in code quality, but it would be more complicated. We've deliberately made this as simple as possible already, because it only kicks in when there are infinite loops (that might have kills in them) and this seems pretty uncommon in real-world shaders. Is this coming from a test or an actual game?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71192/new/

https://reviews.llvm.org/D71192