[llvm] c89b645 - [openmp][openacc] Check for duplicate clauses for directive

[via llvm-commits]

Author: Valentin Clement
Date: 2020-10-28T15:12:38-04:00
New Revision: c89b645755418ad22ffae8e347f7e495943367f0

URL: https://github.com/llvm/llvm-project/commit/c89b645755418ad22ffae8e347f7e495943367f0
DIFF: https://github.com/llvm/llvm-project/commit/c89b645755418ad22ffae8e347f7e495943367f0.diff

LOG: [openmp][openacc] Check for duplicate clauses for directive

Check for duplicate clauses associated with directive. Clauses can appear only once
in the 4 lists associated with each directive (allowedClauses, allowedOnceClauses,
allowedExclusiveClauses, requiredClauses). Duplicates were already present (removed with this
patch) or were introduce in new patches by mistake (D89861).

Reviewed By: kiranchandramohan

Differential Revision: https://reviews.llvm.org/D90241

Added: 
    llvm/test/TableGen/directive3.td

Modified: 
    llvm/include/llvm/Frontend/OpenACC/ACC.td
    llvm/include/llvm/TableGen/DirectiveEmitter.h
    llvm/utils/TableGen/DirectiveEmitter.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Frontend/OpenACC/ACC.td b/llvm/include/llvm/Frontend/OpenACC/ACC.td
index 879bbc21b940..0213abc8e9e2 100644
--- a/llvm/include/llvm/Frontend/OpenACC/ACC.td
+++ b/llvm/include/llvm/Frontend/OpenACC/ACC.td
@@ -517,7 +517,6 @@ def ACC_KernelsLoop : Directive<"kernels loop"> {
     VersionedClause<ACCC_Default>,
     VersionedClause<ACCC_Gang>,
     VersionedClause<ACCC_If>,
-    VersionedClause<ACCC_Independent>,
     VersionedClause<ACCC_NumGangs>,
     VersionedClause<ACCC_NumWorkers>,
     VersionedClause<ACCC_Reduction>,

diff  --git a/llvm/include/llvm/TableGen/DirectiveEmitter.h b/llvm/include/llvm/TableGen/DirectiveEmitter.h
index 0964e7b144c8..79c64b2233cd 100644
--- a/llvm/include/llvm/TableGen/DirectiveEmitter.h
+++ b/llvm/include/llvm/TableGen/DirectiveEmitter.h
@@ -73,6 +73,9 @@ class BaseRecord {
 
   bool isDefault() const { return Def->getValueAsBit("isDefault"); }
 
+  // Returns the record name.
+  const StringRef getRecordName() const { return Def->getName(); }
+
 protected:
   const llvm::Record *Def;
 };

diff  --git a/llvm/test/TableGen/directive3.td b/llvm/test/TableGen/directive3.td
new file mode 100644
index 000000000000..6deece1d57dc
--- /dev/null
+++ b/llvm/test/TableGen/directive3.td
@@ -0,0 +1,30 @@
+// RUN: not llvm-tblgen -gen-directive-decl -I %p/../../include %s 2>&1 | FileCheck -match-full-lines %s
+// RUN: not llvm-tblgen -gen-directive-impl -I %p/../../include %s 2>&1 | FileCheck -match-full-lines %s
+// RUN: not llvm-tblgen -gen-directive-gen -I %p/../../include %s 2>&1 | FileCheck -match-full-lines %s
+
+include "llvm/Frontend/Directive/DirectiveBase.td"
+
+def TestDirectiveLanguage : DirectiveLanguage {
+  let name = "TdlError";
+}
+
+def TDLC_ClauseA : Clause<"clausea"> {
+  let isDefault = 1;
+}
+
+def TDLC_ClauseB : Clause<"clauseb"> {
+}
+
+def TDL_DirA : Directive<"dira"> {
+  let allowedClauses = [
+    VersionedClause<TDLC_ClauseA>,
+    VersionedClause<TDLC_ClauseB>
+  ];
+  let allowedOnceClauses = [
+    VersionedClause<TDLC_ClauseA>
+  ];
+  let isDefault = 1;
+}
+
+// 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

diff  --git a/llvm/utils/TableGen/DirectiveEmitter.cpp b/llvm/utils/TableGen/DirectiveEmitter.cpp
index 426a2c76b50e..5e5254d38f1a 100644
--- a/llvm/utils/TableGen/DirectiveEmitter.cpp
+++ b/llvm/utils/TableGen/DirectiveEmitter.cpp
@@ -111,19 +111,66 @@ void GenerateEnumClauseVal(const std::vector<Record *> &Records,
   }
 }
 
