[PATCH] D112731: [AMDGPU] Really preserve LiveVariables in SILowerControlFlow

Ruiling, Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 1 05:31:18 PDT 2021


ruiling added inline comments.


================
Comment at: llvm/lib/CodeGen/LiveVariables.cpp:729
+  for (unsigned UseBBNum : UseBlocks) {
+    if (VI.AliveBlocks.test(UseBBNum))
+      continue;
----------------
I would suggest you check against `LiveToEndBlocks` instead of `AliveBlocks`.
Think of one possible situation:
bb0:
  %1 = ...
bb1:
  ... = phi [%1 %bb0], [%2 %bb1]
  %2 = ...
   ... = add %2, 1
   br i1 %x, bb1, bb2
The add instruction at bb1 that reads %2 should not be marked as KILL. Sounds like we are not doing it right currently.



================
Comment at: llvm/lib/CodeGen/LiveVariables.cpp:732
+    MachineBasicBlock &UseBB = *MF->getBlockNumbered(UseBBNum);
+    // Only add kills in DefBB if all uses are in DefBB.
+    if (&UseBB == &DefBB && UseBlocks.count() != 1)
----------------
foad wrote:
> ruiling wrote:
> > why do we need this?
> This is the cheapest way I could think of to test whether Reg is live-to-end of DefBB. Note that DefBB never appears in AliveBlocks.
Does my comment above sound good to you? `UseBlocks.count() == 1` does not mean whether the Reg is live-to-end of DefBB.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112731



More information about the llvm-commits mailing list