[clang] [flang] [llvm] [clang][flang][OpenMP] Fix context selector matching and scoring (PR #224431)

[via cfe-commits]

================
@@ -1473,27 +1490,82 @@ semantics::omp::OmpVariantMatchContext makeVariantMatchContext(
 }
 
 void collectEnclosingConstructTraits(
-    mlir::Operation *op,
+    AbstractConverter &converter, const pft::Evaluation *evaluation,
     llvm::SmallVectorImpl<llvm::omp::TraitProperty> &constructTraits) {
-  // Collect enclosing OpenMP operations so variants chosen by an outer
-  // metadirective are part of this metadirective's context. For example, an
-  // inner metadirective inside `target` and an outer-selected `parallel` must
-  // be able to match construct={target, parallel}. The final reverse yields
-  // outermost-to-innermost order as required by OMPContext.
-  for (; op; op = op->getParentOp()) {
-    if (mlir::isa<mlir::omp::WsloopOp>(op))
-      constructTraits.push_back(llvm::omp::TraitProperty::construct_for_for);
-    if (mlir::isa<mlir::omp::ParallelOp>(op))
-      constructTraits.push_back(
-          llvm::omp::TraitProperty::construct_parallel_parallel);
-    if (mlir::isa<mlir::omp::TeamsOp>(op))
-      constructTraits.push_back(
-          llvm::omp::TraitProperty::construct_teams_teams);
-    if (mlir::isa<mlir::omp::TargetOp>(op))
-      constructTraits.push_back(
-          llvm::omp::TraitProperty::construct_target_target);
+  const auto *loopControl =
+      converter.getStateStack().getStackTop<LoopControlContext>();
+  // Use the loop owner's ancestors: host evaluation may have TARGET current
+  // while evaluating a nested loop's bounds.
+  if (loopControl)
+    evaluation = &loopControl->evaluation;
+
+  llvm::SmallVector<const OpenMPContextFrame *, 4> frames;
+  converter.getStateStack().stackWalk<OpenMPContextFrame>(
+      [&](OpenMPContextFrame &frame) {
+        frames.push_back(&frame);
+        return mlir::WalkResult::advance();
+      });
+  std::reverse(frames.begin(), frames.end());
+  llvm::SmallVector<bool, 4> usedFrames(frames.size(), false);
+
+  llvm::SmallVector<const pft::Evaluation *, 8> ancestors;
+  for (const pft::Evaluation *parent = evaluation ? evaluation->parentConstruct
+                                                  : nullptr;
+       parent; parent = parent->parentConstruct) {
+    ancestors.push_back(parent);
+  }
+  std::reverse(ancestors.begin(), ancestors.end());
+
+  auto append = [&](llvm::omp::Directive directive) {
+    semantics::omp::AppendDirectiveContextTraits(directive, constructTraits);
+  };
+  auto getDirective = [&](const pft::Evaluation &eval,
+                          const parser::OpenMPConstruct &omp) {
+    llvm::omp::Directive directive = parser::omp::GetOmpDirectiveName(omp).v;
+    if (directive == llvm::omp::Directive::OMPD_metadirective)
+      for (const OpenMPContextFrame *frame : frames)
+        if (&frame->evaluation == &eval && frame->isReplacement)
+          return frame->directive;
+    return directive;
+  };
+  for (const pft::Evaluation *ancestor : ancestors) {
+    const auto *omp = ancestor->getIf<parser::OpenMPConstruct>();
----------------
MattPD wrote:

When a metadirective appears before any executable statement, its evaluation is an `OpenMPDeclarativeConstruct`, so the ancestor walk skips it without consuming the replacement's entered frames. The collector then appends the selected PARALLEL after a nested TARGET has cleared the outer context.

You can reproduce this by saving the following to `repro.f90` and running `flang -fc1 -fopenmp -fopenmp-version=52 -emit-hlfir -module-dir /tmp repro.f90 -o -`:

```fortran
module m
contains
  subroutine par()
  end subroutine
  subroutine base()
    !$omp declare variant(par) match(construct={parallel})
  end subroutine
  subroutine s()
    !$omp metadirective when(implementation={vendor(llvm)}: parallel)
    block
      !$omp target
        call base()
      !$omp end target
    end block
  end subroutine
end module
```

At 1850727, `s` calls `par` inside TARGET. It should call `base`, because [OpenMP 5.2 7.1](https://www.openmp.org/spec-html/5.2/openmpse42.html) excludes constructs outside the innermost TARGET. Adding an executable statement before the metadirective, or writing PARALLEL directly, preserves the TARGET boundary. At 9d79b83, `s` also calls `base`, although Flang omits the selected PARALLEL.

Could the ancestor walk consume a replacement's entered frames at the metadirective's source position for both `OpenMPConstruct` and `OpenMPDeclarativeConstruct` evaluations?

https://github.com/llvm/llvm-project/pull/224431