[PATCH] D83649: [flang][openacc] OpenACC 3.0 parser

Peter Klausler via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 13 09:39:11 PDT 2020


klausler accepted this revision.
klausler added a comment.
This revision is now accepted and ready to land.

This looks spectacular to me (at least the f18 parts that I reviewed).  Thank you so much for taking on this task and contributing so much value to the project.



================
Comment at: flang/include/flang/Parser/parse-tree.h:3853
+
+// 2.5.13. + | * | max | min | iand | ior | ieor | .and. | .or. | .eqv | .neqv
+struct AccReductionOperator {
----------------
Minor: eqv and neqv are missing their second periods.


================
Comment at: flang/lib/Parser/openacc-parsers.cpp:25
+
+template <typename A> constexpr decltype(auto) verbatim(A x) {
+  return sourced(construct<Verbatim>(x));
----------------
Doesn't OpenMP use something similar?  Put it into token-parsers.h and share it, if so.


================
Comment at: flang/lib/Parser/openacc-parsers.cpp:35
+                    parenthesized(Parser<AccObjectList>{}))) ||
+    "CAPTURE" >> construct<AccClause>(construct<AccClause::Capture>()) ||
+    "BIND" >>
----------------
Extra space at end of line, and not in alphabetic order.


================
Comment at: flang/lib/Parser/openacc-parsers.cpp:87
+            "*" >> construct<std::optional<std::list<Name>>>()))) ||
+    "DTYPE" >>
+        construct<AccClause>(construct<AccClause::DeviceType>(parenthesized(
----------------
Where synonyms like this appear, it would probably be more clear to use something like `("DEVICE_TYPE"_tok || "DTYPE"_tok) >> ...)` to combine the recognition of the synonymous keywords and have just the one following parser.


================
Comment at: flang/lib/Parser/openacc-parsers.cpp:154
+
+/* 2.9 (1609) size-expr is one of:
+ *   *
----------------
We use C++-style comments with `//`, please.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83649





More information about the llvm-commits mailing list