[PATCH] D67007: [ScopBuilder]Skip getting leader when merging statements to close holes

Michael Kruse via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 4 12:14:59 PDT 2019


Meinersbur added a comment.

Ahh, I think I understand the argument. Thank you for your patience. But I strongly suggest to update the description. Suggestion:

  // We are backtracking from the last element until we see Inst's leader in SeenLeaders and merge all into one set. Although which instructions are leaders changes during the execution of this loop, it is irrelevant as we are just searching for the element that we already confirmed is in the list.
   for (Instruction *Prev : reverse(SeenLeaders)) {

If `SetVector.insert` would have returned an iterator to the found element, we could also have forward iterated from that element to the end.

Maybe also add a comment to `bool Inserted = SeenLeaders.insert(Leader)` such as:

  // Since previous iterations might have merged sets, some items in SeenLeaders are not leaders anymore. However, former leaders represent the same set as the actually leader also in SeenLeaders and by construction are all trailing the list.

Since apparently its even hard for myself (who wrote this algorithm) to re-understand what's going on.


Repository:
  rPLO Polly

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

https://reviews.llvm.org/D67007





More information about the llvm-commits mailing list