[PATCH] D132447: AMDGPU: Add a pass to fix SGPR liveness

[Ruiling, Song via Phabricator via llvm-commits]

ruiling added a comment.

In D132447#3758581 <https://reviews.llvm.org/D132447#3758581>, @foad wrote:

> My understanding is that this pass only fixes specific liveness problems that are created by D132450 <https://reviews.llvm.org/D132450>. It is not a general purpose pass that could handle any possible pattern of SGPR usage.

Thank you for your comment @foad. But I don't agree the problem was created by D132450 <https://reviews.llvm.org/D132450>. see the attached sgpr-liveness.ll, a hand-made test can also show the problem (The tricky thing is that the existing structurizer still simplify the IR even it is already structurized, thus it helps hiding the issue, but I don't think it is the expected behavior). Yes the current design of the pass only works for this known pattern. That is just because I don't know if there are any other broken patterns.



================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRLiveness.cpp:17
+///
+/// BB1 ends with a divergent branch. The virtual registers shown in above
+/// example are dead in BB2. After register allocation, they may end up
----------------
foad wrote:
> The implementation does not check that there is a divergent branch, so it would do the same thing for uniform control flow.
Sounds true, this is doing things conservative, but I think it may not cause any regression in real case. Practically, such kind of pattern in source program should already been optimized into the single dominate value. The only possible cases that I can think of are either "opt-none" or "created by structurizeCFG". If such pattern is created by structurizeCFG, BB1 will be very likely to be divergent because structurizeCFG was designed to try to skip uniform branching as much as possible. I am not sure if checking the terminator of BB1 against s_cbranch_scc0/scc1 for uniform branch sounds good idea?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132447