[PATCH] D84612: [openmp][openacc] Add wrapper for records in DirectiveEmitter

Valentin Clement via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 28 16:46:27 PDT 2020


clementval added a comment.

In D84612#2180005 <https://reviews.llvm.org/D84612#2180005>, @jdenny wrote:

> Should this patch be marked NFC?
>
> Except for some tiny fixes, LGTM.  Thanks for the changes and the pointer to MLIR.

Right, I'll flag it as NFC.



================
Comment at: llvm/utils/TableGen/DirectiveEmitter.cpp:44
+// Wrapper class that contains DirectiveLanguage's information defined in
+// DirectiveBase.td and provides helper methods for accessing them.
+class DirectiveLanguage {
----------------
jdenny wrote:
> them -> it.  "information" is singular.  Likewise in other comments below.
Thanks pointer. Sorry for my broken English. I'll update that. 


================
Comment at: llvm/utils/TableGen/DirectiveEmitter.cpp:133
+
+// Wrapper class that contains Clase's information defined in DirectiveBase.td
+// and provides helper methods for accessing them.
----------------
jdenny wrote:
> Clase -> Clause
Good catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84612



More information about the llvm-commits mailing list