[PATCH] D90241: [openmp][openacc] Check for duplicate clauses for directive
Kiran Chandramohan via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 28 01:37:38 PDT 2020
kiranchandramohan added inline comments.
================
Comment at: llvm/test/TableGen/directive3.td:30
+// CHECK: error: Clause TDLC_ClauseA already defined on directive TDL_DirA
+// CHECK: error: One or more clauses are defined multiple times on directive TDL_DirA
----------------
Nit: Is this second error giving any additional information? Is this twin errors so that we get all occurrences of errors rather than fail at the first one?
================
Comment at: llvm/utils/TableGen/DirectiveEmitter.cpp:151-162
const auto &DirectiveLanguages =
Records.getAllDerivedDefinitions("DirectiveLanguage");
if (DirectiveLanguages.size() != 1) {
PrintError("A single definition of DirectiveLanguage is needed.");
return;
}
----------------
Is this code from 151 to 162 similar for all occurrences of HasDuplicateClausesInDirectives? If so would it be better to pull it into a function and use that function?
================
Comment at: llvm/utils/TableGen/DirectiveEmitter.cpp:119
+ VersionedClause VerClause{C};
+ if (CrtClauses.find(VerClause.getClause().getName()) == CrtClauses.end()) {
+ CrtClauses.insert(VerClause.getClause().getName());
----------------
clementval wrote:
> kiranchandramohan wrote:
> > Wouldn't this lead to an O(N^2) loop. Can we do better?
> `CrtClauses` is a `StringSet` implemented on the top of `StringMap`. The lookup should be constant (O(1)) so inside the loop we should be O(N).
Ahh, OK. Completely missed that.
Nit: Would using insert and checking the return value be better than using find and insert?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90241/new/
https://reviews.llvm.org/D90241
More information about the llvm-commits
mailing list