[PATCH] D85535: [OpenMPOpt][SplitMemTransfer][WIP] Splitting the runtime call __tgt_target_data_begin

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:532
+  const unsigned WaitNumParams = 2;
+  Type ** WaitDeclParams = new Type*[WaitNumParams];
+  WaitDeclParams[0] = Type::getInt64Ty(M->getContext());
----------------
hamax97 wrote:
> jdoerfert wrote:
> > You don't need a dynamic array here and below. 
> I didn't know what to do. I think we need a long life array because the array will be erased when the function scope ends, won't it?.
It will but the call copies the entries, that is why it takes an ArrayRef, bascially something that doesn't promise a lifetime. If you need the content, copy it.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:578
+    After = I;
+  }
+}
----------------
hamax97 wrote:
> jdoerfert wrote:
> > If you define `Before = After->getNextNode();` you can `insertBefore(Before)` without resetting the insertion point all the time. Isn't `Issues` unordered?
> Ohh sure, that is handier. But `Issue` is not unordered, if we mess up the order of the instructions we change the semantics. What is nice about `SetVector` is that it allows you to visit its elements in the order they were inserted.
Ordered makes sense ;)


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

https://reviews.llvm.org/D85535