[PATCH] D83659: [flang][OpenMP] upstream OpenMP lowering

[Sourabh Singh Tomar via Phabricator via llvm-commits]

SouraVX added inline comments.


================
Comment at: flang/include/flang/Lower/OpenMP.h:35-40
+void genOMP(AbstractConverter &, pft::Evaluation &,
+            const parser::OpenMPStandaloneConstruct &);
+
+void genOMP(AbstractConverter &, pft::Evaluation &,
+            const parser::OpenMPSimpleStandaloneConstruct &);
+
----------------
kiranchandramohan wrote:
> Do these need to be visible outside?
No, I'll make them static. Thanks. `genOpenMPConstruct` is the only public interface needed.


================
Comment at: flang/include/flang/Optimizer/Dialect/FIRDialect.h:45
     mlir::registerDialect<FIROpsDialect>();
+    mlir::registerDialect<mlir::omp::OpenMPDialect>();
     return true;
----------------
kiranchandramohan wrote:
> Are mlir dialects listed in alphabetical order? If so openmp dialect should also be.
Sure,  it's looking a bit odd. I'll correct.


================
Comment at: flang/lib/Lower/OpenMP.cpp:32
+  switch (directive.v) {
+
+  default:
----------------
kiranchandramohan wrote:
> is this line empty as per coding style or accidental?
Accidental I believe. `clang-format` didn't caught it. Removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83659