[Mlir-commits] [clang] [llvm] [mlir] [openmp] [OpenMP][offload] Cross-team reductions with variable number of teams (PR #195102)

[Robert Imschweiler]

================
@@ -119,9 +119,25 @@ GenericKernelTy::getKernelLaunchEnvironment(
       KernelArgs.Version < OMP_KERNEL_ARG_MIN_VERSION_WITH_DYN_PTR)
     return nullptr;
 
-  if ((!KernelEnvironment.Configuration.ReductionDataSize ||
-       !KernelEnvironment.Configuration.ReductionBufferLength) &&
-      KernelArgs.DynCGroupMem == 0)
+  const auto &RedCfg = KernelEnvironment.Configuration;
+  // ReductionDataSize is the single source of truth for whether a teams-
+  // reduction buffer is needed; ReductionBufferLength is only a static
+  // upper bound on the number of teams and is meaningless without a
+  // non-zero data size. Reject the malformed combination explicitly so we
+  // never hand the device a null ReductionBuffer that the reduction
+  // runtime would then dereference.
+  if (RedCfg.ReductionBufferLength && !RedCfg.ReductionDataSize)
+    return Plugin::error(ErrorCode::INVALID_BINARY,
+                         "kernel environment has a non-zero "
+                         "ReductionBufferLength but ReductionDataSize is 0");
+  const bool NeedsReductionBuffer = RedCfg.ReductionDataSize != 0;
+  if (NeedsReductionBuffer && KernelArgs.Version < OMP_KERNEL_ARG_VERSION)
+    return Plugin::error(ErrorCode::INVALID_BINARY,
+                         "kernel was built against an older OpenMP "
+                         "kernel-launch-environment ABI (v%u); current "
+                         "runtime requires v%u for cross-team reductions",
+                         KernelArgs.Version, OMP_KERNEL_ARG_VERSION);
----------------
ro-i wrote:

I changed my mind halfway when I made those changes, sorry. I now removed ReductionBufLen completely (see the commits I'm about to push in a moment)

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