[PATCH] D136892: [AMDGPU] Unify uniform return and divergent unreachable blocks

[Ruiling, Song via Phabricator via llvm-commits]

ruiling accepted this revision.
ruiling added a comment.

LGTM



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUUnifyDivergentExitNodes.cpp:224
       UnreachableBlocks.push_back(BB);
+      HasDivergentUnreachableBlocks |= !isUniformlyReached(DA, *BB);
     } else if (BranchInst *BI = dyn_cast<BranchInst>(BB->getTerminator())) {
----------------
ruiling wrote:
> I think the key point is we need to make sure to unify the function exits if there is any divergent branch to make sure the structurizer is not skipped when needed. The change does not work if there are both divergent reached return block and uniformly reached return block (no unreachable block). In this case, we still need to unify both the uniform/divergent return blocks. So, I would suggest you change to something like below:
> ```
> scanning the terminators of all the blocks in the function,
> if there is any divergent branch, set HasDivergentBranch to true.
> 
> then:
>       if (!HasDivergentBranch && isUniformlyReached(DA, *BB))
>         ReturningBlocks.push_back(BB);
> 
> It is also preferred to process unreachable blocks in the same way:
>      if (!HasDivergentBranch && isUniformlyReached(DA, *BB))
>         UnreachableBlocks.push_back(BB);
> ```
I think the code suggestion I have given is wrong, really sorry for that. It should be: `if (HasDivergentBranch)` instead of `if (...&&...)`, anyway this new version works for me.


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

https://reviews.llvm.org/D136892