Skip to content

Commit

Permalink
[mlir][openacc] Cleanup acc.data from old data clause operands
Browse files Browse the repository at this point in the history 
Since the new data operand operations have been added in D148389 and
adopted on acc.data in D149673, the old clause operands are no longer
needed.

The LegalizeDataOpForLLVMTranslation will become obsolete when all
operations will be cleaned. For the time being only the appropriate
part are being removed.

processOperands will also receive some updates once all the operands
will be coming from an acc data operand operation.

Reviewed By: razvanlupusoru

Differential Revision: https://reviews.llvm.org/D150155
  • Loading branch information
@clementval
clementval committed May 9, 2023
1 parent f08b94d commit 46e1b09
Show file tree
Hide file tree
Showing 10 changed files with 258 additions and 275 deletions.
1 change: 0 additions & 1 deletion flang/lib/Lower/OpenACC.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1122,7 +1122,6 @@ static void genACCDataOp(Fortran::lower::AbstractConverter &converter,
llvm::SmallVector<mlir::Value> operands;
llvm::SmallVector<int32_t> operandSegments;
addOperand(operands, operandSegments, ifCond);
operandSegments.append({0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0});
addOperands(operands, operandSegments, dataClauseOperands);

auto dataOp = createRegionOp<mlir::acc::DataOp, mlir::acc::TerminatorOp>(
Expand Down
16 changes: 0 additions & 16 deletions flang/lib/Optimizer/Transforms/OpenACC/OpenACCDataOperandConversion.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ void OpenACCDataOperandConversion::runOnOperation() {
options.useOpaquePointers = useOpaquePointers;
fir::LLVMTypeConverter converter(
op.getOperation()->getParentOfType<mlir::ModuleOp>(), true);
patterns.add<LegalizeDataOpForLLVMTranslation<acc::DataOp>>(converter);
patterns.add<LegalizeDataOpForLLVMTranslation<acc::ParallelOp>>(converter);

ConversionTarget target(*context);
Expand All @@ -131,21 +130,6 @@ void OpenACCDataOperandConversion::runOnOperation() {
return true;
};

target.addDynamicallyLegalOp<acc::DataOp>(
[allDataOperandsAreConverted](acc::DataOp op) {
return allDataOperandsAreConverted(op.getCopyOperands()) &&
allDataOperandsAreConverted(op.getCopyinOperands()) &&
allDataOperandsAreConverted(op.getCopyinReadonlyOperands()) &&
allDataOperandsAreConverted(op.getCopyoutOperands()) &&
allDataOperandsAreConverted(op.getCopyoutZeroOperands()) &&
allDataOperandsAreConverted(op.getCreateOperands()) &&
allDataOperandsAreConverted(op.getCreateZeroOperands()) &&
allDataOperandsAreConverted(op.getNoCreateOperands()) &&
allDataOperandsAreConverted(op.getPresentOperands()) &&
allDataOperandsAreConverted(op.getDeviceptrOperands()) &&
allDataOperandsAreConverted(op.getAttachOperands());
});

target.addDynamicallyLegalOp<acc::ParallelOp>(
[allDataOperandsAreConverted](acc::ParallelOp op) {
return allDataOperandsAreConverted(op.getReductionOperands()) &&
Expand Down
24 changes: 0 additions & 24 deletions flang/test/Transforms/OpenACC/convert-data-operands-to-llvmir.fir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,6 @@ fir.global internal @_QFEa : !fir.array<10xf32> {
fir.has_value %0 : !fir.array<10xf32>
}

func.func @_QQsub1() attributes {fir.bindc_name = "arr"} {
%0 = fir.address_of(@_QFEa) : !fir.ref<!fir.array<10xf32>>
acc.data copy(%0 : !fir.ref<!fir.array<10xf32>>) {
acc.terminator
}
return
}

// CHECK-LABEL: func.func @_QQsub1() attributes {fir.bindc_name = "arr"} {
// CHECK: %[[ADDR:.*]] = fir.address_of(@_QFEa) : !fir.ref<!fir.array<10xf32>>
// CHECK: %[[CAST:.*]] = builtin.unrealized_conversion_cast %[[ADDR]] : !fir.ref<!fir.array<10xf32>> to !llvm.ptr<array<10 x f32>>
// CHECK: acc.data copy(%[[CAST]] : !llvm.ptr<array<10 x f32>>)

// LLVMIR-LABEL: llvm.func @_QQsub1() attributes {fir.bindc_name = "arr"} {
// LLVMIR: %[[ADDR:.*]] = llvm.mlir.addressof @_QFEa : !llvm.ptr<array<10 x f32>>
// LLVMIR: acc.data copy(%[[ADDR]] : !llvm.ptr<array<10 x f32>>) {

// -----

fir.global internal @_QFEa : !fir.array<10xf32> {
%0 = fir.undefined !fir.array<10xf32>
fir.has_value %0 : !fir.array<10xf32>
}

func.func @_QQsub_parallel() attributes {fir.bindc_name = "test"} {
%0 = fir.address_of(@_QFEa) : !fir.ref<!fir.array<10xf32>>
%1 = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFEi"}
Expand Down
25 changes: 0 additions & 25 deletions mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -681,17 +681,6 @@ def OpenACC_DataOp : OpenACC_Op<"data",


let arguments = (ins Optional<I1>:$ifCond,
Variadic<AnyType>:$copyOperands,
Variadic<AnyType>:$copyinOperands,
Variadic<AnyType>:$copyinReadonlyOperands,
Variadic<AnyType>:$copyoutOperands,
Variadic<AnyType>:$copyoutZeroOperands,
Variadic<AnyType>:$createOperands,
Variadic<AnyType>:$createZeroOperands,
Variadic<AnyType>:$noCreateOperands,
Variadic<AnyType>:$presentOperands,
Variadic<AnyType>:$deviceptrOperands,
Variadic<AnyType>:$attachOperands,
Variadic<OpenACC_PointerLikeTypeInterface>:$dataClauseOperands,
OptionalAttr<DefaultValueAttr>:$defaultAttr);

Expand All @@ -709,20 +698,6 @@ def OpenACC_DataOp : OpenACC_Op<"data",
oilist(
`if` `(` $ifCond `)`
| `dataOperands` `(` $dataClauseOperands `:` type($dataClauseOperands) `)`
| `copy` `(` $copyOperands `:` type($copyOperands) `)`
| `copyin` `(` $copyinOperands `:` type($copyinOperands) `)`
| `copyin_readonly` `(` $copyinReadonlyOperands `:`
type($copyinReadonlyOperands) `)`
| `copyout` `(` $copyoutOperands `:` type($copyoutOperands) `)`
| `copyout_zero` `(` $copyoutZeroOperands `:`
type($copyoutZeroOperands) `)`
| `create` `(` $createOperands `:` type($createOperands) `)`
| `create_zero` `(` $createZeroOperands `:`
type($createZeroOperands) `)`
| `no_create` `(` $noCreateOperands `:` type($noCreateOperands) `)`
| `present` `(` $presentOperands `:` type($presentOperands) `)`
| `deviceptr` `(` $deviceptrOperands `:` type($deviceptrOperands) `)`
| `attach` `(` $attachOperands `:` type($attachOperands) `)`
)
$region attr-dict-with-keyword
}];
Expand Down
16 changes: 0 additions & 16 deletions mlir/lib/Conversion/OpenACCToLLVM/OpenACCToLLVM.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ class LegalizeDataOpForLLVMTranslation : public ConvertOpToLLVMPattern<Op> {

void mlir::populateOpenACCToLLVMConversionPatterns(
LLVMTypeConverter &converter, RewritePatternSet &patterns) {
patterns.add<LegalizeDataOpForLLVMTranslation<acc::DataOp>>(converter);
patterns.add<LegalizeDataOpForLLVMTranslation<acc::ParallelOp>>(converter);
}

Expand Down Expand Up @@ -192,21 +191,6 @@ void ConvertOpenACCToLLVMPass::runOnOperation() {
return true;
};

target.addDynamicallyLegalOp<acc::DataOp>(
[allDataOperandsAreConverted](acc::DataOp op) {
return allDataOperandsAreConverted(op.getCopyOperands()) &&
allDataOperandsAreConverted(op.getCopyinOperands()) &&
allDataOperandsAreConverted(op.getCopyinReadonlyOperands()) &&
allDataOperandsAreConverted(op.getCopyoutOperands()) &&
allDataOperandsAreConverted(op.getCopyoutZeroOperands()) &&
allDataOperandsAreConverted(op.getCreateOperands()) &&
allDataOperandsAreConverted(op.getCreateZeroOperands()) &&
allDataOperandsAreConverted(op.getNoCreateOperands()) &&
allDataOperandsAreConverted(op.getPresentOperands()) &&
allDataOperandsAreConverted(op.getDeviceptrOperands()) &&
allDataOperandsAreConverted(op.getAttachOperands());
});

target.addDynamicallyLegalOp<acc::ParallelOp>(
[allDataOperandsAreConverted](acc::ParallelOp op) {
return allDataOperandsAreConverted(op.getReductionOperands()) &&
Expand Down
9 changes: 1 addition & 8 deletions mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -520,14 +520,7 @@ LogicalResult acc::DataOp::verify() {
return success();
}

unsigned DataOp::getNumDataOperands() {
return getCopyOperands().size() + getCopyinOperands().size() +
getCopyinReadonlyOperands().size() + getCopyoutOperands().size() +
getCopyoutZeroOperands().size() + getCreateOperands().size() +
getCreateZeroOperands().size() + getNoCreateOperands().size() +
getPresentOperands().size() + getDeviceptrOperands().size() +
getAttachOperands().size() + getDataClauseOperands().size();
}
unsigned DataOp::getNumDataOperands() { return getDataClauseOperands().size(); }

Value DataOp::getDataOperand(unsigned i) {
unsigned numOptional = getIfCond() ? 1 : 0;
Expand Down
89 changes: 49 additions & 40 deletions mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,57 +306,66 @@ static LogicalResult convertDataOp(acc::DataOp &op,

// TODO handle no_create, deviceptr and attach operands.

if (failed(processOperands(
builder, moduleTranslation, op, op.getCopyOperands(), totalNbOperand,
kCopyFlag | kHoldFlag, flags, names, index, mapperAllocas)))
return failure();

if (failed(processOperands(builder, moduleTranslation, op,
op.getCopyinOperands(), totalNbOperand,
kDeviceCopyinFlag | kHoldFlag, flags, names, index,
mapperAllocas)))
return failure();
llvm::SmallVector<mlir::Value> copyin, copyout, create, present,
deleteOperands;
for (mlir::Value dataOp : op.getDataClauseOperands()) {
if (auto devicePtrOp = mlir::dyn_cast_or_null<acc::GetDevicePtrOp>(
dataOp.getDefiningOp())) {
for (auto &u : devicePtrOp.getAccPtr().getUses()) {
if (mlir::dyn_cast_or_null<acc::DeleteOp>(u.getOwner())) {
deleteOperands.push_back(devicePtrOp.getVarPtr());
} else if (mlir::dyn_cast_or_null<acc::CopyoutOp>(u.getOwner())) {
// TODO copyout zero currenlty handled as copyout. Update when
// extension available.
copyout.push_back(devicePtrOp.getVarPtr());
}
}
} else if (auto copyinOp = mlir::dyn_cast_or_null<acc::CopyinOp>(
dataOp.getDefiningOp())) {
// TODO copyin readonly currenlty handled as copyin. Update when extension
// available.
copyin.push_back(copyinOp.getVarPtr());
} else if (auto createOp = mlir::dyn_cast_or_null<acc::CreateOp>(
dataOp.getDefiningOp())) {
// TODO create zero currenlty handled as create. Update when extension
// available.
create.push_back(createOp.getVarPtr());
} else if (auto presentOp = mlir::dyn_cast_or_null<acc::PresentOp>(
dataOp.getDefiningOp())) {
present.push_back(createOp.getVarPtr());
}
}

// TODO copyin readonly currenlty handled as copyin. Update when extension
// available.
if (failed(processOperands(builder, moduleTranslation, op,
op.getCopyinReadonlyOperands(), totalNbOperand,
kDeviceCopyinFlag | kHoldFlag, flags, names, index,
mapperAllocas)))
return failure();
auto nbTotalOperands = copyin.size() + copyout.size() + create.size() +
present.size() + deleteOperands.size();

if (failed(processOperands(builder, moduleTranslation, op,
op.getCopyoutOperands(), totalNbOperand,
kHostCopyoutFlag | kHoldFlag, flags, names, index,
mapperAllocas)))
// Copyin operands are handled as `to` call.
if (failed(processOperands(builder, moduleTranslation, op, copyin,
nbTotalOperands, kDeviceCopyinFlag | kHoldFlag,
flags, names, index, mapperAllocas)))
return failure();

// TODO copyout zero currenlty handled as copyout. Update when extension
// available.
if (failed(processOperands(builder, moduleTranslation, op,
op.getCopyoutZeroOperands(), totalNbOperand,
kHostCopyoutFlag | kHoldFlag, flags, names, index,
// Delete operands are handled as `delete` call.
if (failed(processOperands(builder, moduleTranslation, op, deleteOperands,
nbTotalOperands, kDeleteFlag, flags, names, index,
mapperAllocas)))
return failure();

if (failed(processOperands(builder, moduleTranslation, op,
op.getCreateOperands(), totalNbOperand,
kCreateFlag | kHoldFlag, flags, names, index,
mapperAllocas)))
// Copyout operands are handled as `from` call.
if (failed(processOperands(builder, moduleTranslation, op, copyout,
nbTotalOperands, kHostCopyoutFlag | kHoldFlag,
flags, names, index, mapperAllocas)))
return failure();

// TODO create zero currenlty handled as create. Update when extension
// available.
if (failed(processOperands(builder, moduleTranslation, op,
op.getCreateZeroOperands(), totalNbOperand,
kCreateFlag | kHoldFlag, flags, names, index,
mapperAllocas)))
// Create operands are handled as `alloc` call.
if (failed(processOperands(builder, moduleTranslation, op, create,
nbTotalOperands, kCreateFlag | kHoldFlag, flags,
names, index, mapperAllocas)))
return failure();

if (failed(processOperands(builder, moduleTranslation, op,
op.getPresentOperands(), totalNbOperand,
kPresentFlag | kHoldFlag, flags, names, index,
mapperAllocas)))
if (failed(processOperands(builder, moduleTranslation, op, present,
nbTotalOperands, kPresentFlag | kHoldFlag, flags,
names, index, mapperAllocas)))
return failure();

llvm::GlobalVariable *maptypes =
Expand Down
58 changes: 0 additions & 58 deletions mlir/test/Conversion/OpenACCToLLVM/convert-data-operands-to-llvmir.mlir
View file
Original file line number Diff line number Diff line change
@@ -1,63 +1,5 @@
// RUN: mlir-opt -convert-openacc-to-llvm='use-opaque-pointers=1' -split-input-file %s | FileCheck %s

func.func @testdataregion(%a: memref<10xf32>, %b: memref<10xf32>) -> () {
acc.data copy(%b : memref<10xf32>) copyout(%a : memref<10xf32>) {
acc.parallel {
acc.yield
}
acc.terminator
}
return
}

// CHECK: acc.data copy(%{{.*}} : !llvm.struct<"openacc_data", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>) copyout(%{{.*}} : !llvm.struct<"openacc_data.1", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>)
// CHECK: acc.parallel
// CHECK: acc.yield
// CHECK: acc.terminator

// -----

func.func @testdataregion(%a: !llvm.ptr, %b: memref<10xf32>, %c: !llvm.ptr) -> () {
acc.data copyin(%b : memref<10xf32>) deviceptr(%c: !llvm.ptr) attach(%a : !llvm.ptr) {
}
return
}

// CHECK: acc.data copyin(%{{.*}} : !llvm.struct<"openacc_data", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>) deviceptr(%{{.*}} : !llvm.ptr) attach(%{{.*}} : !llvm.ptr)

// -----

func.func @testdataregion(%a: memref<10xf32>, %b: memref<10xf32>) -> () {
%ifCond = arith.constant true
acc.data if(%ifCond) copyin_readonly(%b : memref<10xf32>) copyout_zero(%a : memref<10xf32>) {
}
return
}

// CHECK: acc.data if(%{{.*}}) copyin_readonly(%{{.*}} : !llvm.struct<"openacc_data", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>) copyout_zero(%{{.*}} : !llvm.struct<"openacc_data.1", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>)

// -----

func.func @testdataregion(%a: !llvm.ptr, %b: memref<10xf32>, %c: !llvm.ptr) -> () {
acc.data create(%b : memref<10xf32>) create_zero(%c: !llvm.ptr) no_create(%a : !llvm.ptr) {
}
return
}

// CHECK: acc.data create(%{{.*}} : !llvm.struct<"openacc_data", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>) create_zero(%{{.*}} : !llvm.ptr) no_create(%{{.*}} : !llvm.ptr)

// -----

func.func @testdataregion(%a: memref<10xf32>, %b: memref<10xf32>) -> () {
acc.data present(%a, %b : memref<10xf32>, memref<10xf32>) {
}
return
}

// CHECK: acc.data present(%{{.*}}, %{{.*}} : !llvm.struct<"openacc_data", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>, !llvm.struct<"openacc_data.1", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>)

// -----

func.func @testparallelop(%a: memref<10xf32>, %b: memref<10xf32>) -> () {
acc.parallel copy(%b : memref<10xf32>) copyout(%a : memref<10xf32>) {
}
Expand Down
Loading

0 comments on commit 46e1b09

Please sign in to comment.