[PATCH] D35663: [Polly] Remove dependency of `Scop::getStmtFor(Inst)` on `getStmtFor(BB)`. [NFC].

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: lib/Analysis/ScopInfo.cpp:4095
+    for (ScopStmt &Stmt : Stmts)
+      if (Stmt.getArrayAccessOrNULLFor(LI)) {
+        invalidate(INVARIANTLOAD, LI->getDebugLoc(), LI->getParent());
----------------
[Suggestion] Could you add a comment what is happening here?


================
Comment at: lib/Transform/ForwardOpTree.cpp:164
     Loop *DefLoop = LI->getLoopFor(UseInst->getParent());
-    ScopStmt *DefStmt = S->getStmtFor(UseInst);
+    ScopStmt *DefStmt = S->getLastStmtFor(UseInst->getParent());
     assert(DefStmt && "Value must be defined somewhere");
----------------
[Serious] The statement where `UseInst` is defined should be the preferred one. The last statement of a BasicBlock is only a backup. Using any BB else than the preferred one would require to add another scalar write (currently not implemented in ForwardOpTree), which is what `-polly-optree` is trying to avoid.

The case is probably rare enough that we could also just `return FD_CannotForward`.
```
if (!DefStmt)
  return FD_CannotForward;
```

Note that in a recent commit this line has been moved to `forwardTree`.


https://reviews.llvm.org/D35663