[PATCH] D102008: [OpenMP]Add support for workshare loop modifier in lowering

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

In D102008#2744138 <https://reviews.llvm.org/D102008#2744138>, @Leporacanthicus wrote:

> Patch doesn't apply because it is relying on my previous patch. Doh!

Add the other patch as parent in Phabricator



================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1436-1442
+    if (SchedType == omp::OMPScheduleType::DynamicChunked ||
+        SchedType == omp::OMPScheduleType::GuidedChunked) {
+      DynamicSchedType |= omp::OMPScheduleType::ModifierNonmonotonic;
+    } else {
+      DynamicSchedType |= omp::OMPScheduleType::ModifierMonotonic;
+    }
+  }
----------------
Leporacanthicus wrote:
> @Meinersbur & @jdoerfert 
> 
> Does this section make sense: use non-monotonic as default for guided and dynamic, and monotonic for Auto and Runtime? From what I can tell, this is what Clang does, and the Fortran compiler complains if I try to use non-monotonic with Auto and Runtime. 
Try to mimic what the current OpenMP implementation does:
```
  // OpenMP 5.0, 2.9.2 Worksharing-Loop Construct, Desription.
  // If the static schedule kind is specified or if the ordered clause is
  // specified, and if the nonmonotonic modifier is not specified, the effect is
  // as if the monotonic modifier is specified. Otherwise, unless the monotonic
  // modifier is specified, the effect is as if the nonmonotonic modifier is
  // specified.
  if (CGM.getLangOpts().OpenMP >= 50 && Modifier == 0) {
    if (!(Schedule == OMP_sch_static_chunked || Schedule == OMP_sch_static ||
          Schedule == OMP_sch_static_balanced_chunked ||
          Schedule == OMP_ord_static_chunked || Schedule == OMP_ord_static ||
          Schedule == OMP_dist_sch_static_chunked ||
          Schedule == OMP_dist_sch_static))
      Modifier = OMP_sch_modifier_nonmonotonic;
  }
  return Schedule | Modifier;
```

i.e. add implicit nonmonotonic only for static schedules and if user did not explicitly specify a modifier. This is `createDynamicWorkshareLoop` so I guess no implicit modifier should be added at all.


================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:1800-1807
+  if (!static_cast<uint32_t>(SchedType & omp::OMPScheduleType::ModifierMask)) {
+    if (SchedType == omp::OMPScheduleType::DynamicChunked ||
+        SchedType == omp::OMPScheduleType::GuidedChunked) {
+      ExpectedSchedType |= omp::OMPScheduleType::ModifierNonmonotonic;
+    } else {
+      ExpectedSchedType |= omp::OMPScheduleType::ModifierMonotonic;
+    }
----------------
Copy&Pasting the code into the tests doesn't seem useful. I'd prefer checking some values and their expected result.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102008/new/

https://reviews.llvm.org/D102008