[Frontend] Add leaf constructs and association to OpenMP/ACC directives #83625

kparzysz · 2024-03-01T21:52:18Z

Add members "leafConstructs" and "association" to .td describing OpenMP/ACC directives. The naming follows the terminology used in the OpenMP standard: a "leaf" construct is a construct that is itself not a composition or a combination of other constructs, and "association" is the source language construct to which the directive applies (e.g. loop, block, etc.)

The tblgen-generated output then contains two additional functions

getLeafConstructs(D), and
getDirectiveAssociation(D) plus "enum class Association", all in namespaces "llvm::omp" and "llvm::acc".

Note: getLeafConstructs returns an empty sequence for a construct that is itself a leaf construct.

Use the new functions to simplify a few OpenMP-related functions in clang.

llvmbot · 2024-03-01T21:52:48Z

@llvm/pr-subscribers-openacc
@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-clang

Author: Krzysztof Parzyszek (kparzysz)

Changes

Add members "leafs" and "association" to .td describing OpenMP/ACC directives: "leafs" are the leaf constructs for composite/combined constructs, and "association" is the source language construct to which the directive applies (e.g. loop, block, etc.)

The tblgen-generated output then contains two additional functions

getLeafConstructs(D), and
getDirectiveAssociation(D) plus "enum class Association", all in namespaces "llvm::omp" and "llvm::acc".

Note: getLeafConstructs returns an empty sequence for a construct that is itself a leaf construct.

Use the new functions to simplify a few OpenMP-related functions in clang.

Patch is 53.90 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/83625.diff

6 Files Affected:

(modified) clang/lib/Basic/OpenMPKinds.cpp (+34-96)
(modified) llvm/include/llvm/Frontend/Directive/DirectiveBase.td (+36)
(modified) llvm/include/llvm/Frontend/OpenACC/ACC.td (+25-2)
(modified) llvm/include/llvm/Frontend/OpenMP/OMP.td (+152-20)
(modified) llvm/include/llvm/TableGen/DirectiveEmitter.h (+12)
(modified) llvm/utils/TableGen/DirectiveEmitter.cpp (+232-4)

