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

[clementval]

Summary

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.

[kiranchandramohan]

LGTM.

llvm/utils/TableGen/DirectiveEmitter.cpp

161–168

Nit: Does the return value match the name of this function?

Shouldn't it be like the following?
if records are valid then it is all good (return true).
If records are not valid then it is bad (return false).

[clementval]

llvm/utils/TableGen/DirectiveEmitter.cpp

161–168

Right it is misleading. I'll update that to be consistent between the name and the return value.

[sameeranjoshi]

llvm/utils/TableGen/DirectiveEmitter.cpp

134

nit: - Is clang-tidy suggesting something? if not ignore me on this.

[clementval]

llvm/utils/TableGen/DirectiveEmitter.cpp

134

Good catch. clang-tidy is not followed always in TableGen backends but this one should be. Thanks!

[clementval]

Rebase + address review comments

llvm/include/llvm/Frontend/Directive/DirectiveBase.td

 class Directive<string d> {
   // Name of the directive. Can be composite directive sepearted by whitespace.
   string name = d;
 
   // Define an alternative name return in get<LanguageName>DirectiveName
   // 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 = [];
 
   // List of clauses that are allowed to appear only once.
   list<VersionedClause> allowedOnceClauses = [];
 
   // List of clauses that are allowed but mutually exclusive.
   list<VersionedClause> allowedExclusiveClauses = [];
 
   // List of clauses that are required.
   list<VersionedClause> requiredClauses = [];
 
   // Set directive used by default when unknown.
   bit isDefault = 0;
 }

llvm/test/TableGen/directive3.td

 
 def TDLC_ClauseA : Clause<"clausea"> {
   let isDefault = 1;
 }
 
 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

llvm/utils/TableGen/DirectiveEmitter.cpp

if (!insRes.second) {
       PrintError("Clause " + VerClause.getClause().getRecordName() +
                  " already defined on directive " + Directive.getRecordName());
       hasError = true;
     }
   }
   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;
   }
 
   return HasDuplicateClausesInDirectives(getDirectives());
 }
 
 // Generate the declaration section for the enumeration in the directive
 // language
 void EmitDirectivesDecl(RecordKeeper &Records, raw_ostream &OS) {
   const auto DirLang = DirectiveLanguage{Records};
   if (DirLang.CheckRecordsValidity())
     return;