[llvm] 01f3e2d - [StackLifetime] More efficient loop for LivenessType::Must

[Vitaly Buka via llvm-commits]

Author: Vitaly Buka
Date: 2022-09-28T16:28:45-07:00
New Revision: 01f3e2d6197829486622a440acca83786d52fc85

URL: https://github.com/llvm/llvm-project/commit/01f3e2d6197829486622a440acca83786d52fc85
DIFF: https://github.com/llvm/llvm-project/commit/01f3e2d6197829486622a440acca83786d52fc85.diff

LOG: [StackLifetime] More efficient loop for LivenessType::Must

CFG with cycles may requires additional passes of "while (Changed)"
iteration if to propagate data back from latter blocks to earlier blocks,
ordered according to depth_fist.

OR logic, used for ::May, converge to stable state faster then AND logic
use for ::Must.

Though the better solution is to switch to some some form of queue, but
having that this one is good enough, I will consider to do that later.

We can switch ::Must to OR logic if we calculate "may be dead" instead
of direct "must be alive" and then convert values to match existing
interface.

Additionally it fixes correctness in "@cycle" test.

Reviewed By: kstoimenov, fmayer

Differential Revision: https://reviews.llvm.org/D134796

Added: 
    

Modified: 
    llvm/lib/Analysis/StackLifetime.cpp
    llvm/test/Analysis/StackSafetyAnalysis/lifetime.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/StackLifetime.cpp b/llvm/lib/Analysis/StackLifetime.cpp
index d6c3fc80e1793..72a1ac7b4eff5 100644
--- a/llvm/lib/Analysis/StackLifetime.cpp
+++ b/llvm/lib/Analysis/StackLifetime.cpp
@@ -174,32 +174,32 @@ void StackLifetime::collectMarkers() {
 
 void StackLifetime::calculateLocalLiveness() {
   bool Changed = true;
+
+  // LiveIn, LiveOut and BitsIn have a 
diff erent meaning deppends on type.
+  // ::Maybe true bits represent "may be alive" allocas, ::Must true bits
+  // represent "may be dead". After the loop we will convert ::Must bits from
+  // "may be dead" to "must be alive".
   while (Changed) {
+    // TODO: Consider switching to worklist instead of traversing entire graph.
     Changed = false;
 
     for (const BasicBlock *BB : depth_first(&F)) {
       BlockLifetimeInfo &BlockInfo = BlockLiveness.find(BB)->getSecond();
 
-      // Compute LiveIn by unioning together the LiveOut sets of all preds.
+      // Compute BitsIn by unioning together the LiveOut sets of all preds.
       BitVector BitsIn;
       for (const auto *PredBB : predecessors(BB)) {
         LivenessMap::const_iterator I = BlockLiveness.find(PredBB);
         // If a predecessor is unreachable, ignore it.
         if (I == BlockLiveness.end())
           continue;
-        switch (Type) {
-        case LivenessType::May:
-          BitsIn |= I->second.LiveOut;
-          break;
-        case LivenessType::Must:
-          if (BitsIn.empty())
-            BitsIn = I->second.LiveOut;
-          else
-            BitsIn &= I->second.LiveOut;
-          break;
-        }
+        BitsIn |= I->second.LiveOut;
       }
 
+      // Everything is "may be dead" for entry without predecessors.
+      if (Type == LivenessType::Must && BitsIn.empty())
+        BitsIn.resize(NumAllocas, true);
+
       // Update block LiveIn set, noting whether it has changed.
       if (BitsIn.test(BlockInfo.LiveIn)) {
         BlockInfo.LiveIn |= BitsIn;
@@ -212,8 +212,18 @@ void StackLifetime::calculateLocalLiveness() {
       // because we already handle the case where the BEGIN comes
       // before the END when collecting the markers (and building the
       // BEGIN/END vectors).
-      BitsIn.reset(BlockInfo.End);
-      BitsIn |= BlockInfo.Begin;
+      switch (Type) {
+      case LivenessType::May:
+        BitsIn.reset(BlockInfo.End);
+        // "may be alive" is set by lifetime start.
+        BitsIn |= BlockInfo.Begin;
+        break;
+      case LivenessType::Must:
+        BitsIn.reset(BlockInfo.Begin);
+        // "may be dead" is set by lifetime end.
+        BitsIn |= BlockInfo.End;
+        break;
+      }
 
       // Update block LiveOut set, noting whether it has changed.
       if (BitsIn.test(BlockInfo.LiveOut)) {
@@ -222,6 +232,14 @@ void StackLifetime::calculateLocalLiveness() {
       }
     }
   } // while changed.
+
+  if (Type == LivenessType::Must) {
+    // Convert from "may be dead" to "must be alive".
+    for (auto &[BB, BlockInfo] : BlockLiveness) {
+      BlockInfo.LiveIn.flip();
+      BlockInfo.LiveOut.flip();
+    }
+  }
 }
 
 void StackLifetime::calculateLiveIntervals() {

diff  --git a/llvm/test/Analysis/StackSafetyAnalysis/lifetime.ll b/llvm/test/Analysis/StackSafetyAnalysis/lifetime.ll
index ed257753cad34..6d5789b91f90e 100644
--- a/llvm/test/Analysis/StackSafetyAnalysis/lifetime.ll
+++ b/llvm/test/Analysis/StackSafetyAnalysis/lifetime.ll
@@ -1070,22 +1070,19 @@ entry:
 if.then:
 ; CHECK: if.then:
 ; MAY-NEXT: Alive: <x y>
-; MUST-NEXT: Alive: <>
+; MUST-NEXT: Alive: <x>
   call void @llvm.lifetime.start.p0i8(i64 1, i8* %y)
 ; CHECK: call void @llvm.lifetime.start.p0i8(i64 1, i8* %y)
-; MAY-NEXT: Alive: <x y>
-; MUST-NEXT: Alive: <y>
-; FIXME: Alive: <x y> is expected above.
+; CHECK-NEXT: Alive: <x y>
 
   br i1 %a, label %if.then, label %if.end
 ; CHECK: br i1 %a, label %if.then, label %if.end
-; MAY-NEXT: Alive: <x y>
-; MUST-NEXT: Alive: <y>
+; CHECK-NEXT: Alive: <x y>
 
 if.end:
 ; CHECK: if.end:
 ; MAY-NEXT: Alive: <x y>
-; MUST-NEXT: Alive: <>
+; MUST-NEXT: Alive: <x>
 
   ret void
 }