[PATCH] D38403: [Polly][ScopBuilder] Introduce -polly-stmt-granularity=scalar-indep option.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur marked 2 inline comments as done.
Meinersbur added a comment.

In https://reviews.llvm.org/D38403#884578, @nandini12396 wrote:

> In https://reviews.llvm.org/D38403#884443, @Meinersbur wrote:
>
> > ForwardOpTree cannot forward everything, only 'speculatable' instructions and loads that are known to not have changed.
>
>
> Ok. So, this option will, in future cover https://reviews.llvm.org/D37337#859617 ?


Yes, although I think Tobias also had in mind what ForwardOpTree does today. Currently `getScopStmt(Instruction*)` allows returning only a single ScopStmt anyway, such that at this stage we cannot duplicate instructions into multiple statements. Although we could choose a 'primary' location for each instruction. There is definitely still room for improvement.



================
Comment at: lib/Analysis/ScopBuilder.cpp:772
+    for (auto Prev : reverse(SeenLeaders)) {
+      auto PrevLeader = UnionFind.getLeaderValue(SeenLeaders.back());
+      if (PrevLeader == Leader)
----------------
bollu wrote:
> AFAICT, we don't need to call `getLeaderValue` on `SeenLeaders.back()`, because `SeenLeaders` only contains leaders. 
Former leaders may have merged into other leaders since it was added.


https://reviews.llvm.org/D38403