[PATCH] D47893: Add a PhiValuesAnalysis pass to calculate the underlying values of phis

[Sebastian Pop via Phabricator via llvm-commits]

sebpop requested changes to this revision.
sebpop added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/Analysis/PhiValues.cpp:45
+// on B they need to depend on the phis that B depends on.
+void PhiValues::processPhi(const PHINode *InitialPhi) {
+  SmallVector<const PHINode *, 4> Worklist = {InitialPhi};
----------------
Why not using Danny's implementation of Tarjan's algorithm?
processPhi() would be findSCC()

>>! In D44564#1040222, @dberlin wrote:
> https://reviews.llvm.org/D28934 has a version of the SCC finder you can
> borrow if you want (TarjanSCC class)

```
// Tarjan's algorithm with Nuutila's improvements
// Wish we could use SCCIterator, but graph traits makes it *very* hard to
// create induced subgraphs because it
// 1. only works on pointers, so i can't just create an intermediate struct
// 2. the functions are static, so i can't just override them in a subclass of
// graphtraits, or otherwise store state in the struct.

struct TarjanSCC {
  unsigned int DFSNum = 1;
  SmallPtrSet<Value *, 8> InComponent;
  DenseMap<Value *, unsigned int> Root;
  SmallPtrSet<Value *, 8> PHISet;
  SmallVector<Value *, 8> Stack;
  // Store the components as vector of ptr sets, because we need the topo order
  // of SCC's, but not individual member order
  SmallVector<SmallPtrSet<PHINode *, 8>, 8> Components;
  TarjanSCC(const SmallVectorImpl<PHINode *> &Phis)
      : PHISet(Phis.begin(), Phis.end()) {
    for (auto Phi : Phis)
      if (Root.lookup(Phi) == 0)
        FindSCC(Phi);
  }
  void FindSCC(PHINode *Phi) {
    Root[Phi] = ++DFSNum;
    // Store the DFS Number we had before it possibly gets incremented.
    unsigned int OurDFS = DFSNum;
    InComponent.erase(Phi);
    for (auto &PhiOp : Phi->operands()) {
      // Only inducing the subgraph of phis we got passed
      if (!PHISet.count(PhiOp))
        continue;
      if (Root.lookup(PhiOp) == 0)
        FindSCC(cast<PHINode>(PhiOp));
      if (!InComponent.count(PhiOp))
        Root[Phi] = std::min(Root.lookup(Phi), Root.lookup(PhiOp));
    }
    // See if we really were the root, by seeing if we still have our DFSNumber,
    // or something lower
    if (Root.lookup(Phi) == OurDFS) {
      Components.resize(Components.size() + 1);
      auto &Component = Components.back();
      Component.insert(Phi);
      DEBUG(dbgs() << "Component root is " << *Phi << "\n");
      InComponent.insert(Phi);
      while (!Stack.empty() && Root.lookup(Stack.back()) >= OurDFS) {
        DEBUG(dbgs() << "Component member is " << *Stack.back() << "\n");
        Component.insert(cast<PHINode>(Stack.back()));
        InComponent.insert(Stack.back());
        Stack.pop_back();
      }
    } else {
      // Part of a component, push to stack
      Stack.push_back(Phi);
    }
  }
};
```


================
Comment at: lib/Analysis/PhiValues.cpp:71
+            // the use list of the Value.
+            for (auto &VH : Vals)
+              PhiToUnderlyingValues[D].push_back((Value *)VH);
----------------
This code is nested at loop depth 4.


================
Comment at: lib/Analysis/PhiValues.cpp:75
+            for (auto &Pair : PhiDependence)
+              if (Pair.second.count(VPN))
+                Pair.second.insert(D);
----------------
Same.


https://reviews.llvm.org/D47893