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

[via flang-commits]

================
@@ -1481,17 +1481,36 @@ void collectEnclosingConstructTraits(
   // 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))
+    if (mlir::isa<mlir::omp::SimdOp>(op))
+      constructTraits.push_back(llvm::omp::TraitProperty::construct_simd_simd);
+    else if (mlir::isa<mlir::omp::WsloopOp>(op))
       constructTraits.push_back(llvm::omp::TraitProperty::construct_for_for);
-    if (mlir::isa<mlir::omp::ParallelOp>(op))
+    else if (mlir::isa<mlir::omp::ParallelOp>(op))
       constructTraits.push_back(
           llvm::omp::TraitProperty::construct_parallel_parallel);
-    if (mlir::isa<mlir::omp::TeamsOp>(op))
+    else if (mlir::isa<mlir::omp::TeamsOp>(op))
       constructTraits.push_back(
           llvm::omp::TraitProperty::construct_teams_teams);
-    if (mlir::isa<mlir::omp::TargetOp>(op))
+    else if (mlir::isa<mlir::omp::TargetOp>(op)) {
       constructTraits.push_back(
           llvm::omp::TraitProperty::construct_target_target);
+      // The construct context starts at the innermost TARGET, as in
+      // semantic analysis.
+      break;
+    } else if (mlir::isa<mlir::omp::CriticalOp, mlir::omp::DistributeOp,
+                         mlir::omp::FuseOp, mlir::omp::LoopOp,
+                         mlir::omp::MaskedOp, mlir::omp::MasterOp,
+                         mlir::omp::OrderedRegionOp, mlir::omp::ScopeOp,
+                         mlir::omp::SectionsOp, mlir::omp::SingleOp,
+                         mlir::omp::TargetDataOp, mlir::omp::TaskgroupOp,
+                         mlir::omp::TaskloopContextOp, mlir::omp::TaskOp,
+                         mlir::omp::TileOp, mlir::omp::UnrollFullOp,
----------------
MattPD wrote:

The following operations have no region: `omp.tile`, `omp.unroll_full`, `omp.unroll_partial`, and `omp.fuse`. They consume a canonical-loop handle and are emitted beside the `omp.canonical_loop` that holds the body. They are therefore never `getParentOp()` ancestors of a call or metadirective in that body, so these four entries cannot match. `atomic` has the same problem for a different reason. `omp.atomic.update` is created after its operand expression is lowered, so a call inside `atomic update` has no atomic ancestor. Loop transformations and `atomic` are constructs under OpenMP 5.2 section 7.1, and Clang counts both after this PR.

You can reproduce the `atomic` case by saving the following to `atomic.f90` and running `flang -fc1 -fopenmp -fopenmp-version=52 -emit-hlfir -o - atomic.f90`:

```fortran
module m
contains
  integer function cpu()
    cpu = 1
  end function
  integer function vendor()
    vendor = 2
  end function
  integer function base()
    !$omp declare variant(vendor) match(implementation={vendor(score(1): llvm)})
    !$omp declare variant(cpu) match(device={kind(cpu)})
    base = 0
  end function
  subroutine task_context(x)
    integer :: x
    !$omp task
    x = x + base()
    !$omp end task
  end subroutine
  subroutine atomic_context(x)
    integer :: x
    !$omp atomic update
    x = x + base()
  end subroutine
end module
```

With this PR `task_context` calls `cpu`, since depth 1 makes `kind(cpu)` score 3 against 2. `atomic_context` calls `vendor`, since depth 0 produces a tie that declaration order resolves. The merge base calls `vendor` in both. Clang selects the CPU variant inside `atomic update`. A call inside `!$omp tile sizes(2)` behaves like the atomic case. If these constructs are meant to count, lowering must derive their positions from the source structure or emit a placeholder before the body. If they are not meant to count, the four entries are dead, and the description should name the excluded constructs.

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