[PATCH] D109890: Flang OpenMP Report Plugin

[Ivan Zhechev via Phabricator via llvm-commits]

ijan1 added inline comments.


================
Comment at: flang/examples/flang-omp-report-plugin/flang-omp-report-support.h:140-142
+  std::map<std::string, int> ompConstructCounter;
+  std::map<std::string, int> ompClauseCounter;
+  std::map<std::pair<std::string, std::string>, int> constructClauseCount;
----------------
kiranchandramohan wrote:
> Are these three maps (ompConstructCounter, ompClauseCounter, constructClauseCount) required if the information collected by these is not emitted?
`constructClauseCount` is required. I've removed the others.


================
Comment at: flang/examples/flang-omp-report-plugin/flang-omp-report-support.h:150-151
+                        // datastructure which can be accessed through indices.
+  LogRecord *curLoopLogRecord{nullptr};
+  std::vector<LogRecord *> loopLogRecordStack;
+  std::vector<OmpWrapperType *> ompWrapperStack;
----------------
kiranchandramohan wrote:
> These two (curLoopLogRecord, loopLogRecordStack) were required to maintain which loop we are currently processing. This was required since the standalone tool was operating before semantic checks (which corrects some loop information). Since the plugin tool runs after semantics, it might not be required now. Please check.
Removing `curLoopLogRecord` leaves the "clauses" part in the yaml file empty. `loopLogRecordStack` was not needed and removed.


================
Comment at: flang/tools/flang-omp-report-plugin/count-openmp.cpp:82
+
+class Dumper {
+  void openStream(llvm::StringRef fileName) {
----------------
awarzynski wrote:
> This is a very generic name. Could it be a bit more specific?
Removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109890