[llvm] a8a10ac - [openacc][openmp] Allow duplicate between required and allowed once/exclusive

[via llvm-commits]

Author: Valentin Clement
Date: 2020-11-05T16:21:26-05:00
New Revision: a8a10acba2a769ae0f77e61380e649e7428f68fb

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

LOG: [openacc][openmp] Allow duplicate between required and allowed once/exclusive

Validity check introduce in D90241 are a bit too restrict and this patch propose to losen
them a bit. The duplicate clauses is now check only between the three allowed lists and between the
requiredClauses and allowedClauses lists. This allows to enable some check where a clause can be
required but also appear only once on the directive. We found these kind of restriction useful
on the set directive in OpenACC for example.

Reviewed By: kiranchandramohan

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

Added: 
    

Modified: 
    llvm/include/llvm/Frontend/Directive/DirectiveBase.td
    llvm/test/TableGen/directive3.td
    llvm/utils/TableGen/DirectiveEmitter.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Frontend/Directive/DirectiveBase.td b/llvm/include/llvm/Frontend/Directive/DirectiveBase.td
index aa415b3f0abc..95514f05afbe 100644
--- a/llvm/include/llvm/Frontend/Directive/DirectiveBase.td
+++ b/llvm/include/llvm/Frontend/Directive/DirectiveBase.td
@@ -131,6 +131,10 @@ class Directive<string d> {
   // function.
   string alternativeName = "";
 
+  // Clauses cannot appear twice in the three allowed lists below. Also, since
+  // required implies allowed, the same clause cannot appear in both the
+  // allowedClauses and requiredClauses lists.
+
   // List of allowed clauses for the directive.
   list<VersionedClause> allowedClauses = [];
 

diff  --git a/llvm/test/TableGen/directive3.td b/llvm/test/TableGen/directive3.td
index 6deece1d57dc..8af4594f2994 100644
--- a/llvm/test/TableGen/directive3.td
+++ b/llvm/test/TableGen/directive3.td
@@ -15,16 +15,29 @@ def TDLC_ClauseA : Clause<"clausea"> {
 def TDLC_ClauseB : Clause<"clauseb"> {
 }
 
+def TDLC_ClauseC : Clause<"clausec"> {
+}
+
+def TDLC_ClauseD : Clause<"claused"> {
+}
+
 def TDL_DirA : Directive<"dira"> {
   let allowedClauses = [
     VersionedClause<TDLC_ClauseA>,
-    VersionedClause<TDLC_ClauseB>
+    VersionedClause<TDLC_ClauseB>,
+    VersionedClause<TDLC_ClauseD>
   ];
   let allowedOnceClauses = [
-    VersionedClause<TDLC_ClauseA>
+    VersionedClause<TDLC_ClauseA>,
+    VersionedClause<TDLC_ClauseC>
+  ];
+  let requiredClauses = [
+    VersionedClause<TDLC_ClauseC>,
+    VersionedClause<TDLC_ClauseD>
   ];
   let isDefault = 1;
 }
 
 // CHECK: error: Clause TDLC_ClauseA already defined on directive TDL_DirA
+// CHECK: error: Clause TDLC_ClauseD 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 2b77c8f81ad2..9a4b3bf17e37 100644
--- a/llvm/utils/TableGen/DirectiveEmitter.cpp
+++ b/llvm/utils/TableGen/DirectiveEmitter.cpp
@@ -127,28 +127,40 @@ bool HasDuplicateClauses(const std::vector<Record *> &Clauses,
   return hasError;
 }
 
+// Check for duplicate clauses in lists. Clauses cannot appear twice in the
+// three allowed list. Also, since required implies allowed, clauses cannot
+// appear in both the allowedClauses and requiredClauses lists.
 bool HasDuplicateClausesInDirectives(const std::vector<Record *> &Directives) {
+  bool hasDuplicate = false;
   for (const auto &D : Directives) {
     Directive Dir{D};
     llvm::StringSet<> Clauses;
+    // Check for duplicates in the three allowed lists.
     if (HasDuplicateClauses(Dir.getAllowedClauses(), Dir, Clauses) ||
         HasDuplicateClauses(Dir.getAllowedOnceClauses(), Dir, Clauses) ||
-        HasDuplicateClauses(Dir.getAllowedExclusiveClauses(), Dir, Clauses) ||
+        HasDuplicateClauses(Dir.getAllowedExclusiveClauses(), Dir, Clauses)) {
+      hasDuplicate = true;
+    }
+    // Check for duplicate between allowedClauses and required
+    Clauses.clear();
+    if (HasDuplicateClauses(Dir.getAllowedClauses(), Dir, Clauses) ||
         HasDuplicateClauses(Dir.getRequiredClauses(), Dir, Clauses)) {
-      PrintFatalError(
-          "One or more clauses are defined multiple times on directive " +
-          Dir.getRecordName());
-      return true;
+      hasDuplicate = true;
     }
+    if (hasDuplicate)
+      PrintFatalError("One or more clauses are defined multiple times on"
+                      " directive " +
+                      Dir.getRecordName());
   }
-  return false;
+
+  return hasDuplicate;
 }
 
 // Check consitency of records. Return true if an error has been detected.
 // Return false if the records are valid.
 bool DirectiveLanguage::CheckRecordsValidity() const {
   if (getDirectiveLanguages().size() != 1) {
-    PrintError("A single definition of DirectiveLanguage is needed.");
+    PrintFatalError("A single definition of DirectiveLanguage is needed.");
     return true;
   }