[PATCH] D92189: [OpenMPIRBuilder] forward arguments as pointers to outlined function

[Johannes Doerfert via Phabricator via llvm-commits]

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

LGTM, one comment and two nits. Feel free to commit if you agree with the suggestions or come back with concerns. Thanks for working on this and taking the time to fix it like this!



================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:124
+  ///              a pointer and can thus be passed directly, or a pointer to
+  ///              \p Original otherwise.
   /// \param ReplVal The replacement value, thus a copy or new created version
----------------
It is not a pointer to original, is it? It is an equivalent but potentially not the same IR value.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:740
+      Builder.restoreIP(PrivCB(InnerAllocaIP, Builder.saveIP(), V,
+                               (Reloaded ? *Reloaded : V), ReplacementValue));
       assert(ReplacementValue &&
----------------
Nit: Rename `Reloaded` to `Inner` to make it consistent with the callback (description). With `Value* Inner = V;` you can avoid the select above.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:745
 
-    for (Use *UPtr : Uses)
-      UPtr->set(ReplacementValue);
+    if (ReplacementValue != &V)
+      for (Use *UPtr : Uses)
----------------
Nit: This could go back up again I guess.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92189