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

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

jplehr added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenModule.h:1009
+  std::vector<llvm::WeakTrackingVH> getLLVMCompilerUsed() {
+    return static_cast<std::vector<llvm::WeakTrackingVH> &>(LLVMCompilerUsed);
+  }
----------------
I believe that this can be simplified to

```
std::vector<llvm::WeakTrackingVH> &getLLVMCompilerUsed() {
  return LLVMCompilerUsed;
}
```


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1509
+  setPropertyExecutionMode(StringRef Name, bool isSPMDMode,
+                           std::vector<llvm::WeakTrackingVH> LLVMCompilerUsed);
+
----------------
```...., bool IsSPMDMode, std::vector<llvm::WeakTrackingVH> &LLVMCompilerUsed);```


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4905
+    StringRef Name, bool isSPMDMode,
+    std::vector<llvm::WeakTrackingVH> LLVMCompilerUsed) {
+  auto *GVMode = new llvm::GlobalVariable(
----------------
```...., std::vector<llvm::WeakTrackingVH> &LLVMCompilerUsed) {````


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4914
+  GVMode->setVisibility(llvm::GlobalVariable::ProtectedVisibility);
+  assert(!GVMode->isDeclaration() &&
+         "Only globals with definition can force usage.");
----------------
Can this assertion ever be not be met? I'm just curious, given that you create that `GVMode` yourself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155794