[llvm] 2308487 - [openmp] Use switch in isAllowedClauseForDirective instead of multiple if

via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 8 16:55:08 PDT 2020


Author: Valentin Clement
Date: 2020-07-08T19:54:59-04:00
New Revision: 23084878e96cadba4ade809b08229f3ee908aee9

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

LOG: [openmp] Use switch in isAllowedClauseForDirective instead of multiple if

Summary:
Change the test in isAllowedClauseForDirective from if with multiple conditions
to a main switch on directive and then switches on clause for each directive. Version
check is still done with a condition in the return statment.

Reviewers: jdoerfert, jdenny

Reviewed By: jdenny

Subscribers: yaxunl, guansong, sstefan1, llvm-commits

Tags: #llvm

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

Added: 
    

Modified: 
    llvm/test/TableGen/directive1.td
    llvm/test/TableGen/directive2.td
    llvm/utils/TableGen/DirectiveEmitter.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/test/TableGen/directive1.td b/llvm/test/TableGen/directive1.td
index b4d1a6ed2026..43b7ec399b99 100644
--- a/llvm/test/TableGen/directive1.td
+++ b/llvm/test/TableGen/directive1.td
@@ -106,9 +106,17 @@ def TDL_DirA : Directive<"dira"> {
 // IMPL-NEXT:  bool llvm::tdl::isAllowedClauseForDirective(Directive D, Clause C, unsigned Version) {
 // IMPL-NEXT:    assert(unsigned(D) <= llvm::tdl::Directive_enumSize);
 // IMPL-NEXT:    assert(unsigned(C) <= llvm::tdl::Clause_enumSize);
-// IMPL-NEXT:    if (D == TDLD_dira && C == TDLC_clausea && 1 <= Version && 2147483647 >= Version)
-// IMPL-NEXT:      return true;
-// IMPL-NEXT:    if (D == TDLD_dira && C == TDLC_clauseb && 1 <= Version && 2147483647 >= Version)
-// IMPL-NEXT:      return true;
-// IMPL-NEXT:    return false;
+// IMPL-NEXT:    switch (D) {
+// IMPL-NEXT:      case TDLD_dira:
+// IMPL-NEXT:        switch (C) {
+// IMPL-NEXT:          case TDLC_clausea:
+// IMPL-NEXT:            return 1 <= Version && 2147483647 >= Version;
+// IMPL-NEXT:          case TDLC_clauseb:
+// IMPL-NEXT:            return 1 <= Version && 2147483647 >= Version;
+// IMPL-NEXT:          default:
+// IMPL-NEXT:            return false;
+// IMPL-NEXT:        }
+// IMPL-NEXT:        break;
+// IMPL-NEXT:    }
+// IMPL-NEXT:    llvm_unreachable("Invalid Tdl Directive kind");
 // IMPL-NEXT:  }

diff  --git a/llvm/test/TableGen/directive2.td b/llvm/test/TableGen/directive2.td
index 8e180e20df1f..10f48c2a3ceb 100644
--- a/llvm/test/TableGen/directive2.td
+++ b/llvm/test/TableGen/directive2.td
@@ -97,10 +97,17 @@ def TDL_DirA : Directive<"dira"> {
 // IMPL-NEXT:  bool llvm::tdl::isAllowedClauseForDirective(Directive D, Clause C, unsigned Version) {
 // IMPL-NEXT:    assert(unsigned(D) <= llvm::tdl::Directive_enumSize);
 // IMPL-NEXT:    assert(unsigned(C) <= llvm::tdl::Clause_enumSize);
-// IMPL-NEXT:    if (D == TDLD_dira && C == TDLC_clausea && 2 <= Version && 4 >= Version)
-// IMPL-NEXT:      return true;
-// IMPL-NEXT:    if (D == TDLD_dira && C == TDLC_clauseb && 2 <= Version && 2147483647 >= Version)
-// IMPL-NEXT:      return true;
-// IMPL-NEXT:    return false;
+// IMPL-NEXT:    switch (D) {
+// IMPL-NEXT:      case TDLD_dira:
+// IMPL-NEXT:        switch (C) {
+// IMPL-NEXT:          case TDLC_clausea:
+// IMPL-NEXT:            return 2 <= Version && 4 >= Version;
+// IMPL-NEXT:          case TDLC_clauseb:
+// IMPL-NEXT:            return 2 <= Version && 2147483647 >= Version;
+// IMPL-NEXT:          default:
+// IMPL-NEXT:            return false;
+// IMPL-NEXT:        }
+// IMPL-NEXT:        break;
+// IMPL-NEXT:    }
+// IMPL-NEXT:    llvm_unreachable("Invalid Tdl Directive kind");
 // IMPL-NEXT:  }
-

diff  --git a/llvm/utils/TableGen/DirectiveEmitter.cpp b/llvm/utils/TableGen/DirectiveEmitter.cpp
index a9f3569c07a2..d4d2b7965420 100644
--- a/llvm/utils/TableGen/DirectiveEmitter.cpp
+++ b/llvm/utils/TableGen/DirectiveEmitter.cpp
@@ -202,29 +202,27 @@ void GenerateGetKind(const std::vector<Record *> &Records, raw_ostream &OS,
   OS << "}\n";
 }
 
-void GenerateTestForAllowedClauses(const std::vector<Record *> &Clauses,
-                                   raw_ostream &OS, StringRef DirectiveName,
-                                   StringRef DirectivePrefix,
-                                   StringRef ClausePrefix) {
-
-  const auto FormattedDirectiveName = getFormattedName(DirectiveName);
+void GenerateCaseForVersionedClauses(const std::vector<Record *> &Clauses,
+                                     raw_ostream &OS, StringRef DirectiveName,
+                                     StringRef DirectivePrefix,
+                                     StringRef ClausePrefix) {
   for (const auto &C : Clauses) {
     const auto MinVersion = C->getValueAsInt("minVersion");
     const auto MaxVersion = C->getValueAsInt("maxVersion");
     const auto SpecificClause = C->getValueAsDef("clause");
     const auto ClauseName = SpecificClause->getValueAsString("name");
-
-    OS << "  if (D == " << DirectivePrefix << FormattedDirectiveName
-       << " && C == " << ClausePrefix << getFormattedName(ClauseName) << " && "
-       << MinVersion << " <= Version && " << MaxVersion << " >= Version)\n";
-    OS << "    return true;\n";
+    OS << "        case " << ClausePrefix << getFormattedName(ClauseName)
+       << ":\n";
+    OS << "          return " << MinVersion << " <= Version && " << MaxVersion
+       << " >= Version;\n";
   }
 }
 
 // Generate the isAllowedClauseForDirective function implementation.
 void GenerateIsAllowedClause(const std::vector<Record *> &Directives,
-                             raw_ostream &OS, StringRef DirectivePrefix,
-                             StringRef ClausePrefix, StringRef CppNamespace) {
+                             raw_ostream &OS, StringRef LanguageName,
+                             StringRef DirectivePrefix, StringRef ClausePrefix,
+                             StringRef CppNamespace) {
   OS << "\n";
   OS << "bool llvm::" << CppNamespace << "::isAllowedClauseForDirective("
      << "Directive D, Clause C, unsigned Version) {\n";
@@ -233,24 +231,39 @@ void GenerateIsAllowedClause(const std::vector<Record *> &Directives,
   OS << "  assert(unsigned(C) <= llvm::" << CppNamespace
      << "::Clause_enumSize);\n";
 
+  OS << "  switch (D) {\n";
+
   for (const auto &D : Directives) {
+
     const auto DirectiveName = D->getValueAsString("name");
 
+    OS << "    case " << DirectivePrefix << getFormattedName(DirectiveName)
+       << ":\n";
+    OS << "      switch (C) {\n";
+
     const auto &AllowedClauses = D->getValueAsListOfDefs("allowedClauses");
-    GenerateTestForAllowedClauses(AllowedClauses, OS, DirectiveName,
-                                  DirectivePrefix, ClausePrefix);
+    GenerateCaseForVersionedClauses(AllowedClauses, OS, DirectiveName,
+                                    DirectivePrefix, ClausePrefix);
 
     const auto &AllowedOnceClauses =
         D->getValueAsListOfDefs("allowedOnceClauses");
-    GenerateTestForAllowedClauses(AllowedOnceClauses, OS, DirectiveName,
-                                  DirectivePrefix, ClausePrefix);
+    GenerateCaseForVersionedClauses(AllowedOnceClauses, OS, DirectiveName,
+                                    DirectivePrefix, ClausePrefix);
 
     const auto &RequiredClauses = D->getValueAsListOfDefs("requiredClauses");
-    GenerateTestForAllowedClauses(RequiredClauses, OS, DirectiveName,
-                                  DirectivePrefix, ClausePrefix);
+    GenerateCaseForVersionedClauses(RequiredClauses, OS, DirectiveName,
+                                    DirectivePrefix, ClausePrefix);
+
+    OS << "        default:\n";
+    OS << "          return false;\n";
+    OS << "      }\n"; // End of clauses switch
+    OS << "      break;\n";
   }
-  OS << "  return false;\n";
-  OS << "}\n";
+
+  OS << "  }\n"; // End of directives switch
+  OS << "  llvm_unreachable(\"Invalid " << LanguageName
+     << " Directive kind\");\n";
+  OS << "}\n"; // End of function isAllowedClauseForDirective
 }
 
 // Generate the implemenation section for the enumeration in the directive
@@ -291,8 +304,8 @@ void EmitDirectivesImpl(RecordKeeper &Records, raw_ostream &OS) {
   GenerateGetName(Clauses, OS, "Clause", ClausePrefix, LanguageName,
                   CppNamespace);
 
-  GenerateIsAllowedClause(Directives, OS, DirectivePrefix, ClausePrefix,
-                          CppNamespace);
+  GenerateIsAllowedClause(Directives, OS, LanguageName, DirectivePrefix,
+                          ClausePrefix, CppNamespace);
 }
 
 } // namespace llvm


        


More information about the llvm-commits mailing list