[Mlir-commits] [mlir] [mlir][SPIR-V] Add OpenCL lowering path for index.min/max (PR #203493)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-spirv

Author: Arseniy Obolenskiy (aobolensk)

<details>
<summary>Changes</summary>

GLSL min/max ops were emitted even for Kernel targets where these ops are illegal

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


2 Files Affected:

- (modified) mlir/lib/Conversion/IndexToSPIRV/IndexToSPIRV.cpp (+18-11) 
- (modified) mlir/test/Conversion/IndexToSPIRV/index-to-spirv.mlir (+26-4) 


``````````diff
diff --git a/mlir/lib/Conversion/IndexToSPIRV/IndexToSPIRV.cpp b/mlir/lib/Conversion/IndexToSPIRV/IndexToSPIRV.cpp
index d4b4c46d3c758..9718361d4c4ba 100644
--- a/mlir/lib/Conversion/IndexToSPIRV/IndexToSPIRV.cpp
+++ b/mlir/lib/Conversion/IndexToSPIRV/IndexToSPIRV.cpp
@@ -30,10 +30,14 @@ using ConvertIndexDivS = spirv::ElementwiseOpPattern<DivSOp, spirv::SDivOp>;
 using ConvertIndexDivU = spirv::ElementwiseOpPattern<DivUOp, spirv::UDivOp>;
 using ConvertIndexRemS = spirv::ElementwiseOpPattern<RemSOp, spirv::SRemOp>;
 using ConvertIndexRemU = spirv::ElementwiseOpPattern<RemUOp, spirv::UModOp>;
-using ConvertIndexMaxS = spirv::ElementwiseOpPattern<MaxSOp, spirv::GLSMaxOp>;
-using ConvertIndexMaxU = spirv::ElementwiseOpPattern<MaxUOp, spirv::GLUMaxOp>;
-using ConvertIndexMinS = spirv::ElementwiseOpPattern<MinSOp, spirv::GLSMinOp>;
-using ConvertIndexMinU = spirv::ElementwiseOpPattern<MinUOp, spirv::GLUMinOp>;
+using ConvertIndexMaxSGL = spirv::ElementwiseOpPattern<MaxSOp, spirv::GLSMaxOp>;
+using ConvertIndexMaxUGL = spirv::ElementwiseOpPattern<MaxUOp, spirv::GLUMaxOp>;
+using ConvertIndexMinSGL = spirv::ElementwiseOpPattern<MinSOp, spirv::GLSMinOp>;
+using ConvertIndexMinUGL = spirv::ElementwiseOpPattern<MinUOp, spirv::GLUMinOp>;
+using ConvertIndexMaxSCL = spirv::ElementwiseOpPattern<MaxSOp, spirv::CLSMaxOp>;
+using ConvertIndexMaxUCL = spirv::ElementwiseOpPattern<MaxUOp, spirv::CLUMaxOp>;
+using ConvertIndexMinSCL = spirv::ElementwiseOpPattern<MinSOp, spirv::CLSMinOp>;
+using ConvertIndexMinUCL = spirv::ElementwiseOpPattern<MinUOp, spirv::CLUMinOp>;
 
 using ConvertIndexShl =
     spirv::ElementwiseOpPattern<ShlOp, spirv::ShiftLeftLogicalOp>;
@@ -350,10 +354,6 @@ void index::populateIndexToSPIRVPatterns(
     ConvertIndexDivU,
     ConvertIndexRemS,
     ConvertIndexRemU,
-    ConvertIndexMaxS,
-    ConvertIndexMaxU,
-    ConvertIndexMinS,
-    ConvertIndexMinU,
     ConvertIndexShl,
     ConvertIndexShrS,
     ConvertIndexShrU,
@@ -370,6 +370,15 @@ void index::populateIndexToSPIRVPatterns(
     ConvertIndexCmpPattern,
     ConvertIndexSizeOf
   >(typeConverter, patterns.getContext());
+  // clang-format on
+
+  // GLSL min/max patterns.
+  patterns.add<ConvertIndexMaxSGL, ConvertIndexMaxUGL, ConvertIndexMinSGL,
+               ConvertIndexMinUGL>(typeConverter, patterns.getContext());
+
+  // OpenCL min/max patterns.
+  patterns.add<ConvertIndexMaxSCL, ConvertIndexMaxUCL, ConvertIndexMinSCL,
+               ConvertIndexMinUCL>(typeConverter, patterns.getContext());
 }
 
 //===----------------------------------------------------------------------===//
@@ -394,7 +403,7 @@ struct ConvertIndexToSPIRVPass
     Operation *op = getOperation();
     spirv::TargetEnvAttr targetAttr = spirv::lookupTargetEnvOrDefault(op);
     std::unique_ptr<SPIRVConversionTarget> target =
-      SPIRVConversionTarget::get(targetAttr);
+        SPIRVConversionTarget::get(targetAttr);
 
     SPIRVConversionOptions options;
     options.use64bitIndex = this->use64bitIndex;
@@ -404,8 +413,6 @@ struct ConvertIndexToSPIRVPass
     // in patterns for other dialects.
     target->addLegalOp<UnrealizedConversionCastOp>();
 
-    // Allow the spirv operations we are converting to
-    target->addLegalDialect<spirv::SPIRVDialect>();
     // Fail hard when there are any remaining 'index' ops.
     target->addIllegalDialect<index::IndexDialect>();
 
diff --git a/mlir/test/Conversion/IndexToSPIRV/index-to-spirv.mlir b/mlir/test/Conversion/IndexToSPIRV/index-to-spirv.mlir
index 7b26f4fc13696..650c030d80f55 100644
--- a/mlir/test/Conversion/IndexToSPIRV/index-to-spirv.mlir
+++ b/mlir/test/Conversion/IndexToSPIRV/index-to-spirv.mlir
@@ -1,9 +1,10 @@
-// RUN: mlir-opt %s -convert-index-to-spirv | FileCheck %s
-// RUN: mlir-opt %s -convert-index-to-spirv=use-64bit-index=false | FileCheck %s --check-prefix=INDEX32
-// RUN: mlir-opt %s -convert-index-to-spirv=use-64bit-index=true | FileCheck %s --check-prefix=INDEX64
+// RUN: mlir-opt %s -split-input-file -convert-index-to-spirv | FileCheck %s
+// RUN: mlir-opt %s -split-input-file -convert-index-to-spirv=use-64bit-index=false | FileCheck %s --check-prefix=INDEX32
+// RUN: mlir-opt %s -split-input-file -convert-index-to-spirv=use-64bit-index=true | FileCheck %s --check-prefix=INDEX64
+// RUN: mlir-opt %s -split-input-file -convert-index-to-spirv | FileCheck %s --check-prefix=CL
 
 module attributes {
-  spirv.target_env = #spirv.target_env<#spirv.vce<v1.0, [Int64], []>, #spirv.resource_limits<>>
+  spirv.target_env = #spirv.target_env<#spirv.vce<v1.0, [Shader, Int64], []>, #spirv.resource_limits<>>
 } {
 // CHECK-LABEL: @trivial_ops
 func.func @trivial_ops(%a: index, %b: index) {
@@ -220,3 +221,24 @@ func.func @index_cast_to(%a: i32, %b: i64) -> (index, index, index, index) {
   return %0, %1, %2, %3 : index, index, index, index
 }
 }
+
+// -----
+
+// On an OpenCL/Kernel target the GLSL extended ops are unavailable, so
+// index.min/max must lower to the OpenCL extended instructions instead.
+module attributes {
+  spirv.target_env = #spirv.target_env<#spirv.vce<v1.0, [Kernel, Int64], []>, #spirv.resource_limits<>>
+} {
+// CL-LABEL: @min_max_ops
+func.func @min_max_ops(%a: index, %b: index) {
+  // CL: spirv.CL.s_max
+  %0 = index.maxs %a, %b
+  // CL: spirv.CL.u_max
+  %1 = index.maxu %a, %b
+  // CL: spirv.CL.s_min
+  %2 = index.mins %a, %b
+  // CL: spirv.CL.u_min
+  %3 = index.minu %a, %b
+  return
+}
+}

``````````

</details>


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