[PATCH] D83311: [CodeMoverUtils] Add optional data dependence checks using MSSA

[rithik sharma via Phabricator via llvm-commits]

RithikSharma marked 2 inline comments as done.
RithikSharma added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:222
+                      SmallPtrSet<Instruction *, 10> InstsToCheck) {
+  return !std::any_of(InstsToCheck.begin(), InstsToCheck.end(),
+                      [&DI, &I](Instruction *CurInst) {
----------------
Whitney wrote:
> 1. !any_of can change to none_of
> 2. use llvm::none_of instead of std::none_of
> ```
> return none_of(InstsToCheck, [&DI, &I]
> ...
> ```
Acknowledged, updated with changes.


================
Comment at: llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp:513
                                        *CI_safecall->getNextNode(), DT, &PDT,
-                                       &DI));
+                                       &DI, &MSSAU));
+        EXPECT_TRUE(isSafeToMoveBefore(*CI_safecall->getPrevNode(),
----------------
Whitney wrote:
> RithikSharma wrote:
> > Whitney wrote:
> > > change all the existing ones to `&DI, nullptr))` to make sure you are testing `DI`.
> > Sure but even when we give preference to DI?
> > 
> > ```
> >  if (DI)
> >     return isDependenceSafe(I, *DI, InstsToCheck);
> >   else if (MSSAU)
> >     return isDependenceSafe(I, *MSSAU, InstsToCheck);
> > ```
> Yes, because the code may change.
Thanks, Acknowledged.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83311