[PATCH] D131526: [OMPIRBuilder] Add support for safelen clause

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added inline comments.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:3029-3032
+    addLoopMetadata(
+        CanonicalLoop,
+        MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.parallel_accesses"),
+                          AccessGroup}));
----------------
Instead of calling `addLoopMetadata` repeatedly, consider collecting metadata in a `SmallVector` and call `addLoopMetadata` only once at the end.
```
SmallVector<Metadata*> MDList;
if (Safelen == nullptr) {
  ... 
  MDList.push_back(MDNode::get(...));
}
...
addLoopMetadata(CanonicalLoop, MDList);
```


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:3048
+    // parameter. Therefore, use safelen only in the absence of simdlen.
+    ConstantInt *VectorizeWidth = Simdlen == nullptr ? Safelen : Simdlen;
     addLoopMetadata(
----------------
`safelen` should not mean the same as `llvm.loop.vectorize.width`. `safelen` could be unreasonably large to use as SIMD width or a non-power-of-2.

That being said, it's what `CGStmtOpenMP.cpp` does as well and I don't know any better way.


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

https://reviews.llvm.org/D131526