Skip to content

Commit

Permalink
[DI] sink injectActorHops() after processing destroys
Browse files Browse the repository at this point in the history
While trying to reuse the liveness-points analysis originally in DI for
injecting actor hops for more general purposes, Pavel and I discovered
that the point at which we are injecting the hops might not have
fully-computed the liveness information.

That appears to be the case because we were computing the fully-initialized
points before having processed destroy/releases of TheMemory. While this
most likely had no influence on the actor hop injection, it does affect
what the outgoing AvailabilitySet contains for a block. In particular, for
this example:

```swift
struct X {
  init(cond: Bool) {
    var _storage: (name: String, age: Int)
    _storage.name = ""
    if cond {
      _storage.age = 30
    } else {
      _storage.age = 40
    }
  }
}
```

But because we are determine the full initialization points before processing
the destroy, the liveness analysis doesn't iterate to correctly determine the
out-availability of block 1 and 3 (corresponding to the then and else blocks
in the example above). Here's the debug output showing that issue:

```
*** Definite Init looking at:   %5 = mark_uninitialized [var] %4 : $*(name: String, age: Int) // users: %37, %12, %22, %32

Get liveness 0, #1 at   assign %11 to %13 : $*String                    // id: %14
Get liveness 1, #1 at   assign %21 to %23 : $*Int                       // id: %24
  Get liveness for block 1
    Iteration 0
    Result: (yn)
Get liveness 1, #1 at   assign %31 to %33 : $*Int                       // id: %34
  Get liveness for block 3
    add block 2 to worklist
    Iteration 0
      Block 2 out: (yn)
    Iteration 1
      Block 2 out: (yn)
    Result: (yn)
full-init-finder: rejecting bb0 b/c non-Yes OUT avail
full-init-finder: rejecting bb1 b/c non-Yes OUT avail
full-init-finder: rejecting bb2 b/c no non-load uses.
full-init-finder: rejecting bb3 b/c non-Yes OUT avail
full-init-finder: rejecting bb4 b/c no non-load uses.
Get liveness 0, #2 at   destroy_addr %5 : $*(name: String, age: Int)    // id: %37
  Get liveness for block 4
    add block 3 to worklist
    add block 1 to worklist
    Iteration 0
      Block 1 out: (yy)
      Block 3 out: (yy)
    Iteration 1
      Block 1 out: (yy)
      Block 3 out: (yy)
    Result: (yy)
```

So, this patch basically just sinks the computation so it happens after, so that
we force the incremental liveness analysis to also consider the liveness at the
point of the destroy, but before having done any other transformations or modifications
to the CFG to handle a destroy of something partially initialized.
  • Loading branch information
kavon committed Sep 23, 2022
1 parent 6dda91c commit 7719582
Showing 1 changed file with 9 additions and 3 deletions.
12 changes: 9 additions & 3 deletions lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1142,9 +1142,6 @@ void LifetimeChecker::doIt() {
// If we emitted an error, there is no reason to proceed with load promotion.
if (!EmittedErrorLocs.empty()) return;

// Insert hop_to_executor instructions for actor initializers, if needed.
injectActorHops();

// If the memory object has nontrivial type, then any destroy/release of the
// memory object will destruct the memory. If the memory (or some element
// thereof) is not initialized on some path, the bad things happen. Process
Expand All @@ -1155,6 +1152,15 @@ void LifetimeChecker::doIt() {
processNonTrivialRelease(i);
}

/// At this point, we should have computed enough liveness information to
/// provide accurate information about initialization points, even for
/// local variables within a function, because we've now processed the
/// destroy/releases.

// Insert hop_to_executor instructions for actor initializers, if needed.
injectActorHops();


// If the memory object had any non-trivial stores that are init or assign
// based on the control flow path reaching them, then insert dynamic control
// logic and CFG diamonds to handle this.
Expand Down

0 comments on commit 7719582

Please sign in to comment.