[PATCH] D86155: [OpenMPOpt][SplitMemTransfer] Moving the "wait" down

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:719
+  Instruction *CurrentI = RuntimeCall;
+  bool IsWorthIt = false;
+  while ((CurrentI = CurrentI->getNextNode())) {
----------------
Can't you just check if CurrentI is RuntimeCall?


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:737
+        return nullptr;
+      }
+    }
----------------
You cannot look for the target calls. What if you encounter a call to a function that contains such a call? However, as long as we don't allow side effects this should not be an issue, just remove this part.

You need to check if it is a call that it has a `nosync` attribute though.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:746
+  // Return end of BasicBlock.
+  return &*(--RuntimeCall->getParent()->end());
+}
----------------
Or something else that looks less confusing.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:752
+                             Instruction *WaitMovementPoint) {
+    assert(WaitMovementPoint && "No place to move the split runtime call!");
+
----------------
Style: Just make them references instead of pointers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86155