-// Generate the declaration section for the enumeration in the directive
-// language
-void EmitDirectivesDecl(RecordKeeper &Records, raw_ostream &OS) {
+bool HasDuplicateClauses(const std::vector<Record *> &Clauses,
+                         const Directive &Directive,
+                         llvm::StringSet<> &CrtClauses) {
+  bool hasError = false;
+  for (const auto &C : Clauses) {
+    VersionedClause VerClause{C};
+    const auto insRes = CrtClauses.insert(VerClause.getClause().getName());
+    if (!insRes.second) {
+      PrintError("Clause " + VerClause.getClause().getRecordName() +
+                 " already defined on directive " + Directive.getRecordName());
+      hasError = true;
+    }
+  }
+  return hasError;
+}
+
+bool HasDuplicateClausesInDirectives(const std::vector<Record *> &Directives) {
+  for (const auto &D : Directives) {
+    Directive Dir{D};
+    llvm::StringSet<> Clauses;
+    if (HasDuplicateClauses(Dir.getAllowedClauses(), Dir, Clauses) ||
+        HasDuplicateClauses(Dir.getAllowedOnceClauses(), Dir, Clauses) ||
+        HasDuplicateClauses(Dir.getAllowedExclusiveClauses(), Dir, Clauses) ||
+        HasDuplicateClauses(Dir.getRequiredClauses(), Dir, Clauses)) {
+      PrintFatalError(
+          "One or more clauses are defined multiple times on directive " +
+          Dir.getRecordName());
+      return true;
+    }
+  }
+  return false;
+}
 
+// Check consitency of records. Return true if an error has been detected.
+// Return false if the records are valid.
+bool CheckRecordsValidity(RecordKeeper &Records) {
   const auto &DirectiveLanguages =
       Records.getAllDerivedDefinitions("DirectiveLanguage");
 
   if (DirectiveLanguages.size() != 1) {
     PrintError("A single definition of DirectiveLanguage is needed.");
-    return;
+    return true;
   }
 
+  const auto &Directives = Records.getAllDerivedDefinitions("Directive");
+  return HasDuplicateClausesInDirectives(Directives);
+}
+
+// Generate the declaration section for the enumeration in the directive
+// language
+void EmitDirectivesDecl(RecordKeeper &Records, raw_ostream &OS) {
+
+  if (CheckRecordsValidity(Records))
+    return;
+
+  const auto &DirectiveLanguages =
+      Records.getAllDerivedDefinitions("DirectiveLanguage");
+
   DirectiveLanguage DirLang{DirectiveLanguages[0]};
+  const auto &Directives = Records.getAllDerivedDefinitions("Directive");
 
   OS << "#ifndef LLVM_" << DirLang.getName() << "_INC\n";
   OS << "#define LLVM_" << DirLang.getName() << "_INC\n";
@@ -145,7 +192,6 @@ void EmitDirectivesDecl(RecordKeeper &Records, raw_ostream &OS) {
     OS << "\nLLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();\n";
 
   // Emit Directive enumeration
-  const auto &Directives = Records.getAllDerivedDefinitions("Directive");
   GenerateEnumClass(Directives, OS, "Directive", DirLang.getDirectivePrefix(),
                     DirLang);
 
@@ -605,15 +651,11 @@ void EmitDirectivesFlangImpl(const std::vector<Record *> &Directives,
 // Generate the implemenation section for the enumeration in the directive
 // language.
 void EmitDirectivesGen(RecordKeeper &Records, raw_ostream &OS) {
+  if (CheckRecordsValidity(Records))
+    return;
 
   const auto &DirectiveLanguages =
       Records.getAllDerivedDefinitions("DirectiveLanguage");
-
-  if (DirectiveLanguages.size() != 1) {
-    PrintError("A single definition of DirectiveLanguage is needed.");
-    return;
-  }
-
   const auto &Directives = Records.getAllDerivedDefinitions("Directive");
   const auto &Clauses = Records.getAllDerivedDefinitions("Clause");
   DirectiveLanguage DirectiveLanguage{DirectiveLanguages[0]};
@@ -623,17 +665,12 @@ void EmitDirectivesGen(RecordKeeper &Records, raw_ostream &OS) {
 // Generate the implemenation for the enumeration in the directive
 // language. This code can be included in library.
 void EmitDirectivesImpl(RecordKeeper &Records, raw_ostream &OS) {
+  if (CheckRecordsValidity(Records))
+    return;
 
   const auto &DirectiveLanguages =
       Records.getAllDerivedDefinitions("DirectiveLanguage");
-
-  if (DirectiveLanguages.size() != 1) {
-    PrintError("A single definition of DirectiveLanguage is needed.");
-    return;
-  }
-
   const auto &Directives = Records.getAllDerivedDefinitions("Directive");
-
   DirectiveLanguage DirLang = DirectiveLanguage{DirectiveLanguages[0]};
 
   const auto &Clauses = Records.getAllDerivedDefinitions("Clause");