[PATCH] D118558: [IVDescriptors] Support FOR where we have multiple sink pointed

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 10 12:24:06 PST 2022


fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.

LGTM, thanks with the inline suggestions.

> Handles the case where Previous doesn't come before LastPrev incorrectly.

Could you update the commit messages before landing this change? It should definitely handle this correctly rather than incorrectly :)



================
Comment at: llvm/lib/Analysis/IVDescriptors.cpp:927
+      const BasicBlock *CurBB = Previous->getParent();
+      if (LastBB != CurBB)
+        return false;
----------------
might be more compact to just check `LastPrev->getParent() != Previous->getParent()`. 




================
Comment at: llvm/lib/Analysis/IVDescriptors.cpp:930
+
+      return Previous->comesBefore(LastPrev);
+    }
----------------
Maybe add a comment along the lines `If LastPrev comes after the current Previous, SinkCandidate already gets sunk past Previous and there is nothing to do`.


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

https://reviews.llvm.org/D118558



More information about the llvm-commits mailing list