diff --git a/clang/lib/Basic/OpenMPKinds.cpp b/clang/lib/Basic/OpenMPKinds.cpp
index 6c31b0824eb8a4..dd1a096d178111 100644
--- a/clang/lib/Basic/OpenMPKinds.cpp
+++ b/clang/lib/Basic/OpenMPKinds.cpp
@@ -574,31 +574,7 @@ const char *clang::getOpenMPSimpleClauseTypeName(OpenMPClauseKind Kind,
 }
 
 bool clang::isOpenMPLoopDirective(OpenMPDirectiveKind DKind) {
-  return DKind == OMPD_simd || DKind == OMPD_for || DKind == OMPD_for_simd ||
-         DKind == OMPD_parallel_for || DKind == OMPD_parallel_for_simd ||
-         DKind == OMPD_taskloop || DKind == OMPD_taskloop_simd ||
-         DKind == OMPD_master_taskloop || DKind == OMPD_master_taskloop_simd ||
-         DKind == OMPD_parallel_master_taskloop ||
-         DKind == OMPD_parallel_master_taskloop_simd ||
-         DKind == OMPD_masked_taskloop || DKind == OMPD_masked_taskloop_simd ||
-         DKind == OMPD_parallel_masked_taskloop || DKind == OMPD_distribute ||
-         DKind == OMPD_parallel_masked_taskloop_simd ||
-         DKind == OMPD_target_parallel_for ||
-         DKind == OMPD_distribute_parallel_for ||
-         DKind == OMPD_distribute_parallel_for_simd ||
-         DKind == OMPD_distribute_simd ||
-         DKind == OMPD_target_parallel_for_simd || DKind == OMPD_target_simd ||
-         DKind == OMPD_teams_distribute ||
-         DKind == OMPD_teams_distribute_simd ||
-         DKind == OMPD_teams_distribute_parallel_for_simd ||
-         DKind == OMPD_teams_distribute_parallel_for ||
-         DKind == OMPD_target_teams_distribute ||
-         DKind == OMPD_target_teams_distribute_parallel_for ||
-         DKind == OMPD_target_teams_distribute_parallel_for_simd ||
-         DKind == OMPD_target_teams_distribute_simd || DKind == OMPD_tile ||
-         DKind == OMPD_unroll || DKind == OMPD_loop ||
-         DKind == OMPD_teams_loop || DKind == OMPD_target_teams_loop ||
-         DKind == OMPD_parallel_loop || DKind == OMPD_target_parallel_loop;
+  return getDirectiveAssociation(DKind) == Association::Loop;
 }
 
 bool clang::isOpenMPWorksharingDirective(OpenMPDirectiveKind DKind) {
@@ -619,44 +595,22 @@ bool clang::isOpenMPWorksharingDirective(OpenMPDirectiveKind DKind) {
 }
 
 bool clang::isOpenMPTaskLoopDirective(OpenMPDirectiveKind DKind) {
-  return DKind == OMPD_taskloop || DKind == OMPD_taskloop_simd ||
-         DKind == OMPD_master_taskloop || DKind == OMPD_master_taskloop_simd ||
-         DKind == OMPD_parallel_master_taskloop ||
-         DKind == OMPD_masked_taskloop || DKind == OMPD_masked_taskloop_simd ||
-         DKind == OMPD_parallel_masked_taskloop ||
-         DKind == OMPD_parallel_masked_taskloop_simd ||
-         DKind == OMPD_parallel_master_taskloop_simd;
+  return DKind == OMPD_taskloop ||
+         llvm::is_contained(getLeafConstructs(DKind), OMPD_taskloop);
 }
 
 bool clang::isOpenMPParallelDirective(OpenMPDirectiveKind DKind) {
-  return DKind == OMPD_parallel || DKind == OMPD_parallel_for ||
-         DKind == OMPD_parallel_for_simd || DKind == OMPD_parallel_sections ||
-         DKind == OMPD_target_parallel || DKind == OMPD_target_parallel_for ||
-         DKind == OMPD_distribute_parallel_for ||
-         DKind == OMPD_distribute_parallel_for_simd ||
-         DKind == OMPD_target_parallel_for_simd ||
-         DKind == OMPD_teams_distribute_parallel_for ||
-         DKind == OMPD_teams_distribute_parallel_for_simd ||
-         DKind == OMPD_target_teams_distribute_parallel_for ||
-         DKind == OMPD_target_teams_distribute_parallel_for_simd ||
-         DKind == OMPD_parallel_master || DKind == OMPD_parallel_masked ||
-         DKind == OMPD_parallel_master_taskloop ||
-         DKind == OMPD_parallel_master_taskloop_simd ||
-         DKind == OMPD_parallel_masked_taskloop ||
-         DKind == OMPD_parallel_masked_taskloop_simd ||
-         DKind == OMPD_parallel_loop || DKind == OMPD_target_parallel_loop ||
-         DKind == OMPD_teams_loop;
+  if (DKind == OMPD_parallel_workshare)
+    return false;
+  if (DKind == OMPD_teams_loop)
+    return true;
+  return DKind == OMPD_parallel ||
+         llvm::is_contained(getLeafConstructs(DKind), OMPD_parallel);
 }
 
 bool clang::isOpenMPTargetExecutionDirective(OpenMPDirectiveKind DKind) {
-  return DKind == OMPD_target || DKind == OMPD_target_parallel ||
-         DKind == OMPD_target_parallel_for ||
-         DKind == OMPD_target_parallel_for_simd || DKind == OMPD_target_simd ||
-         DKind == OMPD_target_teams || DKind == OMPD_target_teams_distribute ||
-         DKind == OMPD_target_teams_distribute_parallel_for ||
-         DKind == OMPD_target_teams_distribute_parallel_for_simd ||
-         DKind == OMPD_target_teams_distribute_simd ||
-         DKind == OMPD_target_teams_loop || DKind == OMPD_target_parallel_loop;
+  return DKind == OMPD_target ||
+         llvm::is_contained(getLeafConstructs(DKind), OMPD_target);
 }
 
 bool clang::isOpenMPTargetDataManagementDirective(OpenMPDirectiveKind DKind) {
@@ -665,60 +619,44 @@ bool clang::isOpenMPTargetDataManagementDirective(OpenMPDirectiveKind DKind) {
 }
 
 bool clang::isOpenMPNestingTeamsDirective(OpenMPDirectiveKind DKind) {
-  return DKind == OMPD_teams || DKind == OMPD_teams_distribute ||
-         DKind == OMPD_teams_distribute_simd ||
-         DKind == OMPD_teams_distribute_parallel_for_simd ||
-         DKind == OMPD_teams_distribute_parallel_for ||
-         DKind == OMPD_teams_loop;
+  if (DKind == OMPD_teams)
+    return true;
+  auto leafs = getLeafConstructs(DKind);
+  return !leafs.empty() && leafs.front() == OMPD_teams;
 }
 
 bool clang::isOpenMPTeamsDirective(OpenMPDirectiveKind DKind) {
-  return isOpenMPNestingTeamsDirective(DKind) || DKind == OMPD_target_teams ||
-         DKind == OMPD_target_teams_distribute ||
-         DKind == OMPD_target_teams_distribute_parallel_for ||
-         DKind == OMPD_target_teams_distribute_parallel_for_simd ||
-         DKind == OMPD_target_teams_distribute_simd ||
-         DKind == OMPD_target_teams_loop;
+  return DKind == OMPD_teams ||
+         llvm::is_contained(getLeafConstructs(DKind), OMPD_teams);
 }
 
 bool clang::isOpenMPSimdDirective(OpenMPDirectiveKind DKind) {
-  return DKind == OMPD_simd || DKind == OMPD_for_simd ||
-         DKind == OMPD_parallel_for_simd || DKind == OMPD_taskloop_simd ||
-         DKind == OMPD_master_taskloop_simd ||
-         DKind == OMPD_masked_taskloop_simd ||
-         DKind == OMPD_parallel_master_taskloop_simd ||
-         DKind == OMPD_parallel_masked_taskloop_simd ||
-         DKind == OMPD_distribute_parallel_for_simd ||
-         DKind == OMPD_distribute_simd || DKind == OMPD_target_simd ||
-         DKind == OMPD_teams_distribute_simd ||
-         DKind == OMPD_teams_distribute_parallel_for_simd ||
-         DKind == OMPD_target_teams_distribute_parallel_for_simd ||
-         DKind == OMPD_target_teams_distribute_simd ||
-         DKind == OMPD_target_parallel_for_simd;
+  // Avoid OMPD_declare_simd
+  if (DKind == OMPD_end_do_simd ||
+      getDirectiveAssociation(DKind) != Association::Loop)
+    return false;
+
+  return DKind == OMPD_simd ||
+         llvm::is_contained(getLeafConstructs(DKind), OMPD_simd);
 }
 
 bool clang::isOpenMPNestingDistributeDirective(OpenMPDirectiveKind Kind) {
-  return Kind == OMPD_distribute || Kind == OMPD_distribute_parallel_for ||
-         Kind == OMPD_distribute_parallel_for_simd ||
-         Kind == OMPD_distribute_simd;
-  // TODO add next directives.
+  if (Kind == OMPD_distribute)
+    return true;
+  auto leafs = getLeafConstructs(Kind);
+  return !leafs.empty() && leafs.front() == OMPD_distribute;
 }
 
 bool clang::isOpenMPDistributeDirective(OpenMPDirectiveKind Kind) {
-  return isOpenMPNestingDistributeDirective(Kind) ||
-         Kind == OMPD_teams_distribute || Kind == OMPD_teams_distribute_simd ||
-         Kind == OMPD_teams_distribute_parallel_for_simd ||
-         Kind == OMPD_teams_distribute_parallel_for ||
-         Kind == OMPD_target_teams_distribute ||
-         Kind == OMPD_target_teams_distribute_parallel_for ||
-         Kind == OMPD_target_teams_distribute_parallel_for_simd ||
-         Kind == OMPD_target_teams_distribute_simd;
+  return Kind == OMPD_distribute ||
+         llvm::is_contained(getLeafConstructs(Kind), OMPD_distribute);
 }
 
 bool clang::isOpenMPGenericLoopDirective(OpenMPDirectiveKind Kind) {
-  return Kind == OMPD_loop || Kind == OMPD_teams_loop ||
-         Kind == OMPD_target_teams_loop || Kind == OMPD_parallel_loop ||
-         Kind == OMPD_target_parallel_loop;
+  if (Kind == OMPD_loop)
+    return true;
+  auto leafs = getLeafConstructs(Kind);
+  return !leafs.empty() && leafs.back() == OMPD_loop;
 }
 
 bool clang::isOpenMPPrivate(OpenMPClauseKind Kind) {
diff --git a/llvm/include/llvm/Frontend/Directive/DirectiveBase.td b/llvm/include/llvm/Frontend/Directive/DirectiveBase.td
index 31578710365b21..4d5201aabe8afb 100644
--- a/llvm/include/llvm/Frontend/Directive/DirectiveBase.td
+++ b/llvm/include/llvm/Frontend/Directive/DirectiveBase.td
@@ -127,6 +127,35 @@ class VersionedClause<Clause c, int min = 1, int max = 0x7FFFFFFF> {
   int maxVersion = max;
 }
 
+// Kinds of directive associations.
+class Association<string n> {
+  string name = n;  // Name of the enum value in enum class Association.
+}
+// All of the AS_Xyz names are recognized by TableGen in order to calculate
+// the association in the AS_FromLeafs case.
+def AS_None : Association<"None"> {}              // No association
+def AS_Block : Association<"Block"> {}            // Block (incl. single
+                                                  // statement)
+def AS_Declaration : Association<"Declaration"> {}   // Declaration
+def AS_Delimited : Association<"Delimited"> {}    // Region delimited with
+                                                  // begin/end
+def AS_Loop : Association<"Loop"> {}              // Loop
+def AS_Separating : Association<"Separating"> {}  // Separates parts of a
+                                                  // construct
+
+def AS_FromLeafs : Association<"FromLeafs"> {}    // See below
+// AS_FromLeafs can be used for combined/composite directives, and the actual
+// association will be computed based on associations of the leaf constructs:
+//   (x + y) + z = x + (y + z)
+//   x + y = y + x
+//   x + x = x
+//   AS_None + x = x
+//   AS_Block + AS_Loop = AS_Loop
+// Other combinations are not allowed.
+// This association is not valid for leaf constructs.
+// The name "AS_FromLeafs" is recognized by TableGen, and there is no enum
+// generated for it.
+
 // Information about a specific directive.
 class Directive<string d> {
   // Name of the directive. Can be composite directive sepearted by whitespace.
@@ -152,6 +181,13 @@ class Directive<string d> {
   // List of clauses that are required.
   list<VersionedClause> requiredClauses = [];
 
+  // List of leaf constituent directives in the order in which they appear
+  // in the combined/composite directive.
+  list<Directive> leafs = [];
+
   // Set directive used by default when unknown.
   bit isDefault = false;
+
+  // What the directive is associated with.
+  Association association = AS_FromLeafs;
 }
diff --git a/llvm/include/llvm/Frontend/OpenACC/ACC.td b/llvm/include/llvm/Frontend/OpenACC/ACC.td
index 0dbd934d83f064..3c8cc1af0518b1 100644
--- a/llvm/include/llvm/Frontend/OpenACC/ACC.td
+++ b/llvm/include/llvm/Frontend/OpenACC/ACC.td
@@ -266,7 +266,9 @@ def ACCC_Unknown : Clause<"unknown"> {
 //===----------------------------------------------------------------------===//
 
 // 2.12
-def ACC_Atomic : Directive<"atomic"> {}
+def ACC_Atomic : Directive<"atomic"> {
+  let association = AS_Block;
+}
 
 // 2.6.5
 def ACC_Data : Directive<"data"> {
@@ -290,6 +292,7 @@ def ACC_Data : Directive<"data"> {
     VersionedClause<ACCC_NoCreate>,
     VersionedClause<ACCC_Present>
   ];
+  let association = AS_Block;
 }
 
 // 2.13
@@ -304,6 +307,7 @@ def ACC_Declare : Directive<"declare"> {
     VersionedClause<ACCC_DeviceResident>,
     VersionedClause<ACCC_Link>
   ];
+  let association = AS_None;
 }
 
 // 2.5.3
@@ -329,6 +333,7 @@ def ACC_Kernels : Directive<"kernels"> {
     VersionedClause<ACCC_Self>,
     VersionedClause<ACCC_VectorLength>
   ];
+  let association = AS_Block;
 }
 
 // 2.5.1
@@ -357,6 +362,7 @@ def ACC_Parallel : Directive<"parallel"> {
     VersionedClause<ACCC_If>,
     VersionedClause<ACCC_Self>
   ];
+  let association = AS_Block;
 }
 
 // 2.5.2
@@ -384,6 +390,7 @@ def ACC_Serial : Directive<"serial"> {
     VersionedClause<ACCC_If>,
     VersionedClause<ACCC_Self>
   ];
+  let association = AS_Block;
 }
 
 // 2.9
@@ -403,10 +410,13 @@ def ACC_Loop : Directive<"loop"> {
     VersionedClause<ACCC_Independent>,
     VersionedClause<ACCC_Seq>
   ];
+  let association = AS_Loop;
 }
 
 // 2.10
