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

Michael Kruse via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 8 05:10:07 PDT 2017

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`.


More information about the llvm-commits mailing list