[Mlir-commits] [mlir] [OpenACC] add isEffectivelySerial() to acc.parallel and acc.kernels (PR #209192)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-openacc

Author: Scott Manley (rscottmanley)

<details>
<summary>Changes</summary>

Mirror the isEffectivelySerial() function in acc.compute_region to acc.parallel and acc.kernel so this check is unified anywhere it needs to be used, namely before ACCComputeLowering. Should be NFC

---
Full diff: https://github.com/llvm/llvm-project/pull/209192.diff


3 Files Affected:

- (modified) mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td (+6) 
- (modified) mlir/lib/Dialect/OpenACC/IR/OpenACCCG.cpp (+35) 
- (modified) mlir/lib/Dialect/OpenACC/Transforms/ACCComputeLowering.cpp (+3-38) 


``````````diff
diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
index 30298767c0f3e..db2f0e0605db5 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
@@ -1842,6 +1842,9 @@ def OpenACC_ParallelOp
     /// clause for the given device_type.
     bool hasAnyGangWorkerVector(mlir::acc::DeviceType deviceType);
 
+    /// Check whether num_gangs, num_workers, and vector_length are all 1.
+    bool isEffectivelySerial();
+
     /// Return true if the op has the wait attribute for the
     /// mlir::acc::DeviceType::None device_type.
     bool hasWaitOnly();
@@ -2173,6 +2176,9 @@ def OpenACC_KernelsOp
     /// clause for the given device_type.
     bool hasAnyGangWorkerVector(mlir::acc::DeviceType deviceType);
 
+    /// Check whether num_gangs, num_workers, and vector_length are all 1.
+    bool isEffectivelySerial();
+
     /// Return true if the op has the wait attribute for the
     /// mlir::acc::DeviceType::None device_type.
     bool hasWaitOnly();
diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACCCG.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACCCG.cpp
index 7de59058ead0d..3b168b7631db8 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACCCG.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACCCG.cpp
@@ -14,6 +14,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "mlir/Dialect/Arith/IR/Arith.h"
 #include "mlir/Dialect/GPU/IR/GPUDialect.h"
 #include "mlir/Dialect/OpenACC/OpenACC.h"
 #include "mlir/Dialect/Utils/StaticValueUtils.h"
@@ -571,6 +572,40 @@ ComputeRegionOp::wireHoistedValueThroughIns(Value value) {
   return arg;
 }
 
+/// Strip index_cast operations from a value before checking for a constant.
+static Value stripIndexCasts(Value val) {
+  while (auto castOp = val.getDefiningOp<arith::IndexCastOp>())
+    val = castOp.getIn();
+  return val;
+}
+
+template <typename ComputeOpT>
+static bool isGangWorkerVectorAllOne(ComputeOpT op) {
+  auto numGangs = op.getNumGangsValues();
+  if (numGangs.empty())
+    return false;
+  for (Value gangSize : numGangs) {
+    if (!isConstantIntValue(stripIndexCasts(gangSize), 1))
+      return false;
+  }
+  Value numWorkers = op.getNumWorkersValue();
+  if (!numWorkers)
+    return false;
+  Value vectorLength = op.getVectorLengthValue();
+  if (!vectorLength)
+    return false;
+  return isConstantIntValue(stripIndexCasts(numWorkers), 1) &&
+         isConstantIntValue(stripIndexCasts(vectorLength), 1);
+}
+
+bool ParallelOp::isEffectivelySerial() {
+  return isGangWorkerVectorAllOne(*this);
+}
+
+bool KernelsOp::isEffectivelySerial() {
+  return isGangWorkerVectorAllOne(*this);
+}
+
 bool ComputeRegionOp::isEffectivelySerial() {
   auto *ctx = getContext();
 
diff --git a/mlir/lib/Dialect/OpenACC/Transforms/ACCComputeLowering.cpp b/mlir/lib/Dialect/OpenACC/Transforms/ACCComputeLowering.cpp
index b3d54d2e05697..a275a794c9fd5 100644
--- a/mlir/lib/Dialect/OpenACC/Transforms/ACCComputeLowering.cpp
+++ b/mlir/lib/Dialect/OpenACC/Transforms/ACCComputeLowering.cpp
@@ -78,41 +78,6 @@ namespace {
 // Helper functions
 //===----------------------------------------------------------------------===//
 
-/// Strip index_cast operations from a value before checking for a constant.
-static Value stripIndexCasts(Value val) {
-  while (auto castOp = val.getDefiningOp<arith::IndexCastOp>())
-    val = castOp.getIn();
-  return val;
-}
-
-template <typename ComputeOpT>
-static bool isGangWorkerVectorAllOne(ComputeOpT op) {
-  auto numGangs = op.getNumGangsValues();
-  if (numGangs.empty())
-    return false;
-  for (Value gangSize : numGangs) {
-    if (!isConstantIntValue(stripIndexCasts(gangSize), 1))
-      return false;
-  }
-  Value numWorkers = op.getNumWorkersValue();
-  if (!numWorkers)
-    return false;
-  Value vectorLength = op.getVectorLengthValue();
-  if (!vectorLength)
-    return false;
-  return isConstantIntValue(stripIndexCasts(numWorkers), 1) &&
-         isConstantIntValue(stripIndexCasts(vectorLength), 1);
-}
-
-/// A compute construct is "effectively serial" when it specifies
-/// num_gangs(1), num_workers(1), and vector_length(1). This is because
-/// these are the only parallelism dimensions expressible from OpenACC spec
-/// point-of-view and is consistent with how `serial` semantics are defined.
-template <typename ComputeOpT>
-static bool isEffectivelySerial(ComputeOpT op) {
-  return isGangWorkerVectorAllOne(op);
-}
-
 static bool isOpInComputeRegion(Operation *op) {
   Region *region = op->getBlock()->getParent();
   return getEnclosingComputeOp(*region) != nullptr;
@@ -120,9 +85,9 @@ static bool isOpInComputeRegion(Operation *op) {
 
 static bool isOpInSerialRegion(Operation *op) {
   if (auto parallelOp = op->getParentOfType<ParallelOp>())
-    return isEffectivelySerial(parallelOp);
+    return parallelOp.isEffectivelySerial();
   if (auto kernelsOp = op->getParentOfType<KernelsOp>())
-    return isEffectivelySerial(kernelsOp);
+    return kernelsOp.isEffectivelySerial();
   if (op->getParentOfType<SerialOp>())
     return true;
   if (auto computeRegion = op->getParentOfType<ComputeRegionOp>())
@@ -230,7 +195,7 @@ assignKnownLaunchArgs(ComputeConstructT computeOp, DeviceType deviceType,
     return {ParWidthOp::create(rewriter, loc, Value(), policy.seqDim(ctx))};
   } else if constexpr (llvm::is_one_of<ComputeConstructT, ParallelOp,
                                        KernelsOp>::value) {
-    if (isEffectivelySerial(computeOp))
+    if (computeOp.isEffectivelySerial())
       return {ParWidthOp::create(rewriter, loc, Value(), policy.seqDim(ctx))};
 
     deviceType = getParDimsDeviceType(computeOp, deviceType);

``````````

</details>


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