-def ACC_Cache : Directive<"cache"> {}
+def ACC_Cache : Directive<"cache"> {
+  let association = AS_None;
+}
 
 // 2.14.1
 def ACC_Init : Directive<"init"> {
@@ -415,6 +425,7 @@ def ACC_Init : Directive<"init"> {
     VersionedClause<ACCC_DeviceType>,
     VersionedClause<ACCC_If>
   ];
+  let association = AS_None;
 }
 
 // 2.15.1
@@ -430,6 +441,7 @@ def ACC_Routine : Directive<"routine"> {
   let allowedOnceClauses = [
     VersionedClause<ACCC_NoHost>
   ];
+  let association = AS_Declaration;
 }
 
 // 2.14.3
@@ -448,6 +460,7 @@ def ACC_Set : Directive<"set"> {
     VersionedClause<ACCC_DeviceNum>,
     VersionedClause<ACCC_DeviceType>
   ];
+  let association = AS_None;
 }
 
 // 2.14.2
@@ -457,6 +470,7 @@ def ACC_Shutdown : Directive<"shutdown"> {
     VersionedClause<ACCC_DeviceType>,
     VersionedClause<ACCC_If>
   ];
+  let association = AS_None;
 }
 
 // 2.14.4
@@ -475,6 +489,7 @@ def ACC_Update : Directive<"update"> {
     VersionedClause<ACCC_Host>,
     VersionedClause<ACCC_Self>
   ];
