[PATCH] D155794: [OpenMP][OpenMPIRBuilder] Migrate setPropertyExecutionMode() from Clang to OpenMPIRBuilder.

[Jan-Patrick Lehr via Phabricator via cfe-commits]

jplehr added a comment.

Not an expert here, only some minor comments.



================
Comment at: clang/lib/CodeGen/CodeGenModule.h:1008
 
+  std::vector<llvm::WeakTrackingVH> *getLLVMCompilerUsed() {
+    return &LLVMCompilerUsed;
----------------
Return a reference instead of pointer here?


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1506
+  void
+  setPropertyExecutionMode(StringRef Name, bool Mode,
+                           std::vector<llvm::WeakTrackingVH> *LLVMCompilerUsed);
----------------
Rename to sth like `isSPMDMode` or so? To me `Mode` reads overly general.


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1507
+  setPropertyExecutionMode(StringRef Name, bool Mode,
+                           std::vector<llvm::WeakTrackingVH> *LLVMCompilerUsed);
+
----------------
Should this preferably be a reference instead of pointer?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155794