[Openmp-commits] [PATCH] D90103: Add OpenMP for optimization

[Stefan Stipanovic via Phabricator via Openmp-commits]

sstefan1 added a comment.

On the first look, you should at least clang-format this as it is not very readable right now.

There are still no tests.



================
Comment at: .arcconfig:2
 {
-  "phabricator.uri" : "https://reviews.llvm.org/",
-  "repository.callsign" : "G",
-  "conduit_uri" : "https://reviews.llvm.org/"
+  "phabricator.uri" : "https://reviews.llvm.org/"
 }
----------------
This seems unrelated.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:71
+    /// Keeps map of __kmpc_static_init4 and its __kmpc_static_fini calls for each OpenMP for loop
+    std::map<CallInst *, CallInst *> call_init_fini_mapping;
+    std::map<CallInst *, BasicBlock*> call_basicblock_mapping;
----------------
Maybe consider using LLVM's ADTs.? Same goes for the rest of the patch.

Also, you should follow the [[ https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly | coding style  ]]


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:431
+/// does function printing
+void printFunction(Function &F) {
+    F.print(errs(), nullptr);
----------------
Seems unused.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90103