[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

Joel E. Denny via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 17 14:35:07 PDT 2020


jdenny added a comment.

Thanks for addressing my comments.  As @jdoerfert says, let's give others time to review.



================
Comment at: llvm/test/TableGen/directive1.td:1
+// RUN: llvm-tblgen -gen-directive-decls -I %p/../../include %s | FileCheck %s
+
----------------
I realize that organizing tests into files like `directive1.td` and `directive2.td` is common, and it's probably fine when you have only two test files.  However, I think this is going to become hard to navigate as the number of tests grow.  Even now, I find myself wanting to run diff to understand the significance of these two files.

Can you define these two languages in the same .td file?  That might make sense here given how similar these two are.  Comments can then easily explain the difference between them.

Eventually, I think there should be a `directive` directory and its files should be named to indicate their purpose.

You don't necessarily need to address all this now, but please keep it in mind as this evolves.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81736





More information about the cfe-commits mailing list