Address review comments

llvm · Aug 9, 2024 · 61af447 · 61af447
1 parent aa0403a
commit 61af447
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 31 deletions.
diff --git a/flang/lib/Lower/OpenMP/Decomposer.cpp b/flang/lib/Lower/OpenMP/Decomposer.cpp
@@ -100,15 +100,19 @@ ConstructQueue buildConstructQueue(
 
 bool matchLeafSequence(ConstructQueue::const_iterator item,
                        const ConstructQueue &queue,
-                       llvm::ArrayRef<llvm::omp::Directive> directives) {
+                       llvm::omp::Directive directive) {
+  llvm::ArrayRef<llvm::omp::Directive> leafDirs =
+      llvm::omp::getLeafConstructsOrSelf(directive);
+
   for (auto [dir, leaf] :
-       llvm::zip_longest(directives, llvm::make_range(item, queue.end()))) {
-    if (!dir || !leaf)
+       llvm::zip_longest(leafDirs, llvm::make_range(item, queue.end()))) {
+    if (!dir.has_value() || !leaf.has_value())
       return false;
 
-    if (dir.value() != leaf.value().id)
+    if (*dir != leaf->id)
       return false;
   }
+
   return true;
 }
 

diff --git a/flang/lib/Lower/OpenMP/Decomposer.h b/flang/lib/Lower/OpenMP/Decomposer.h
@@ -49,11 +49,14 @@ ConstructQueue buildConstructQueue(mlir::ModuleOp modOp,
 bool isLastItemInQueue(ConstructQueue::const_iterator item,
                        const ConstructQueue &queue);
 
-/// Try to match a sequence of \c directives to the range of leaf constructs
-/// starting from \c item to the end of the \c queue.
+/// Try to match the leaf constructs conforming the given \c directive to the
+/// range of leaf constructs starting from \c item to the end of the \c queue.
+/// If \c directive doesn't represent a compound directive, check that \c item
+/// matches that directive and is the only element before the end of the
+/// \c queue.
 bool matchLeafSequence(ConstructQueue::const_iterator item,
                        const ConstructQueue &queue,
-                       llvm::ArrayRef<llvm::omp::Directive> directives);
+                       llvm::omp::Directive directive);
 } // namespace Fortran::lower::omp
 
 #endif // FORTRAN_LOWER_OPENMP_DECOMPOSER_H
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -2168,28 +2168,22 @@ static bool genOMPCompositeDispatch(
     mlir::Location loc, const ConstructQueue &queue,
     ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
   using llvm::omp::Directive;
-  using llvm::omp::getLeafConstructs, lower::omp::matchLeafSequence;
+  using lower::omp::matchLeafSequence;
 
-  if (matchLeafSequence(
-          item, queue,
-          getLeafConstructs(Directive::OMPD_distribute_parallel_do)))
+  if (matchLeafSequence(item, queue, Directive::OMPD_distribute_parallel_do))
     genCompositeDistributeParallelDo(converter, symTable, semaCtx, eval, loc,
                                      queue, item, dsp);
-  else if (matchLeafSequence(
-               item, queue,
-               getLeafConstructs(Directive::OMPD_distribute_parallel_do_simd)))
+  else if (matchLeafSequence(item, queue,
+                             Directive::OMPD_distribute_parallel_do_simd))
     genCompositeDistributeParallelDoSimd(converter, symTable, semaCtx, eval,
                                          loc, queue, item, dsp);
-  else if (matchLeafSequence(
-               item, queue, getLeafConstructs(Directive::OMPD_distribute_simd)))
+  else if (matchLeafSequence(item, queue, Directive::OMPD_distribute_simd))
     genCompositeDistributeSimd(converter, symTable, semaCtx, eval, loc, queue,
                                item, dsp);
-  else if (matchLeafSequence(item, queue,
-                             getLeafConstructs(Directive::OMPD_do_simd)))
+  else if (matchLeafSequence(item, queue, Directive::OMPD_do_simd))
     genCompositeDoSimd(converter, symTable, semaCtx, eval, loc, queue, item,
                        dsp);
-  else if (matchLeafSequence(item, queue,
-                             getLeafConstructs(Directive::OMPD_taskloop_simd)))
+  else if (matchLeafSequence(item, queue, Directive::OMPD_taskloop_simd))
     genCompositeTaskloopSimd(converter, symTable, semaCtx, eval, loc, queue,
                              item, dsp);
   else
@@ -2318,18 +2312,11 @@ static void genOMPDispatch(lower::AbstractConverter &converter,
     // that use this construct, add a single construct for now.
     genSingleOp(converter, symTable, semaCtx, eval, loc, queue, item);
     break;
-
-  // Composite constructs
-  case llvm::omp::Directive::OMPD_distribute_parallel_do:
-  case llvm::omp::Directive::OMPD_distribute_parallel_do_simd:
-  case llvm::omp::Directive::OMPD_distribute_simd:
-  case llvm::omp::Directive::OMPD_do_simd:
-  case llvm::omp::Directive::OMPD_taskloop_simd:
-    // Composite constructs should have been split into a sequence of leaf
-    // constructs and lowered by genOMPCompositeDispatch().
-    llvm_unreachable("Unexpected composite construct.");
-    break;
   default:
+    // Combined and composite constructs should have been split into a sequence
+    // of leaf constructs when building the construct queue.
+    assert(!llvm::omp::isLeafConstruct(dir) &&
+           "Unexpected compound construct.");
     break;
   }