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

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:138
+              A.push_back(v);
+      };
+      /// It takes two call instructions and compares for the merging
----------------
Why would we unpack the arguments if we can look them up in a call?


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:237
+      return false;
+  }
+    /// Try to delete parallel regions if possible
----------------
Let's create a revision just with the test first. The code above will require rewriting and reviewing is hard, partially because of formatting and naming conventions. FWIW, OpenMPOpt is clang-formatted.


================
Comment at: llvm/test/Transforms/OpenMP/parallel_omp_for_loop_merge1.ll:2
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+; RUN: opt -S  -openmpopt <  %s | FileCheck %s
+; The IR was produced using -O0 and later applying mem2reg, simplifycfg, and instrcombine passes
----------------
can you run the update_test_checks.py script on this to create the run lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90103