[PATCH] D114146: [DA][NFC] Update publication - add remarks

[Sameer Sahasrabuddhe via Phabricator via llvm-commits]

sameerds added a comment.

Not related to the changes proposed in this review, but it seems the PDT is not used by the implementation. It might be good to take out that dependency.



================
Comment at: llvm/lib/Analysis/SyncDependenceAnalysis.cpp:103
+//
+// -- Known Limitations & Future Work --
+// * The algorithm requires reducible loops because the implementation
----------------
Good to see both these points spelled out. We are currently working on an implementation that works with irreducible control flow. It's still a work in progress, but involves the new "CycleInfo" being introduced in D112696. I do believe that the single pass of DFA is a strength that need not be lost when handling irreducible control flow. CycleInfo provides a predictable way to work around the lack of a unique header. One just needs to take extra care about entering the same irreducible loop multiple times when constructing ModifiedPO.


================
Comment at: llvm/lib/Analysis/SyncDependenceAnalysis.cpp:149
 // * We (virtually) modify the CFG.
-// * We want a loop-compact block enumeration, that is the numbers assigned by
-//   the traveral to the blocks of a loop are an interval.
+// * We want a loop-compact block enumeration, that is the numbers assigned to
+//   blocks of a loop form an interval
----------------
Is it correct to say that this is not critical for correctness? From what I understood, if other blocks (beyond the loop exit) got interleaved, the implementation will work, but unnecessarily visit and skip the interleaved blocks when walking backwards on the ModifiedPO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114146