+  let association = AS_None;
 }
 
 // 2.16.3
@@ -483,6 +498,7 @@ def ACC_Wait : Directive<"wait"> {
     VersionedClause<ACCC_Async>,
     VersionedClause<ACCC_If>
   ];
+  let association = AS_None;
 }
 
 // 2.14.6
@@ -499,6 +515,7 @@ def ACC_EnterData : Directive<"enter data"> {
     VersionedClause<ACCC_Create>,
     VersionedClause<ACCC_Copyin>
   ];
+  let association = AS_None;
 }
 
 // 2.14.7
@@ -516,6 +533,7 @@ def ACC_ExitData : Directive<"exit data"> {
     VersionedClause<ACCC_Delete>,
     VersionedClause<ACCC_Detach>
   ];
+  let association = AS_None;
 }
 
 // 2.8
@@ -527,6 +545,7 @@ def ACC_HostData : Directive<"host_data"> {
   let requiredClauses = [
     VersionedClause<ACCC_UseDevice>
   ];
+  let association = AS_Block;
 }
 
 // 2.11
@@ -564,6 +583,7 @@ def ACC_KernelsLoop : Directive<"kernels loop"> {
     VersionedClause<ACCC_Independent>,
     VersionedClause<ACCC_Seq>
   ];
+  let leafs = [ACC_Kernels, ACC_Loop];
 }
 
 // 2.11
