[PATCH] D74524: [Scheduling] Improve memory ops cluster preparation

[Qiu Chaofan via Phabricator via llvm-commits]

qiucf added a subscriber: atrick.
qiucf added a comment.

In D74524#1873828 <https://reviews.llvm.org/D74524#1873828>, @foad wrote:

> Thanks for working on this. I have to admit I have never understood the store chain stuff in BaseMemOpClusterMutation::apply. Can you point me at any documentation that explains what a store chain is, and why it's based on control dependence, and why it's useful for clustering?


Per my understanding (maybe not correct), ‘chains’ are used to describe some kinds of dependency other than use-def, such as memory operations. (This discussion <http://lists.llvm.org/pipermail/llvm-dev/2017-June/114782.html> is a good reference) So it’s natural for the chains to become anti, output, or order dependencies.

About why it's useful for clustering, I guess what `apply` does is a cheap but not perfect way to eliminate 'impossible' cluster pairs (like `SU(3)` clustered with `SU(5)` but `SU(4)` is a barrier), since if two units has the same pred-dep, they're likely to be able to neighbor. Actually, as I roughly change `if (Pred.isCtrl() && !Pred.isArtificial())` to `if (Pred.isNormalMemoryOrBarrier())`, all `check-llvm` tests passed. Since this method was written by @atrick several years ago and its core logic haven't changed, it's sometimes also confusing to me :) There're some other issue with this implementation, nevertheless, they may save compiling time.

In D74524#1873912 <https://reviews.llvm.org/D74524#1873912>, @nhaehnle wrote:

> Is this approach generally sound? I worry that you may get circular dependencies, e.g.: A has no control dependency, but B has a control dependency on A (possibly indirectly). Clustering introduces new artificial dependencies, which could potentially lead to A becoming dependent on an SUnit C that depends on B, creating a circular dependency.


Currently (before D72031 <https://reviews.llvm.org/D72031> lands), after cluster edges created, only succ-deps (assume that's C) of A will depend on B. If we want circular dependency to happen, B has to directly depends on C or depends on D which depends on C. But we calls `Topo.IsReachable` before adding edges and that would be detected. (A can't depends on C because C depends on A) This seems not related to whether A or B has control dependencies. Am I right?

  +->B+---->A<-+
  |  +         |
  |  +>D+      |
  |     |      |
  |     v      |
  +----+C+-----+


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74524