[flang][NFC] move extractSequenceType helper out of OpenACC to share code #84957
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tblah
requested review from
DavidTruby,
clementval,
kiranchandramohan,
Leporacanthicus,
jeanPerier and
vzakhari
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir Author: Tom Eccles (tblah) ChangesMoving extractSequenceType to FIRType.h so that this can also be used from OpenMP. OpenMP array reductions 5/6 Full diff: https://github.com/llvm/llvm-project/pull/84957.diff 3 Files Affected:
diff --git a/flang/include/flang/Optimizer/Dialect/FIRType.h b/flang/include/flang/Optimizer/Dialect/FIRType.h
index a526b4ddf3b98c..7fcd9c1babf24f 100644
--- a/flang/include/flang/Optimizer/Dialect/FIRType.h
+++ b/flang/include/flang/Optimizer/Dialect/FIRType.h
@@ -237,6 +237,9 @@ inline mlir::Type unwrapSequenceType(mlir::Type t) {
return t;
}
+/// Return the nested sequence type if any.
+mlir::Type extractSequenceType(mlir::Type ty);
+
inline mlir::Type unwrapRefType(mlir::Type t) {
if (auto eleTy = dyn_cast_ptrEleTy(t))
return eleTy;
diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp
index d2c6006ecf914a..6539de4d88304c 100644
--- a/flang/lib/Lower/OpenACC.cpp
+++ b/flang/lib/Lower/OpenACC.cpp
@@ -406,19 +406,6 @@ fir::ShapeOp genShapeOp(mlir::OpBuilder &builder, fir::SequenceType seqTy,
return builder.create<fir::ShapeOp>(loc, extents);
}
-/// Return the nested sequence type if any.
-static mlir::Type extractSequenceType(mlir::Type ty) {
- if (mlir::isa<fir::SequenceType>(ty))
- return ty;
- if (auto boxTy = mlir::dyn_cast<fir::BaseBoxType>(ty))
- return extractSequenceType(boxTy.getEleTy());
- if (auto heapTy = mlir::dyn_cast<fir::HeapType>(ty))
- return extractSequenceType(heapTy.getEleTy());
- if (auto ptrTy = mlir::dyn_cast<fir::PointerType>(ty))
- return extractSequenceType(ptrTy.getEleTy());
- return mlir::Type{};
-}
-
template <typename RecipeOp>
static void genPrivateLikeInitRegion(mlir::OpBuilder &builder, RecipeOp recipe,
mlir::Type ty, mlir::Location loc) {
@@ -454,7 +441,7 @@ static void genPrivateLikeInitRegion(mlir::OpBuilder &builder, RecipeOp recipe,
}
}
} else if (auto boxTy = mlir::dyn_cast_or_null<fir::BaseBoxType>(ty)) {
- mlir::Type innerTy = extractSequenceType(boxTy);
+ mlir::Type innerTy = fir::extractSequenceType(boxTy);
if (!innerTy)
TODO(loc, "Unsupported boxed type in OpenACC privatization");
fir::FirOpBuilder firBuilder{builder, recipe.getOperation()};
@@ -688,7 +675,7 @@ mlir::acc::FirstprivateRecipeOp Fortran::lower::createOrGetFirstprivateRecipe(
} else if (auto boxTy = mlir::dyn_cast_or_null<fir::BaseBoxType>(ty)) {
fir::FirOpBuilder firBuilder{builder, recipe.getOperation()};
llvm::SmallVector<mlir::Value> tripletArgs;
- mlir::Type innerTy = extractSequenceType(boxTy);
+ mlir::Type innerTy = fir::extractSequenceType(boxTy);
fir::SequenceType seqTy =
mlir::dyn_cast_or_null<fir::SequenceType>(innerTy);
if (!seqTy)
@@ -1018,7 +1005,7 @@ static mlir::Value genReductionInitRegion(fir::FirOpBuilder &builder,
return declareOp.getBase();
}
} else if (auto boxTy = mlir::dyn_cast_or_null<fir::BaseBoxType>(ty)) {
- mlir::Type innerTy = extractSequenceType(boxTy);
+ mlir::Type innerTy = fir::extractSequenceType(boxTy);
if (!mlir::isa<fir::SequenceType>(innerTy))
TODO(loc, "Unsupported boxed type for reduction");
// Create the private copy from the initial fir.box.
@@ -1230,7 +1217,7 @@ static void genCombiner(fir::FirOpBuilder &builder, mlir::Location loc,
builder.create<fir::StoreOp>(loc, res, addr1);
builder.setInsertionPointAfter(loops[0]);
} else if (auto boxTy = mlir::dyn_cast<fir::BaseBoxType>(ty)) {
- mlir::Type innerTy = extractSequenceType(boxTy);
+ mlir::Type innerTy = fir::extractSequenceType(boxTy);
fir::SequenceType seqTy =
mlir::dyn_cast_or_null<fir::SequenceType>(innerTy);
if (!seqTy)
diff --git a/flang/lib/Optimizer/Dialect/FIRType.cpp b/flang/lib/Optimizer/Dialect/FIRType.cpp
index 8a2c681d958609..5c4cad6d208344 100644
--- a/flang/lib/Optimizer/Dialect/FIRType.cpp
+++ b/flang/lib/Optimizer/Dialect/FIRType.cpp
@@ -254,6 +254,18 @@ bool hasDynamicSize(mlir::Type t) {
return false;
}
+mlir::Type extractSequenceType(mlir::Type ty) {
+ if (mlir::isa<fir::SequenceType>(ty))
+ return ty;
+ if (auto boxTy = mlir::dyn_cast<fir::BaseBoxType>(ty))
+ return extractSequenceType(boxTy.getEleTy());
+ if (auto heapTy = mlir::dyn_cast<fir::HeapType>(ty))
+ return extractSequenceType(heapTy.getEleTy());
+ if (auto ptrTy = mlir::dyn_cast<fir::PointerType>(ty))
+ return extractSequenceType(ptrTy.getEleTy());
+ return mlir::Type{};
+}
+
bool isPointerType(mlir::Type ty) {
if (auto refTy = fir::dyn_cast_ptrEleTy(ty))
ty = refTy;
|
|
@llvm/pr-subscribers-openacc Author: Tom Eccles (tblah) ChangesMoving extractSequenceType to FIRType.h so that this can also be used from OpenMP. OpenMP array reductions 5/6 Full diff: https://github.com/llvm/llvm-project/pull/84957.diff 3 Files Affected:
diff --git a/flang/include/flang/Optimizer/Dialect/FIRType.h b/flang/include/flang/Optimizer/Dialect/FIRType.h
index a526b4ddf3b98c..7fcd9c1babf24f 100644
--- a/flang/include/flang/Optimizer/Dialect/FIRType.h
+++ b/flang/include/flang/Optimizer/Dialect/FIRType.h
@@ -237,6 +237,9 @@ inline mlir::Type unwrapSequenceType(mlir::Type t) {
return t;
}
+/// Return the nested sequence type if any.
+mlir::Type extractSequenceType(mlir::Type ty);
+
inline mlir::Type unwrapRefType(mlir::Type t) {
if (auto eleTy = dyn_cast_ptrEleTy(t))
return eleTy;
diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp
index d2c6006ecf914a..6539de4d88304c 100644
--- a/flang/lib/Lower/OpenACC.cpp
+++ b/flang/lib/Lower/OpenACC.cpp
@@ -406,19 +406,6 @@ fir::ShapeOp genShapeOp(mlir::OpBuilder &builder, fir::SequenceType seqTy,
return builder.create<fir::ShapeOp>(loc, extents);
}
-/// Return the nested sequence type if any.
-static mlir::Type extractSequenceType(mlir::Type ty) {
- if (mlir::isa<fir::SequenceType>(ty))
- return ty;
- if (auto boxTy = mlir::dyn_cast<fir::BaseBoxType>(ty))
- return extractSequenceType(boxTy.getEleTy());
- if (auto heapTy = mlir::dyn_cast<fir::HeapType>(ty))
- return extractSequenceType(heapTy.getEleTy());
- if (auto ptrTy = mlir::dyn_cast<fir::PointerType>(ty))
- return extractSequenceType(ptrTy.getEleTy());
- return mlir::Type{};
-}
-
template <typename RecipeOp>
static void genPrivateLikeInitRegion(mlir::OpBuilder &builder, RecipeOp recipe,
mlir::Type ty, mlir::Location loc) {
@@ -454,7 +441,7 @@ static void genPrivateLikeInitRegion(mlir::OpBuilder &builder, RecipeOp recipe,
}
}
} else if (auto boxTy = mlir::dyn_cast_or_null<fir::BaseBoxType>(ty)) {
- mlir::Type innerTy = extractSequenceType(boxTy);
+ mlir::Type innerTy = fir::extractSequenceType(boxTy);
if (!innerTy)
TODO(loc, "Unsupported boxed type in OpenACC privatization");
fir::FirOpBuilder firBuilder{builder, recipe.getOperation()};
@@ -688,7 +675,7 @@ mlir::acc::FirstprivateRecipeOp Fortran::lower::createOrGetFirstprivateRecipe(
} else if (auto boxTy = mlir::dyn_cast_or_null<fir::BaseBoxType>(ty)) {
fir::FirOpBuilder firBuilder{builder, recipe.getOperation()};
llvm::SmallVector<mlir::Value> tripletArgs;
- mlir::Type innerTy = extractSequenceType(boxTy);
+ mlir::Type innerTy = fir::extractSequenceType(boxTy);
fir::SequenceType seqTy =
mlir::dyn_cast_or_null<fir::SequenceType>(innerTy);
if (!seqTy)
@@ -1018,7 +1005,7 @@ static mlir::Value genReductionInitRegion(fir::FirOpBuilder &builder,
return declareOp.getBase();
}
} else if (auto boxTy = mlir::dyn_cast_or_null<fir::BaseBoxType>(ty)) {
- mlir::Type innerTy = extractSequenceType(boxTy);
+ mlir::Type innerTy = fir::extractSequenceType(boxTy);
if (!mlir::isa<fir::SequenceType>(innerTy))
TODO(loc, "Unsupported boxed type for reduction");
// Create the private copy from the initial fir.box.
@@ -1230,7 +1217,7 @@ static void genCombiner(fir::FirOpBuilder &builder, mlir::Location loc,
builder.create<fir::StoreOp>(loc, res, addr1);
builder.setInsertionPointAfter(loops[0]);
} else if (auto boxTy = mlir::dyn_cast<fir::BaseBoxType>(ty)) {
- mlir::Type innerTy = extractSequenceType(boxTy);
+ mlir::Type innerTy = fir::extractSequenceType(boxTy);
fir::SequenceType seqTy =
mlir::dyn_cast_or_null<fir::SequenceType>(innerTy);
if (!seqTy)
diff --git a/flang/lib/Optimizer/Dialect/FIRType.cpp b/flang/lib/Optimizer/Dialect/FIRType.cpp
index 8a2c681d958609..5c4cad6d208344 100644
--- a/flang/lib/Optimizer/Dialect/FIRType.cpp
+++ b/flang/lib/Optimizer/Dialect/FIRType.cpp
@@ -254,6 +254,18 @@ bool hasDynamicSize(mlir::Type t) {
return false;
}
+mlir::Type extractSequenceType(mlir::Type ty) {
+ if (mlir::isa<fir::SequenceType>(ty))
+ return ty;
+ if (auto boxTy = mlir::dyn_cast<fir::BaseBoxType>(ty))
+ return extractSequenceType(boxTy.getEleTy());
+ if (auto heapTy = mlir::dyn_cast<fir::HeapType>(ty))
+ return extractSequenceType(heapTy.getEleTy());
+ if (auto ptrTy = mlir::dyn_cast<fir::PointerType>(ty))
+ return extractSequenceType(ptrTy.getEleTy());
+ return mlir::Type{};
+}
+
bool isPointerType(mlir::Type ty) {
if (auto refTy = fir::dyn_cast_ptrEleTy(ty))
ty = refTy;
|
clementval
approved these changes
Mar 12, 2024
This was referenced Mar 12, 2024
jeanPerier
approved these changes
Mar 13, 2024
tblah
force-pushed
the
users/tblah/omp_array_reduction_4
branch
2 times, most recently
from
02550e1
to
a726997
Compare
tblah
added a commit
that referenced
this pull request
Mar 20, 2024
…code Moving extractSequenceType to FIRType.h so that this can also be used from OpenMP.
tblah
force-pushed
the
users/tblah/omp_array_reduction_5
branch
from
2ff12fa
to
d4485b1
Compare
tblah
added a commit
that referenced
this pull request
Mar 20, 2024
This has been tested with arrays with compile-time constant bounds. Allocatable arrays and arrays with non-constant bounds are not yet supported. User-defined reduction functions are also not yet supported. The design is intended to work for arrays with non-constant bounds too without a lot of extra work (mostly there are bugs in OpenMPIRBuilder I haven't fixed yet). We need some way to get these runtime bounds into the reduction init and combiner regions. To keep things simple for now I opted to always box the array arguments so the box can be passed as one argument and the lower bounds and extents read from the box. This has the disadvantage of resulting in fir.box_dim operations inside of the critical section. If these prove to be a performance issue, we could follow OpenACC reading box lower bounds and extents before the reduction and passing them as block arguments to the reduction init and combiner regions. I would prefer to keep things simple for now. Note: this implementation only works when the HLFIR lowering is used. I don't think it is worth supporting FIR-only lowering because the plan is for that to be removed soon. OpenMP array reductions 6/6 Previous PR: #84957
chencha3
pushed a commit
to chencha3/llvm-project
that referenced
this pull request
Mar 23, 2024
It looks like the mappings for call instructions were forgotten here. This fixes a bug in OpenMP when in-lining a region containing call operations multiple times. OpenMP array reductions 4/6 Previous PR: llvm#84954 Next PR: llvm#84957
chencha3
pushed a commit
to chencha3/llvm-project
that referenced
this pull request
Mar 23, 2024
…code (llvm#84957) Moving extractSequenceType to FIRType.h so that this can also be used from OpenMP. OpenMP array reductions 5/6 Previous PR: llvm#84955 Next PR: llvm#84958
chencha3
pushed a commit
to chencha3/llvm-project
that referenced
this pull request
Mar 23, 2024
This has been tested with arrays with compile-time constant bounds. Allocatable arrays and arrays with non-constant bounds are not yet supported. User-defined reduction functions are also not yet supported. The design is intended to work for arrays with non-constant bounds too without a lot of extra work (mostly there are bugs in OpenMPIRBuilder I haven't fixed yet). We need some way to get these runtime bounds into the reduction init and combiner regions. To keep things simple for now I opted to always box the array arguments so the box can be passed as one argument and the lower bounds and extents read from the box. This has the disadvantage of resulting in fir.box_dim operations inside of the critical section. If these prove to be a performance issue, we could follow OpenACC reading box lower bounds and extents before the reduction and passing them as block arguments to the reduction init and combiner regions. I would prefer to keep things simple for now. Note: this implementation only works when the HLFIR lowering is used. I don't think it is worth supporting FIR-only lowering because the plan is for that to be removed soon. OpenMP array reductions 6/6 Previous PR: llvm#84957
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Moving extractSequenceType to FIRType.h so that this can also be used from OpenMP.
OpenMP array reductions 5/6
Previous PR: #84955
Next PR: #84958