@@ -602,6 +622,7 @@ def ACC_ParallelLoop : Directive<"parallel loop"> {
     VersionedClause<ACCC_Independent>,
     VersionedClause<ACCC_Seq>
   ];
+  let leafs = [ACC_Parallel, ACC_Loop];
 }
 
 // 2.11
@@ -637,8 +658,10 @@ def ACC_SerialLoop : Directive<"serial loop"> {
     VersionedClause<ACCC_Independent>,
     VersionedClause<ACCC_Seq>
   ];
+  let leafs = [ACC_Serial, ACC_Loop];
 }
 
 def ACC_Unknown : Directive<"unknown"> {
   let isDefault = true;
+  let association = AS_None;
 }
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index 77d207f2b10a83..fecc49e888a454 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -463,7 +463,9 @@ def OMPC_OMX_Bare : Clause<"ompx_bare"> {
 // Definition of OpenMP directives
 //===----------------------------------------------------------------------===//
 
-def OMP_ThreadPrivate : Directive<"threadprivate"> {}
+def OMP_ThreadPrivate : Directive<"threadprivate"> {
+  let association = AS_None;
+}
 def OMP_Parallel : Directive<"parallel"> {
   let allowedClauses = [
     VersionedClause<OMPC_Private>,
@@ -480,6 +482,7 @@ def OMP_Parallel : Directive<"parallel"> {
     VersionedClause<OMPC_NumThreads>,
     VersionedClause<OMPC_ProcBind>,
   ];
+  let association = AS_Block;
 }
 def OMP_Task : Directive<"task"> {
   let allowedClauses = [
@@ -500,6 +503,7 @@ def OMP_Task : Directive<"task"> {
     VersionedClause<OMPC_Final>,
     VersionedClause<OMPC_Priority>
   ];
+  let association = AS_Block;
 }
 def OMP_Simd : Directive<"simd"> {
   let allowedClauses = [
@@ -518,17 +522,20 @@ def OMP_Simd : Directive<"simd"> {
     VersionedClause<OMPC_If, 50>,
     VersionedClause<OMPC_Order, 50>
   ];
+  let association = AS_Loop;
 }
 def OMP_Tile : Directive<"tile"> {
   let allowedOnceClauses = [
     VersionedClause<OMPC_Sizes, 51>,
   ];
+  let association = AS_Loop;
 }
 def OMP_Unroll : Directive<"unroll"> {
   let allowedOnceClauses = [
     VersionedClause<OMPC_Full, 51>,
     VersionedClause<OMPC_Partial, 51>,
   ];
+  let association = AS_Loop;
 }
 def OMP_For : Directive<"for"> {
   let allowedClauses = [
@@ -544,6 +551,7 @@ def OMP_For : Directive<"for"> {
     VersionedClause<OMPC_Allocate>,
     VersionedClause<OMPC_Order, 50>
   ];
+  let association = AS_Loop;
 }
 def OMP_Do : Directive<"do"> {
   let allowedClauses = [
@@ -560,6 +568,7 @@ def OMP_Do : Directive<"do"> {
     VersionedClause<OMPC_NoWait>,
     VersionedClause<OMPC_Order, 50>
   ];
+  let association = AS_Loop;
 }
 def OMP_Sections : Directive<"sections"> {
   let allowedClauses = [
@@ -570,8 +579,11 @@ def OMP_Sections : Directive<"sections"> {
     VersionedClause<OMPC_NoWait>,
     VersionedClause<OMPC_Allocate>
   ];
+  let association = AS_Block;
+}
+def OMP_Section : Directive<"section"> {
+  let association = AS_Separating;
 }
-def OMP_Section : Directive<"section"> {}
 def OMP_Single : Directive<"single"> {
   let allowedClauses = [
     VersionedClause<OMPC_Private>,
@@ -580,33 +592,44 @@ def OMP_Single : Directive<"single"> {
     VersionedClause<OMPC_NoWait>,
     VersionedClause<OMPC_Allocate>
   ];
+  let association = AS_Block;
+}
+def OMP_Master : Directive<"master"> {
+  let association = AS_Block;
 }
-def OMP_Master : Directive<"master"> {}
 def OMP_Critical : Directive<"critical"> {
   let allowedClauses = [
     VersionedClause<OMPC_Hint>
   ];
+  let association = AS_Block;
+}
+def OMP_TaskYield : Directive<"taskyield"> {
+  let association = AS_None;
+}
+def OMP_Barrier : Directive<"barrier"> {
+  let association = AS_None;
 }
-def OMP_TaskYield : Directive<"taskyield"> {}
-def OMP_Barrier : Directive<"barrier"> {}
 def OMP_Error : Directive<"error"> {
   let allowedClauses = [
     VersionedClause<OMPC_At, 51>,
     VersionedClause<OMPC_Severity, 51>,
     VersionedClause<OMPC_Message, 51>
   ];
+  let association = AS_None;
 }
 def OMP_TaskWait : Directive<"taskwait"> {
   let allowedClauses = [
     VersionedClause<OMPC_Depend, 50>,
     VersionedClause<OMPC_NoWait, 51>
   ];
+  let association = AS_None;
 }
 def OMP_TaskGroup : Directive<"taskgroup"> {
   let allowedClauses = [
     VersionedClause<OMPC_TaskReduction, 50>,
     VersionedClause<OMPC_Allocate, 50>
   ];
+  let association = AS_Block;
 }
 def OMP_Flush : Directive<"flush"> {
   let allowedOnceClauses = [
@@ -617,6 +640,7 @@ def OMP_Flush : Directive<"flush"> {
     // OMPKinds.def.
     VersionedClause<OMPC_Flush>
   ];
+  let association = AS_None;
 }
 def OMP_Ordered : Directive<"ordered"> {
   let allowedClauses = [
@@ -627,6 +651,8 @@ def OMP_Ordered : Directive<"ordered"> {
     VersionedClause<OMPC_Threads>,
     VersionedClause<OMPC_Simd>
   ];
+  let association = AS_None;
+  // There is also a block-associated "ordered" directive.
 }
 def OMP_Atomic : Directive<"atomic"> {
   let allowedClauses = [
@@ -646,6 +672,7 @@ def OMP_Atomic : Directive<"atomic"> {
     VersionedClause<OMPC_Fail, 51>,
     VersionedClause<OMPC_Weak, 51>
   ];
+  let association = AS_Block;
 }
 def OMP_Target : Directive<"target"> {
   let allowedClauses = [
@@ -669,6 +696,7 @@ def OMP_Target : Directive<"target"> {
     VersionedClause<OMPC_NoWait>,
     VersionedClause<OMPC_OMPX_DynCGroupMem>,
   ];
+  let association = AS_Block;
 }
 def OMP_Teams : Directive<"teams"> {
   let allowedClauses = [
@@ -685,11 +713,13 @@ def OMP_Teams : Directive<"teams"> {
     VersionedClause<OMPC_NumTeams>,
     VersionedClause<OMPC_ThreadLimit>
   ];
+  let association = AS_Block;
 }
 def OMP_Cancel : Directive<"cancel"> {
   let allowedOnceClauses = [
     VersionedClause<OMPC_If>
   ];
+  let association = AS_None;
 }
 def OMP_Req...
[truncated]

github-actions · 2024-03-01T21:54:48Z

✅ With the latest revision this PR passed the C/C++ code formatter.

clang/lib/Basic/OpenMPKinds.cpp

Add members "leafs" and "association" to .td describing OpenMP/ACC directives: "leafs" are the leaf constructs for composite/combined constructs, and "association" is the source language construct to which the directive applies (e.g. loop, block, etc.) The tblgen-generated output then contains two additional functions - getLeafConstructs(D), and - getDirectiveAssociation(D) plus "enum class Association", all in namespaces "llvm::omp" and "llvm::acc". Note: getLeafConstructs returns an empty sequence for a construct that is itself a leaf construct. Use the new functions to simplify a few OpenMP-related functions in clang.

kparzysz · 2024-03-01T22:06:48Z

I removed the check for end do simd with a comment that it can't appear in clang. ~~I think you deleted that remark, let me know if I should bring the check back.~~
Nvm: I can see the comment now.

clementval

Looks good to me from the OpenACC point of view. Someone from OpenMP should approve as well.

kparzysz · 2024-03-02T14:18:37Z

The failing test on Windows is unrelated. I've seen it fail in other builds before.

kiranchandramohan

Add members "leafs" and "association" to .td describing OpenMP/ACC directives: "leafs" are the leaf constructs for composite/combined constructs, and "association" is the source language construct to which the directive applies (e.g. loop, block, etc.)

Could you clarify that these are terminology used in the OpenMP standard?

Great work @kparzysz.

kiranchandramohan · 2024-03-03T16:08:27Z

clang/lib/Basic/OpenMPKinds.cpp

+  if (DKind == OMPD_parallel_workshare)
+    return false;


Nit: A comment that workshare is not applicable to C/C++ might be helpful.

I removed the check. Formally, it does fall under the "parallel" family of directives, so I think we shouldn't exclude it on the basis that it will never show up in clang.

clang/lib/Basic/OpenMPKinds.cpp

alexey-bataev · 2024-03-03T23:04:33Z

llvm/test/TableGen/directive1.td

@@ -341,5 +356,19 @@ def TDL_DirA : Directive<"dira"> {
 // IMPL-NEXT:    }
 // IMPL-NEXT:    llvm_unreachable("Invalid Tdl Directive kind");
 // IMPL-NEXT:  }
+// IMPL-NEXT:  const llvm::SmallVector<llvm::tdl::Directive> &llvm::tdl::getLeafConstructs(llvm::tdl::Directive Dir) {


Suggested change

// IMPL-NEXT: const llvm::SmallVector<llvm::tdl::Directive> &llvm::tdl::getLeafConstructs(llvm::tdl::Directive Dir) {

// IMPL-NEXT: ArrayRef<llvm::tdl::Directive> llvm::tdl::getLeafConstructs(llvm::tdl::Directive Dir) {

alexey-bataev · 2024-03-03T23:05:26Z

llvm/utils/TableGen/DirectiveEmitter.cpp

@@ -231,6 +244,8 @@ static void EmitDirectivesDecl(RecordKeeper &Records, raw_ostream &OS) {
  OS << "bool isAllowedClauseForDirective(Directive D, "
     << "Clause C, unsigned Version);\n";
  OS << "\n";
+  OS << "const llvm::SmallVector<Directive> &getLeafConstructs(Directive D);\n";


Suggested change

OS << "const llvm::SmallVector<Directive> &getLeafConstructs(Directive D);\n";

OS << "ArrayRef<Directive> getLeafConstructs(Directive D);\n";

I don't think this is a good idea. Returning a reference is pretty much free, Returning an ArrayRef will always create a temporary object. You can always use ArrayRef in the caller, if that's what's desired there.
Edit: I'm open to changing my mind, but I'm not sure what the motivation is for this change.

No, it won't. ArrayRef is not an owning container, so it does not create a copy.

The ArrayRef will be the temporary object.

No, it is not.

I'm expecting you to be more professional in your replies. I'm simply trying to understand your motivation here.

Read the documentation https://llvm.org/docs/ProgrammersManual.html#llvm-adt-arrayref-h

That's about input parameters (because then we don't restrict what objects can be passed, i.e. converted to an ArrayRef). For return values it doesn't matter, at least from the point of view of interface flexibility. The static constructor issue you pointed out will need this though.

llvm/utils/TableGen/DirectiveEmitter.cpp

kparzysz · 2024-03-06T13:27:52Z

Hey @alexey-bataev, I have made all the changes you requested. Could you take another look?

alexey-bataev · 2024-03-06T13:34:44Z

llvm/test/TableGen/directive1.td

@@ -342,4 +359,22 @@ def TDL_DirA : Directive<"dira"> {
 // IMPL-NEXT:    llvm_unreachable("Invalid Tdl Directive kind");
 // IMPL-NEXT:  }
 // IMPL-EMPTY:
+// IMPL-NEXT:  llvm::ArrayRef<llvm::tdl::Directive> llvm::tdl::getLeafConstructs(llvm::tdl::Directive Dir) {
+// IMPL-NEXT:    static llvm::ArrayRef<llvm::tdl::Directive> nothing {};


Is this supposed to be a container for the leaves? ArrayRef is not owning container, I'm afraid it may cause some UB. Better just to use POD arrays here, if I understand correctly generated code.

Eh, sorry, it's a mistake. Thanks for the catch.

Edit: This was meant to return an empty ArrayRef. Having it static is an overkill though.

Returning an empty ArrayRef by value now. I can't use regular array (as in the case of non-empty sets), because I can't create a zero-length array. The empty ArrayRef does not refer to any storage, so returning a non-static one is not going to be a problem.

alexey-bataev · 2024-03-06T14:36:31Z

llvm/utils/TableGen/DirectiveEmitter.cpp

+    OS << "  static const " << DirectiveTypeName << ' ' << ListName
+       << "[] = {\n";
+    for (Record *L : LeafConstructs) {
+      Directive LeafDir{L};
+      OS << "    " << getQualifiedName(LeafDir.getFormattedName()) << ",\n";
+    }
+    OS << "  };\n";


The actual storage here is just POD arrays, right?

Yes,

static const llvm::omp::Directive[] = {...};

kparzysz · 2024-03-06T15:21:30Z

Hey @erichkeane, do you have anything to add? I have replaced leafs in the .td file with leafConstructs, and with leaves elsewhere.

alexey-bataev

LG

erichkeane · 2024-03-06T15:43:24Z

Hey @erichkeane, do you have anything to add? I have replaced leafs in the .td file with leafConstructs, and with leaves elsewhere.

Nothing left, mine was just a drive-by comment that got out of hand :) LGTM.

kparzysz requested review from clementval, jsjodin, kiranchandramohan, alexey-bataev, agozillon and jeanPerier March 1, 2024 21:52

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" flang:openmp openacc clang:openmp OpenMP related changes to Clang labels Mar 1, 2024

alexey-bataev reviewed Mar 1, 2024

View reviewed changes

clang/lib/Basic/OpenMPKinds.cpp Outdated Show resolved Hide resolved

clang/lib/Basic/OpenMPKinds.cpp Outdated Show resolved Hide resolved

clang/lib/Basic/OpenMPKinds.cpp Outdated Show resolved Hide resolved

clang/lib/Basic/OpenMPKinds.cpp Outdated Show resolved Hide resolved

kparzysz force-pushed the omp-flags branch from 544b309 to b62919c Compare March 1, 2024 22:00

Address review comments

5ce3416

clementval requested a review from erichkeane March 1, 2024 22:59

clementval approved these changes Mar 1, 2024

View reviewed changes

kparzysz added 2 commits March 1, 2024 17:28

Update failing TableGen testcases

9cd4e59

Merge branch 'main' into omp-flags

1aa6ae9

Merge branch 'main' into omp-flags

e751905

kiranchandramohan approved these changes Mar 3, 2024

View reviewed changes

Remove explicit check for parallel workshare

ff8e5ee

alexey-bataev reviewed Mar 3, 2024

View reviewed changes

Replace auto Leafs = ... with ArrayRef<Directive> Leafs = ...

3eabfa0

alexey-bataev requested changes Mar 4, 2024

View reviewed changes

llvm/utils/TableGen/DirectiveEmitter.cpp Show resolved Hide resolved

Replace static SmallVector constructors with static arrays

89d0b08

kparzysz requested a review from alexey-bataev March 4, 2024 13:51

kparzysz added 2 commits March 4, 2024 09:38

Relent to public pressure...

ff5b690

More relenting

3432cd3

alexey-bataev reviewed Mar 6, 2024

View reviewed changes

Avoid empty static ArrayRef, return one by value

6793db9

alexey-bataev reviewed Mar 6, 2024

View reviewed changes

alexey-bataev approved these changes Mar 6, 2024

View reviewed changes

kparzysz merged commit 67c82d6 into llvm:main Mar 6, 2024
4 checks passed

kparzysz deleted the omp-flags branch March 6, 2024 16:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Frontend] Add leaf constructs and association to OpenMP/ACC directives #83625

[Frontend] Add leaf constructs and association to OpenMP/ACC directives #83625

kparzysz commented Mar 1, 2024 •

edited

llvmbot commented Mar 1, 2024 •

edited

github-actions bot commented Mar 1, 2024 •

edited

kparzysz commented Mar 1, 2024 •

edited

clementval left a comment

kparzysz commented Mar 2, 2024

kiranchandramohan left a comment

kiranchandramohan Mar 3, 2024

kparzysz Mar 3, 2024 •

edited

alexey-bataev Mar 3, 2024

alexey-bataev Mar 3, 2024

kparzysz Mar 3, 2024 •

edited

alexey-bataev Mar 4, 2024

kparzysz Mar 4, 2024

alexey-bataev Mar 4, 2024

kparzysz Mar 4, 2024

alexey-bataev Mar 4, 2024

kparzysz Mar 4, 2024

kparzysz commented Mar 6, 2024

alexey-bataev Mar 6, 2024

kparzysz Mar 6, 2024 •

edited

kparzysz Mar 6, 2024 •

edited

alexey-bataev Mar 6, 2024

kparzysz Mar 6, 2024

kparzysz commented Mar 6, 2024

alexey-bataev left a comment

erichkeane commented Mar 6, 2024

	// IMPL-NEXT: const llvm::SmallVector<llvm::tdl::Directive> &llvm::tdl::getLeafConstructs(llvm::tdl::Directive Dir) {
	// IMPL-NEXT: ArrayRef<llvm::tdl::Directive> llvm::tdl::getLeafConstructs(llvm::tdl::Directive Dir) {

	OS << "const llvm::SmallVector<Directive> &getLeafConstructs(Directive D);\n";
	OS << "ArrayRef<Directive> getLeafConstructs(Directive D);\n";

[Frontend] Add leaf constructs and association to OpenMP/ACC directives #83625

[Frontend] Add leaf constructs and association to OpenMP/ACC directives #83625

Conversation

kparzysz commented Mar 1, 2024 • edited

llvmbot commented Mar 1, 2024 • edited

github-actions bot commented Mar 1, 2024 • edited

kparzysz commented Mar 1, 2024 • edited

clementval left a comment

Choose a reason for hiding this comment

kparzysz commented Mar 2, 2024

kiranchandramohan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kparzysz Mar 3, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kparzysz Mar 3, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kparzysz commented Mar 6, 2024

Choose a reason for hiding this comment

kparzysz Mar 6, 2024 • edited

Choose a reason for hiding this comment

kparzysz Mar 6, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kparzysz commented Mar 6, 2024

alexey-bataev left a comment

Choose a reason for hiding this comment

erichkeane commented Mar 6, 2024

kparzysz commented Mar 1, 2024 •

edited

llvmbot commented Mar 1, 2024 •

edited

github-actions bot commented Mar 1, 2024 •

edited

kparzysz commented Mar 1, 2024 •

edited

kparzysz Mar 3, 2024 •

edited

kparzysz Mar 3, 2024 •

edited

kparzysz Mar 6, 2024 •

edited

kparzysz Mar 6, 2024 •

edited