[Mlir-commits] [mlir] [MLIR][SCFToOpenMP] Remove typed pointer support (PR #71028)
Christian Ulmann
llvmlistbot at llvm.org
Wed Nov 1 23:51:37 PDT 2023
https://github.com/Dinistro created https://github.com/llvm/llvm-project/pull/71028
This commit removes the support for lowering SCF to OpenMP dialect with typed pointers. Typed pointers have been deprecated for a while now and it's planned to soon remove them from the LLVM dialect.
Related PSA: https://discourse.llvm.org/t/psa-removal-of-typed-pointers-from-the-llvm-dialect/74502
>From b7aec33aedf9bd48a9027abb9e29d2af687ed8e1 Mon Sep 17 00:00:00 2001
From: Christian Ulmann <christian.ulmann at nextsilicon.com>
Date: Thu, 2 Nov 2023 06:48:26 +0000
Subject: [PATCH] [MLIR][SCFToOpenMP] Remove typed pointer support
This commit removes the support for lowering SCF to OpenMP dialect
with typed pointers. Typed pointers have been deprecated for a while
now and it's planned to soon remove them from the LLVM dialect.
Related PSA: https://discourse.llvm.org/t/psa-removal-of-typed-pointers-from-the-llvm-dialect/74502
---
mlir/include/mlir/Conversion/Passes.td | 6 --
.../Conversion/SCFToOpenMP/SCFToOpenMP.cpp | 59 +++++---------
.../Conversion/SCFToOpenMP/reductions.mlir | 2 +-
.../Conversion/SCFToOpenMP/scf-to-openmp.mlir | 2 +-
.../SCFToOpenMP/typed-pointers.mlir | 78 -------------------
5 files changed, 21 insertions(+), 126 deletions(-)
delete mode 100644 mlir/test/Conversion/SCFToOpenMP/typed-pointers.mlir
diff --git a/mlir/include/mlir/Conversion/Passes.td b/mlir/include/mlir/Conversion/Passes.td
index d7253212e5fe3f8..781aa47cb1c65ae 100644
--- a/mlir/include/mlir/Conversion/Passes.td
+++ b/mlir/include/mlir/Conversion/Passes.td
@@ -887,12 +887,6 @@ def ConvertSCFToOpenMPPass : Pass<"convert-scf-to-openmp", "ModuleOp"> {
let summary = "Convert SCF parallel loop to OpenMP parallel + workshare "
"constructs.";
- let options = [
- Option<"useOpaquePointers", "use-opaque-pointers", "bool",
- /*default=*/"true", "Generate LLVM IR using opaque pointers "
- "instead of typed pointers">
- ];
-
let dependentDialects = ["omp::OpenMPDialect", "LLVM::LLVMDialect",
"memref::MemRefDialect"];
}
diff --git a/mlir/lib/Conversion/SCFToOpenMP/SCFToOpenMP.cpp b/mlir/lib/Conversion/SCFToOpenMP/SCFToOpenMP.cpp
index 50087751053ba65..ee3ee02cf535e13 100644
--- a/mlir/lib/Conversion/SCFToOpenMP/SCFToOpenMP.cpp
+++ b/mlir/lib/Conversion/SCFToOpenMP/SCFToOpenMP.cpp
@@ -211,25 +211,14 @@ static omp::ReductionDeclareOp createDecl(PatternRewriter &builder,
return decl;
}
-/// Returns an LLVM pointer type with the given element type, or an opaque
-/// pointer if 'useOpaquePointers' is true.
-static LLVM::LLVMPointerType getPointerType(Type elementType,
- bool useOpaquePointers) {
- if (useOpaquePointers)
- return LLVM::LLVMPointerType::get(elementType.getContext());
- return LLVM::LLVMPointerType::get(elementType);
-}
-
/// Adds an atomic reduction combiner to the given OpenMP reduction declaration
/// using llvm.atomicrmw of the given kind.
static omp::ReductionDeclareOp addAtomicRMW(OpBuilder &builder,
LLVM::AtomicBinOp atomicKind,
omp::ReductionDeclareOp decl,
- scf::ReduceOp reduce,
- bool useOpaquePointers) {
+ scf::ReduceOp reduce) {
OpBuilder::InsertionGuard guard(builder);
- Type type = reduce.getOperand().getType();
- Type ptrType = getPointerType(type, useOpaquePointers);
+ auto ptrType = LLVM::LLVMPointerType::get(builder.getContext());
Location reduceOperandLoc = reduce.getOperand().getLoc();
builder.createBlock(&decl.getAtomicReductionRegion(),
decl.getAtomicReductionRegion().end(), {ptrType, ptrType},
@@ -250,8 +239,7 @@ static omp::ReductionDeclareOp addAtomicRMW(OpBuilder &builder,
/// the neutral value, necessary for the OpenMP declaration. If the reduction
/// cannot be recognized, returns null.
static omp::ReductionDeclareOp declareReduction(PatternRewriter &builder,
- scf::ReduceOp reduce,
- bool useOpaquePointers) {
+ scf::ReduceOp reduce) {
Operation *container = SymbolTable::getNearestSymbolTable(reduce);
SymbolTable symbolTable(container);
@@ -272,34 +260,29 @@ static omp::ReductionDeclareOp declareReduction(PatternRewriter &builder,
if (matchSimpleReduction<arith::AddFOp, LLVM::FAddOp>(reduction)) {
omp::ReductionDeclareOp decl = createDecl(builder, symbolTable, reduce,
builder.getFloatAttr(type, 0.0));
- return addAtomicRMW(builder, LLVM::AtomicBinOp::fadd, decl, reduce,
- useOpaquePointers);
+ return addAtomicRMW(builder, LLVM::AtomicBinOp::fadd, decl, reduce);
}
if (matchSimpleReduction<arith::AddIOp, LLVM::AddOp>(reduction)) {
omp::ReductionDeclareOp decl = createDecl(builder, symbolTable, reduce,
builder.getIntegerAttr(type, 0));
- return addAtomicRMW(builder, LLVM::AtomicBinOp::add, decl, reduce,
- useOpaquePointers);
+ return addAtomicRMW(builder, LLVM::AtomicBinOp::add, decl, reduce);
}
if (matchSimpleReduction<arith::OrIOp, LLVM::OrOp>(reduction)) {
omp::ReductionDeclareOp decl = createDecl(builder, symbolTable, reduce,
builder.getIntegerAttr(type, 0));
- return addAtomicRMW(builder, LLVM::AtomicBinOp::_or, decl, reduce,
- useOpaquePointers);
+ return addAtomicRMW(builder, LLVM::AtomicBinOp::_or, decl, reduce);
}
if (matchSimpleReduction<arith::XOrIOp, LLVM::XOrOp>(reduction)) {
omp::ReductionDeclareOp decl = createDecl(builder, symbolTable, reduce,
builder.getIntegerAttr(type, 0));
- return addAtomicRMW(builder, LLVM::AtomicBinOp::_xor, decl, reduce,
- useOpaquePointers);
+ return addAtomicRMW(builder, LLVM::AtomicBinOp::_xor, decl, reduce);
}
if (matchSimpleReduction<arith::AndIOp, LLVM::AndOp>(reduction)) {
omp::ReductionDeclareOp decl = createDecl(
builder, symbolTable, reduce,
builder.getIntegerAttr(
type, llvm::APInt::getAllOnes(type.getIntOrFloatBitWidth())));
- return addAtomicRMW(builder, LLVM::AtomicBinOp::_and, decl, reduce,
- useOpaquePointers);
+ return addAtomicRMW(builder, LLVM::AtomicBinOp::_and, decl, reduce);
}
// Match simple binary reductions that cannot be expressed with atomicrmw.
@@ -335,7 +318,7 @@ static omp::ReductionDeclareOp declareReduction(PatternRewriter &builder,
builder, symbolTable, reduce, minMaxValueForSignedInt(type, !isMin));
return addAtomicRMW(builder,
isMin ? LLVM::AtomicBinOp::min : LLVM::AtomicBinOp::max,
- decl, reduce, useOpaquePointers);
+ decl, reduce);
}
if (matchSelectReduction<arith::CmpIOp, arith::SelectOp>(
reduction, {arith::CmpIPredicate::ult, arith::CmpIPredicate::ule},
@@ -347,7 +330,7 @@ static omp::ReductionDeclareOp declareReduction(PatternRewriter &builder,
builder, symbolTable, reduce, minMaxValueForUnsignedInt(type, !isMin));
return addAtomicRMW(
builder, isMin ? LLVM::AtomicBinOp::umin : LLVM::AtomicBinOp::umax,
- decl, reduce, useOpaquePointers);
+ decl, reduce);
}
return nullptr;
@@ -357,11 +340,8 @@ namespace {
struct ParallelOpLowering : public OpRewritePattern<scf::ParallelOp> {
- bool useOpaquePointers;
-
- ParallelOpLowering(MLIRContext *context, bool useOpaquePointers)
- : OpRewritePattern<scf::ParallelOp>(context),
- useOpaquePointers(useOpaquePointers) {}
+ ParallelOpLowering(MLIRContext *context)
+ : OpRewritePattern<scf::ParallelOp>(context) {}
LogicalResult matchAndRewrite(scf::ParallelOp parallelOp,
PatternRewriter &rewriter) const override {
@@ -370,8 +350,7 @@ struct ParallelOpLowering : public OpRewritePattern<scf::ParallelOp> {
// declaration and use it instead of redeclaring.
SmallVector<Attribute> reductionDeclSymbols;
for (auto reduce : parallelOp.getOps<scf::ReduceOp>()) {
- omp::ReductionDeclareOp decl =
- declareReduction(rewriter, reduce, useOpaquePointers);
+ omp::ReductionDeclareOp decl = declareReduction(rewriter, reduce);
if (!decl)
return failure();
reductionDeclSymbols.push_back(
@@ -385,14 +364,14 @@ struct ParallelOpLowering : public OpRewritePattern<scf::ParallelOp> {
loc, rewriter.getIntegerType(64), rewriter.getI64IntegerAttr(1));
SmallVector<Value> reductionVariables;
reductionVariables.reserve(parallelOp.getNumReductions());
+ auto ptrType = LLVM::LLVMPointerType::get(parallelOp.getContext());
for (Value init : parallelOp.getInitVals()) {
assert((LLVM::isCompatibleType(init.getType()) ||
isa<LLVM::PointerElementTypeInterface>(init.getType())) &&
"cannot create a reduction variable if the type is not an LLVM "
"pointer element");
- Value storage = rewriter.create<LLVM::AllocaOp>(
- loc, getPointerType(init.getType(), useOpaquePointers),
- init.getType(), one, 0);
+ Value storage =
+ rewriter.create<LLVM::AllocaOp>(loc, ptrType, init.getType(), one, 0);
rewriter.create<LLVM::StoreOp>(loc, init, storage);
reductionVariables.push_back(storage);
}
@@ -464,14 +443,14 @@ struct ParallelOpLowering : public OpRewritePattern<scf::ParallelOp> {
};
/// Applies the conversion patterns in the given function.
-static LogicalResult applyPatterns(ModuleOp module, bool useOpaquePointers) {
+static LogicalResult applyPatterns(ModuleOp module) {
ConversionTarget target(*module.getContext());
target.addIllegalOp<scf::ReduceOp, scf::ReduceReturnOp, scf::ParallelOp>();
target.addLegalDialect<omp::OpenMPDialect, LLVM::LLVMDialect,
memref::MemRefDialect>();
RewritePatternSet patterns(module.getContext());
- patterns.add<ParallelOpLowering>(module.getContext(), useOpaquePointers);
+ patterns.add<ParallelOpLowering>(module.getContext());
FrozenRewritePatternSet frozen(std::move(patterns));
return applyPartialConversion(module, target, frozen);
}
@@ -484,7 +463,7 @@ struct SCFToOpenMPPass
/// Pass entry point.
void runOnOperation() override {
- if (failed(applyPatterns(getOperation(), useOpaquePointers)))
+ if (failed(applyPatterns(getOperation())))
signalPassFailure();
}
};
diff --git a/mlir/test/Conversion/SCFToOpenMP/reductions.mlir b/mlir/test/Conversion/SCFToOpenMP/reductions.mlir
index c5723d96d4f1de2..25b18b58a6adbd1 100644
--- a/mlir/test/Conversion/SCFToOpenMP/reductions.mlir
+++ b/mlir/test/Conversion/SCFToOpenMP/reductions.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt -convert-scf-to-openmp='use-opaque-pointers=1' -split-input-file %s | FileCheck %s
+// RUN: mlir-opt -convert-scf-to-openmp -split-input-file %s | FileCheck %s
// CHECK: omp.reduction.declare @[[$REDF:.*]] : f32
diff --git a/mlir/test/Conversion/SCFToOpenMP/scf-to-openmp.mlir b/mlir/test/Conversion/SCFToOpenMP/scf-to-openmp.mlir
index 508052d483ec1d7..e0fdcae1b896b8a 100644
--- a/mlir/test/Conversion/SCFToOpenMP/scf-to-openmp.mlir
+++ b/mlir/test/Conversion/SCFToOpenMP/scf-to-openmp.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt -convert-scf-to-openmp='use-opaque-pointers=1' %s | FileCheck %s
+// RUN: mlir-opt -convert-scf-to-openmp %s | FileCheck %s
// CHECK-LABEL: @parallel
func.func @parallel(%arg0: index, %arg1: index, %arg2: index,
diff --git a/mlir/test/Conversion/SCFToOpenMP/typed-pointers.mlir b/mlir/test/Conversion/SCFToOpenMP/typed-pointers.mlir
deleted file mode 100644
index fb90c5d7d10fd1f..000000000000000
--- a/mlir/test/Conversion/SCFToOpenMP/typed-pointers.mlir
+++ /dev/null
@@ -1,78 +0,0 @@
-// RUN: mlir-opt -convert-scf-to-openmp='use-opaque-pointers=0' -split-input-file %s | FileCheck %s
-
-// CHECK: omp.reduction.declare @[[$REDF1:.*]] : f32
-
-// CHECK: init
-// CHECK: %[[INIT:.*]] = llvm.mlir.constant(-3.4
-// CHECK: omp.yield(%[[INIT]] : f32)
-
-// CHECK: combiner
-// CHECK: ^{{.*}}(%[[ARG0:.*]]: f32, %[[ARG1:.*]]: f32)
-// CHECK: %[[CMP:.*]] = arith.cmpf oge, %[[ARG0]], %[[ARG1]]
-// CHECK: %[[RES:.*]] = arith.select %[[CMP]], %[[ARG0]], %[[ARG1]]
-// CHECK: omp.yield(%[[RES]] : f32)
-
-// CHECK-NOT: atomic
-
-// CHECK: omp.reduction.declare @[[$REDF2:.*]] : i64
-
-// CHECK: init
-// CHECK: %[[INIT:.*]] = llvm.mlir.constant
-// CHECK: omp.yield(%[[INIT]] : i64)
-
-// CHECK: combiner
-// CHECK: ^{{.*}}(%[[ARG0:.*]]: i64, %[[ARG1:.*]]: i64)
-// CHECK: %[[CMP:.*]] = arith.cmpi slt, %[[ARG0]], %[[ARG1]]
-// CHECK: %[[RES:.*]] = arith.select %[[CMP]], %[[ARG1]], %[[ARG0]]
-// CHECK: omp.yield(%[[RES]] : i64)
-
-// CHECK: atomic
-// CHECK: ^{{.*}}(%[[ARG0:.*]]: !llvm.ptr<i64>, %[[ARG1:.*]]: !llvm.ptr<i64>):
-// CHECK: %[[RHS:.*]] = llvm.load %[[ARG1]]
-// CHECK: llvm.atomicrmw max %[[ARG0]], %[[RHS]] monotonic
-
-// CHECK-LABEL: @reduction4
-func.func @reduction4(%arg0 : index, %arg1 : index, %arg2 : index,
- %arg3 : index, %arg4 : index) -> (f32, i64) {
- %step = arith.constant 1 : index
- // CHECK: %[[ZERO:.*]] = arith.constant 0.0
- %zero = arith.constant 0.0 : f32
- // CHECK: %[[IONE:.*]] = arith.constant 1
- %ione = arith.constant 1 : i64
- // CHECK: %[[BUF1:.*]] = llvm.alloca %{{.*}} x f32
- // CHECK: llvm.store %[[ZERO]], %[[BUF1]]
- // CHECK: %[[BUF2:.*]] = llvm.alloca %{{.*}} x i64
- // CHECK: llvm.store %[[IONE]], %[[BUF2]]
-
- // CHECK: omp.parallel
- // CHECK: omp.wsloop
- // CHECK-SAME: reduction(@[[$REDF1]] -> %[[BUF1]]
- // CHECK-SAME: @[[$REDF2]] -> %[[BUF2]]
- // CHECK: memref.alloca_scope
- %res:2 = scf.parallel (%i0, %i1) = (%arg0, %arg1) to (%arg2, %arg3)
- step (%arg4, %step) init (%zero, %ione) -> (f32, i64) {
- %one = arith.constant 1.0 : f32
- // CHECK: omp.reduction %{{.*}}, %[[BUF1]]
- scf.reduce(%one) : f32 {
- ^bb0(%lhs : f32, %rhs: f32):
- %cmp = arith.cmpf oge, %lhs, %rhs : f32
- %res = arith.select %cmp, %lhs, %rhs : f32
- scf.reduce.return %res : f32
- }
- // CHECK: arith.fptosi
- %1 = arith.fptosi %one : f32 to i64
- // CHECK: omp.reduction %{{.*}}, %[[BUF2]]
- scf.reduce(%1) : i64 {
- ^bb1(%lhs: i64, %rhs: i64):
- %cmp = arith.cmpi slt, %lhs, %rhs : i64
- %res = arith.select %cmp, %rhs, %lhs : i64
- scf.reduce.return %res : i64
- }
- // CHECK: omp.yield
- }
- // CHECK: omp.terminator
- // CHECK: %[[RES1:.*]] = llvm.load %[[BUF1]] : !llvm.ptr<f32>
- // CHECK: %[[RES2:.*]] = llvm.load %[[BUF2]] : !llvm.ptr<i64>
- // CHECK: return %[[RES1]], %[[RES2]]
- return %res#0, %res#1 : f32, i64
-}
More information about the Mlir-commits
mailing list