[Flang][MLIR][OpenMP] Move host op filtering to the omp dialect#208189
Open
skatrak wants to merge 1 commit into
Open
[Flang][MLIR][OpenMP] Move host op filtering to the omp dialect#208189skatrak wants to merge 1 commit into
skatrak wants to merge 1 commit into
Conversation
The MLIR pass that removes operations exclusively intended for the host from OpenMP target offload modules is currently defined as part of Flang. However, this is a feature that would benefit from being reusable by other frontends, as removing such operations is a requirement for all OpenMP target device modules prior to LLVM IR translation. By moving the `omp-host-op-filtering` pass out of Flang, it had to be updated to work on a lower-level LLVM dialect-based representation, rather than FIR. This simplified some of the existing edge cases, such as `fir.declare` ops and `fir.boxchar` type handling. In addition, new function arguments are introduced as placeholders and return values from host-only functions are removed, producing a cleaner result and simplifying the pass as compared to previously. As a result of a later execution of this pass, dynamic dispatch of host functions via dispatch table using `fir.dispatch`, `fir.type_info` and `fir.dt_entry` ops would break due to the removal of `fir.dt_entry` operations pointing to deleted host functions, while `fir.dispatch` ops pointing to the same functions would remain. The `omp-function-filtering` pass is updated to prevent `fir.dt_entry` from being deleted and let them point to an undefined symbol. This lets FIR lowering to LLVM work well enough to get to the host op filtering stage, where any resulting operations for this host dynamic dispatch are cleaned up.
Member
Author
llvmorg-github-actions
Bot
added
mlir
flang
|
@llvm/pr-subscribers-mlir-openmp @llvm/pr-subscribers-flang-fir-hlfir Author: Sergio Afonso (skatrak) ChangesThe MLIR pass that removes operations exclusively intended for the host from OpenMP target offload modules is currently defined as part of Flang. However, this is a feature that would benefit from being reusable by other frontends, as removing such operations is a requirement for all OpenMP target device modules prior to LLVM IR translation. By moving the As a result of a later execution of this pass, dynamic dispatch of host functions via dispatch table using Patch is 109.58 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/208189.diff 13 Files Affected:
diff --git a/flang/include/flang/Optimizer/OpenMP/Passes.td b/flang/include/flang/Optimizer/OpenMP/Passes.td
index b5d9372e356f1..9ec159e1ba1e0 100644
--- a/flang/include/flang/Optimizer/OpenMP/Passes.td
+++ b/flang/include/flang/Optimizer/OpenMP/Passes.td
@@ -57,28 +57,6 @@ def FunctionFilteringPass : Pass<"omp-function-filtering"> {
];
}
-def HostOpFilteringPass : Pass<"omp-host-op-filtering"> {
- let summary =
- "Removes host-only operations from host functions retained by the "
- "function filtering pass when compiling for a target device.";
- let description = [{
- Removes operations from host functions that are not placed inside of an
- `omp.target` region when compiling for the target device.
-
- It must run after any passes that might introduce new host operations,
- especially if they belong to the 'omp' dialect, like `LowerWorkdistribute`.
- This ensures all potentially problematic host operations are accounted for
- prior to LLVM IR translation.
-
- This pass works at the FIR level of abstraction.
- }];
- let dependentDialects = [
- "mlir::arith::ArithDialect",
- "mlir::func::FuncDialect",
- "fir::FIROpsDialect"
- ];
-}
-
def DoConcurrentConversionPass : Pass<"omp-do-concurrent-conversion", "mlir::ModuleOp"> {
let summary = "Map `DO CONCURRENT` loops to OpenMP worksharing loops.";
diff --git a/flang/lib/Optimizer/OpenMP/CMakeLists.txt b/flang/lib/Optimizer/OpenMP/CMakeLists.txt
index f25e95f041d20..db29e93b71dad 100644
--- a/flang/lib/Optimizer/OpenMP/CMakeLists.txt
+++ b/flang/lib/Optimizer/OpenMP/CMakeLists.txt
@@ -5,7 +5,6 @@ add_flang_library(FlangOpenMPTransforms
DoConcurrentConversion.cpp
FunctionFiltering.cpp
GenericLoopConversion.cpp
- HostOpFiltering.cpp
MapsForPrivatizedSymbols.cpp
MapInfoFinalization.cpp
DeleteUnreachableTargets.cpp
diff --git a/flang/lib/Optimizer/OpenMP/FunctionFiltering.cpp b/flang/lib/Optimizer/OpenMP/FunctionFiltering.cpp
index e8bcc0c7fa062..ee61dd8881fd0 100644
--- a/flang/lib/Optimizer/OpenMP/FunctionFiltering.cpp
+++ b/flang/lib/Optimizer/OpenMP/FunctionFiltering.cpp
@@ -48,8 +48,7 @@ checkDeviceImplementationStatus(omp::OffloadModuleInterface offloadModule) {
if (!redOp.getByrefElementType())
return WalkResult::advance();
- auto seqTy =
- mlir::dyn_cast<fir::SequenceType>(*redOp.getByrefElementType());
+ auto seqTy = dyn_cast<fir::SequenceType>(*redOp.getByrefElementType());
bool isByRefReductionSupported =
!seqTy || !fir::sequenceWithNonConstantShape(seqTy);
@@ -104,7 +103,7 @@ class FunctionFilteringPass
SymbolTable::UseRange funcUses = *funcOp.getSymbolUses(op);
for (SymbolTable::SymbolUse use : funcUses) {
Operation *callOp = use.getUser();
- if (auto internalFunc = mlir::dyn_cast<func::FuncOp>(callOp)) {
+ if (auto internalFunc = dyn_cast<func::FuncOp>(callOp)) {
// Do not delete internal procedures holding the symbol of their
// Fortran host procedure as attribute.
internalFunc->removeAttr(fir::getHostSymbolAttrName());
@@ -114,6 +113,15 @@ class FunctionFilteringPass
internalFunc.setVisibility(mlir::SymbolTable::Visibility::Public);
continue;
}
+ // Prevent dispatch table entries pointing to deleted functions
+ // from being removed. This prevents the lowering of any
+ // corresponding fir.dispatch ops from triggering errors. These
+ // fir.dt_entry ops will point to an undefined symbol as a result,
+ // which currently doesn't cause an issue, as fir.dispatch-related ops
+ // are later removed by the host op filtering pass.
+ if (isa<fir::DTEntryOp>(callOp))
+ continue;
+
// If the callOp has users then replace them with Undef values.
if (!callOp->use_empty()) {
SmallVector<Value> undefResults;
diff --git a/flang/lib/Optimizer/OpenMP/HostOpFiltering.cpp b/flang/lib/Optimizer/OpenMP/HostOpFiltering.cpp
deleted file mode 100644
index 05ea0bc08c4ab..0000000000000
--- a/flang/lib/Optimizer/OpenMP/HostOpFiltering.cpp
+++ /dev/null
@@ -1,422 +0,0 @@
-//===- HostOpFiltering.cpp -----------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-//
-// This file implements transforms to filter out host-only operations from
-// any remaining host functions when compiling for a target device.
-//
-//===----------------------------------------------------------------------===//
-
-#include "flang/Optimizer/Dialect/FIRDialect.h"
-#include "flang/Optimizer/Dialect/FIROps.h"
-#include "flang/Optimizer/OpenMP/Passes.h"
-
-#include "mlir/Dialect/Arith/IR/Arith.h"
-#include "mlir/Dialect/Func/IR/FuncOps.h"
-#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
-#include "mlir/Dialect/OpenMP/OpenMPInterfaces.h"
-#include "llvm/ADT/SetVector.h"
-#include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/TypeSwitch.h"
-
-namespace flangomp {
-#define GEN_PASS_DEF_HOSTOPFILTERINGPASS
-#include "flang/Optimizer/OpenMP/Passes.h.inc"
-} // namespace flangomp
-
-using namespace mlir;
-
-/// Add an operation to one of the output sets to be later rewritten.
-template <typename OpTy>
-static void collectRewrite(OpTy op, llvm::SetVector<OpTy> &rewrites) {
- rewrites.insert(op);
-}
-
-/// Add an \c omp.map.info operation and all its members recursively to the
-/// output set to be later rewritten.
-///
-/// Dependencies across \c omp.map.info are maintained by ensuring dependencies
-/// are added to the output sets before operations based on them.
-template <>
-void collectRewrite(omp::MapInfoOp mapOp,
- llvm::SetVector<omp::MapInfoOp> &rewrites) {
- for (Value member : mapOp.getMembers())
- collectRewrite(cast<omp::MapInfoOp>(member.getDefiningOp()), rewrites);
-
- rewrites.insert(mapOp);
-}
-
-/// Add the given value to a sorted set if it should be replaced by a
-/// placeholder when used as an operand that must remain for the device.
-///
-/// Values that are block arguments of \c func.func operations are skipped,
-/// since they will still be available after all rewrites are completed.
-static void collectRewrite(Value value, llvm::SetVector<Value> &rewrites) {
- if ((isa<BlockArgument>(value) &&
- isa<func::FuncOp>(
- cast<BlockArgument>(value).getOwner()->getParentOp())) ||
- rewrites.contains(value))
- return;
-
- rewrites.insert(value);
-}
-
-/// Provide the `device_type` of an `omp.declare_target` attribute, if defined.
-static std::optional<omp::DeclareTargetDeviceType>
-getDeclareTargetDevice(Operation &op) {
- auto declareTargetOp = dyn_cast<omp::DeclareTargetInterface>(op);
- if (declareTargetOp && declareTargetOp.isDeclareTarget())
- return declareTargetOp.getDeclareTargetDeviceType();
- return std::nullopt;
-}
-
-namespace {
-class HostOpFilteringPass
- : public flangomp::impl::HostOpFilteringPassBase<HostOpFilteringPass> {
-public:
- HostOpFilteringPass() = default;
-
- void runOnOperation() override {
- auto op = dyn_cast<omp::OffloadModuleInterface>(getOperation());
- if (!op || !op.getIsTargetDevice())
- return;
-
- op->walk<WalkOrder::PreOrder>([&](func::FuncOp funcOp) {
- omp::DeclareTargetDeviceType declareType =
- getDeclareTargetDevice(*funcOp.getOperation())
- .value_or(omp::DeclareTargetDeviceType::host);
-
- // Only process host function definitions.
- if (funcOp.isExternal() ||
- declareType != omp::DeclareTargetDeviceType::host)
- return WalkResult::advance();
-
- if (failed(rewriteHostFunction(funcOp))) {
- funcOp.emitOpError() << "could not filter host-only operations";
- return WalkResult::interrupt();
- }
- return WalkResult::advance();
- });
-
- // Make non-declare target globals internal for the device. They cannot be
- // deleted, because they are needed in order to properly lower map clauses.
- // However, no uses will remain in the device module, so we make them
- // internal to prevent link time issues.
- op->walk([&](fir::GlobalOp globalOp) {
- if (!getDeclareTargetDevice(*globalOp.getOperation()).has_value())
- globalOp.setLinkName("internal");
- });
- }
-
-private:
- /// Rewrite the given host device function containing \c omp.target
- /// operations, to remove host-only operations that are not used by device
- /// codegen.
- ///
- /// It is based on the expected form of the MLIR module as produced by Flang
- /// lowering, after HLFIR to FIR lowering, and it performs the following
- /// mutations:
- /// - Replace all values returned by the function with \c fir.undefined.
- /// - \c omp.target operations are moved to the end of the function. If they
- /// are nested inside of any other operations, they are hoisted out of
- /// them.
- /// - \c depend, \c device and \c if clauses are removed from these target
- /// functions. Values used to initialize other clauses are replaced by
- /// placeholders as follows:
- /// - Values defined by block arguments are replaced by placeholders only
- /// if they are not attached to the parent \c func.func operation. In
- /// that case, they are passed unmodified.
- /// - \c arith.constant and \c fir.address_of ops are maintained.
- /// - Values of type \c fir.boxchar are replaced with a combination of
- /// \c fir.alloca for a single bit and a \c fir.emboxchar.
- /// - Other values are replaced by a combination of an \c fir.alloca for a
- /// single bit and an \c fir.convert to the original type of the value.
- /// This can be done because the code eventually generated for these
- /// operations will be discarded, as they aren't runnable by the target
- /// device.
- /// - \c omp.map.info operations associated to these target regions are
- /// preserved. These are moved above all \c omp.target and sorted to
- /// satisfy dependencies among them.
- /// - \c bounds arguments are removed from \c omp.map.info operations.
- /// - \c var_ptr and \c var_ptr_ptr arguments of \c omp.map.info are
- /// handled as follows:
- /// - \c var_ptr_ptr is expected to be defined by a \c fir.box_offset
- /// operation which is preserved. Otherwise, the pass will fail.
- /// - \c var_ptr can be defined by a \c fir.declare which is also
- /// preserved. Its \c memref argument is replaced by a placeholder or
- /// maintained, similarly to non-map clauses of target operations
- /// described above. If it has \c shape or \c typeparams arguments, they
- /// are replaced by applicable constants. \c dummy_scope arguments
- /// are discarded.
- /// - Every other operation not located inside of an \c omp.target is
- /// removed.
- LogicalResult rewriteHostFunction(func::FuncOp funcOp) {
- Region ®ion = funcOp.getRegion();
-
- // Collect target operations inside of the function.
- llvm::SmallVector<omp::TargetOp> targetOps;
- region.walk<WalkOrder::PreOrder>([&](Operation *op) {
- // Skip the inside of omp.target regions, since these contain device code.
- if (auto targetOp = dyn_cast<omp::TargetOp>(op)) {
- targetOps.push_back(targetOp);
- return WalkResult::skip();
- }
-
- // Replace omp.target_data entry block argument uses with the value used
- // to initialize the associated omp.map.info operation. This way,
- // references are still valid once the omp.target operation has been
- // extracted out of the omp.target_data region.
- if (auto targetDataOp = dyn_cast<omp::TargetDataOp>(op)) {
- llvm::SmallVector<std::pair<Value, BlockArgument>> argPairs;
- cast<omp::BlockArgOpenMPOpInterface>(*targetDataOp)
- .getBlockArgsPairs(argPairs);
- for (auto [operand, blockArg] : argPairs) {
- auto mapInfo = cast<omp::MapInfoOp>(operand.getDefiningOp());
- Value varPtr = mapInfo.getVarPtr();
-
- // If the var_ptr operand of the omp.map.info op defining this entry
- // block argument is a fir.declare, the uses of all users of that
- // entry block argument that are themselves fir.declare are replaced
- // by the value produced by the outer one.
- //
- // This prevents this pass from producing chains of fir.declare of the
- // type:
- // %0 = ...
- // %1 = fir.declare %0
- // %2 = fir.declare %1...
- // %3 = omp.map.info var_ptr(%2 ...
- if (auto outerDeclare = varPtr.getDefiningOp<fir::DeclareOp>())
- for (Operation *user : blockArg.getUsers())
- if (isa<fir::DeclareOp>(user))
- user->replaceAllUsesWith(outerDeclare);
-
- // All remaining uses of the entry block argument are replaced with
- // the var_ptr initialization value.
- blockArg.replaceAllUsesWith(varPtr);
- }
- }
- return WalkResult::advance();
- });
-
- // Make a temporary clone of the parent operation with an empty region,
- // and update all references to entry block arguments to those of the new
- // region. Users will later either be moved to the new region or deleted
- // when the original region is replaced by the new.
- OpBuilder builder(&getContext());
- builder.setInsertionPointAfter(funcOp);
- Operation *newOp = builder.cloneWithoutRegions(funcOp);
- Block &block = newOp->getRegion(0).emplaceBlock();
-
- llvm::SmallVector<Location> locs;
- locs.reserve(region.getNumArguments());
- llvm::transform(region.getArguments(), std::back_inserter(locs),
- [](const BlockArgument &arg) { return arg.getLoc(); });
- block.addArguments(region.getArgumentTypes(), locs);
-
- for (auto [oldArg, newArg] :
- llvm::zip_equal(region.getArguments(), block.getArguments()))
- oldArg.replaceAllUsesWith(newArg);
-
- // Collect omp.map.info ops while satisfying interdependencies and remove
- // operands that aren't used by target device codegen.
- //
- // This logic must be updated whenever operands to omp.target change.
- llvm::SetVector<Value> rewriteValues;
- llvm::SetVector<omp::MapInfoOp> mapInfos;
- for (omp::TargetOp targetOp : targetOps) {
- assert(targetOp.getHostEvalVars().empty() &&
- "unexpected host_eval in target device module");
-
- // Variables unused by the device.
- targetOp.getDependVarsMutable().clear();
- targetOp.setDependKindsAttr(nullptr);
- targetOp.getDependIteratedMutable().clear();
- targetOp.setDependIteratedKindsAttr(nullptr);
- targetOp.getDeviceMutable().clear();
- targetOp.getDynGroupprivateSizeMutable().clear();
- targetOp.getIfExprMutable().clear();
-
- // TODO: Clear some of these operands rather than rewriting them,
- // depending on whether they are needed by device codegen once support for
- // them is fully implemented.
- for (Value allocVar : targetOp.getAllocateVars())
- collectRewrite(allocVar, rewriteValues);
- for (Value allocVar : targetOp.getAllocatorVars())
- collectRewrite(allocVar, rewriteValues);
- for (Value inReduction : targetOp.getInReductionVars())
- collectRewrite(inReduction, rewriteValues);
- for (Value isDevPtr : targetOp.getIsDevicePtrVars())
- collectRewrite(isDevPtr, rewriteValues);
- for (Value mapVar : targetOp.getHasDeviceAddrVars())
- collectRewrite(cast<omp::MapInfoOp>(mapVar.getDefiningOp()), mapInfos);
- for (Value mapVar : targetOp.getMapVars())
- collectRewrite(cast<omp::MapInfoOp>(mapVar.getDefiningOp()), mapInfos);
- for (Value privateVar : targetOp.getPrivateVars())
- collectRewrite(privateVar, rewriteValues);
- for (Value threadLimit : targetOp.getThreadLimitVars())
- collectRewrite(threadLimit, rewriteValues);
- }
-
- // Move omp.map.info ops to the new block and collect dependencies.
- llvm::SetVector<fir::DeclareOp> declareOps;
- llvm::SetVector<fir::BoxOffsetOp> boxOffsets;
- for (omp::MapInfoOp mapOp : mapInfos) {
- if (auto declareOp = dyn_cast_if_present<fir::DeclareOp>(
- mapOp.getVarPtr().getDefiningOp()))
- collectRewrite(declareOp, declareOps);
- else
- collectRewrite(mapOp.getVarPtr(), rewriteValues);
-
- if (Value varPtrPtr = mapOp.getVarPtrPtr()) {
- if (auto boxOffset = llvm::dyn_cast_if_present<fir::BoxOffsetOp>(
- varPtrPtr.getDefiningOp()))
- collectRewrite(boxOffset, boxOffsets);
- else
- return mapOp->emitOpError() << "var_ptr_ptr rewrite only supported "
- "if defined by fir.box_offset";
- }
-
- // Bounds are not used during target device codegen.
- mapOp.getBoundsMutable().clear();
- mapOp->moveBefore(&block, block.end());
- }
-
- // Create a temporary marker to simplify the op moving process below.
- builder.setInsertionPointToStart(&block);
- auto marker = fir::UndefOp::create(builder, builder.getUnknownLoc(),
- builder.getNoneType());
- builder.setInsertionPoint(marker);
-
- // Handle dependencies of fir.declare ops.
- for (fir::DeclareOp declareOp : declareOps) {
- collectRewrite(declareOp.getMemref(), rewriteValues);
-
- if (declareOp.getStorage())
- collectRewrite(declareOp.getStorage(), rewriteValues);
-
- // Shape and typeparams aren't needed for target device codegen, but
- // removing them would break verifiers.
- Value zero;
- if (declareOp.getShape() || !declareOp.getTypeparams().empty())
- zero = arith::ConstantOp::create(builder, declareOp.getLoc(),
- builder.getI64IntegerAttr(0));
-
- if (auto shape = declareOp.getShape()) {
- // The pre-cg rewrite pass requires the shape to be defined by one of
- // fir.shape, fir.shapeshift or fir.shift, so we need to make sure it's
- // still defined by one of these after this pass.
- Operation *shapeOp = shape.getDefiningOp();
- llvm::SmallVector<Value> extents(shapeOp->getNumOperands(), zero);
- Value newShape =
- llvm::TypeSwitch<Operation *, Value>(shapeOp)
- .Case([&](fir::ShapeOp op) {
- return fir::ShapeOp::create(builder, op.getLoc(), extents);
- })
- .Case([&](fir::ShapeShiftOp op) {
- auto type = fir::ShapeShiftType::get(op.getContext(),
- extents.size() / 2);
- return fir::ShapeShiftOp::create(builder, op.getLoc(), type,
- extents);
- })
- .Case([&](fir::ShiftOp op) {
- auto type =
- fir::ShiftType::get(op.getContext(), extents.size());
- return fir::ShiftOp::create(builder, op.getLoc(), type,
- extents);
- })
- .Default([](Operation *op) {
- op->emitOpError()
- << "fir.declare shape expected to be one of: "
- "fir.shape, fir.shapeshift or fir.shift";
- return nullptr;
- });
-
- if (!newShape)
- return failure();
-
- declareOp.getShapeMutable().assign(newShape);
- }
-
- for (OpOperand &typeParam : declareOp.getTypeparamsMutable())
- typeParam.assign(zero);
-
- declareOp.getDummyScopeMutable().clear();
- }
-
- // We don't actually need the proper initialization, but rather just
- // maintain the basic form of these operands. Generally, we create 1-bit
- // placeholder allocas that we "typecast" to the expected type and replace
- // all uses. Using fir.undefined here instead is not possible because these
- // ...
[truncated]
|
|
@llvm/pr-subscribers-flang-openmp Author: Sergio Afonso (skatrak) ChangesThe MLIR pass that removes operations exclusively intended for the host from OpenMP target offload modules is currently defined as part of Flang. However, this is a feature that would benefit from being reusable by other frontends, as removing such operations is a requirement for all OpenMP target device modules prior to LLVM IR translation. By moving the As a result of a later execution of this pass, dynamic dispatch of host functions via dispatch table using Patch is 109.58 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/208189.diff 13 Files Affected:
diff --git a/flang/include/flang/Optimizer/OpenMP/Passes.td b/flang/include/flang/Optimizer/OpenMP/Passes.td
index b5d9372e356f1..9ec159e1ba1e0 100644
--- a/flang/include/flang/Optimizer/OpenMP/Passes.td
+++ b/flang/include/flang/Optimizer/OpenMP/Passes.td
@@ -57,28 +57,6 @@ def FunctionFilteringPass : Pass<"omp-function-filtering"> {
];
}
-def HostOpFilteringPass : Pass<"omp-host-op-filtering"> {
- let summary =
- "Removes host-only operations from host functions retained by the "
- "function filtering pass when compiling for a target device.";
- let description = [{
- Removes operations from host functions that are not placed inside of an
- `omp.target` region when compiling for the target device.
-
- It must run after any passes that might introduce new host operations,
- especially if they belong to the 'omp' dialect, like `LowerWorkdistribute`.
- This ensures all potentially problematic host operations are accounted for
- prior to LLVM IR translation.
-
- This pass works at the FIR level of abstraction.
- }];
- let dependentDialects = [
- "mlir::arith::ArithDialect",
- "mlir::func::FuncDialect",
- "fir::FIROpsDialect"
- ];
-}
-
def DoConcurrentConversionPass : Pass<"omp-do-concurrent-conversion", "mlir::ModuleOp"> {
let summary = "Map `DO CONCURRENT` loops to OpenMP worksharing loops.";
diff --git a/flang/lib/Optimizer/OpenMP/CMakeLists.txt b/flang/lib/Optimizer/OpenMP/CMakeLists.txt
index f25e95f041d20..db29e93b71dad 100644
--- a/flang/lib/Optimizer/OpenMP/CMakeLists.txt
+++ b/flang/lib/Optimizer/OpenMP/CMakeLists.txt
@@ -5,7 +5,6 @@ add_flang_library(FlangOpenMPTransforms
DoConcurrentConversion.cpp
FunctionFiltering.cpp
GenericLoopConversion.cpp
- HostOpFiltering.cpp
MapsForPrivatizedSymbols.cpp
MapInfoFinalization.cpp
DeleteUnreachableTargets.cpp
diff --git a/flang/lib/Optimizer/OpenMP/FunctionFiltering.cpp b/flang/lib/Optimizer/OpenMP/FunctionFiltering.cpp
index e8bcc0c7fa062..ee61dd8881fd0 100644
--- a/flang/lib/Optimizer/OpenMP/FunctionFiltering.cpp
+++ b/flang/lib/Optimizer/OpenMP/FunctionFiltering.cpp
@@ -48,8 +48,7 @@ checkDeviceImplementationStatus(omp::OffloadModuleInterface offloadModule) {
if (!redOp.getByrefElementType())
return WalkResult::advance();
- auto seqTy =
- mlir::dyn_cast<fir::SequenceType>(*redOp.getByrefElementType());
+ auto seqTy = dyn_cast<fir::SequenceType>(*redOp.getByrefElementType());
bool isByRefReductionSupported =
!seqTy || !fir::sequenceWithNonConstantShape(seqTy);
@@ -104,7 +103,7 @@ class FunctionFilteringPass
SymbolTable::UseRange funcUses = *funcOp.getSymbolUses(op);
for (SymbolTable::SymbolUse use : funcUses) {
Operation *callOp = use.getUser();
- if (auto internalFunc = mlir::dyn_cast<func::FuncOp>(callOp)) {
+ if (auto internalFunc = dyn_cast<func::FuncOp>(callOp)) {
// Do not delete internal procedures holding the symbol of their
// Fortran host procedure as attribute.
internalFunc->removeAttr(fir::getHostSymbolAttrName());
@@ -114,6 +113,15 @@ class FunctionFilteringPass
internalFunc.setVisibility(mlir::SymbolTable::Visibility::Public);
continue;
}
+ // Prevent dispatch table entries pointing to deleted functions
+ // from being removed. This prevents the lowering of any
+ // corresponding fir.dispatch ops from triggering errors. These
+ // fir.dt_entry ops will point to an undefined symbol as a result,
+ // which currently doesn't cause an issue, as fir.dispatch-related ops
+ // are later removed by the host op filtering pass.
+ if (isa<fir::DTEntryOp>(callOp))
+ continue;
+
// If the callOp has users then replace them with Undef values.
if (!callOp->use_empty()) {
SmallVector<Value> undefResults;
diff --git a/flang/lib/Optimizer/OpenMP/HostOpFiltering.cpp b/flang/lib/Optimizer/OpenMP/HostOpFiltering.cpp
deleted file mode 100644
index 05ea0bc08c4ab..0000000000000
--- a/flang/lib/Optimizer/OpenMP/HostOpFiltering.cpp
+++ /dev/null
@@ -1,422 +0,0 @@
-//===- HostOpFiltering.cpp -----------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-//
-// This file implements transforms to filter out host-only operations from
-// any remaining host functions when compiling for a target device.
-//
-//===----------------------------------------------------------------------===//
-
-#include "flang/Optimizer/Dialect/FIRDialect.h"
-#include "flang/Optimizer/Dialect/FIROps.h"
-#include "flang/Optimizer/OpenMP/Passes.h"
-
-#include "mlir/Dialect/Arith/IR/Arith.h"
-#include "mlir/Dialect/Func/IR/FuncOps.h"
-#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
-#include "mlir/Dialect/OpenMP/OpenMPInterfaces.h"
-#include "llvm/ADT/SetVector.h"
-#include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/TypeSwitch.h"
-
-namespace flangomp {
-#define GEN_PASS_DEF_HOSTOPFILTERINGPASS
-#include "flang/Optimizer/OpenMP/Passes.h.inc"
-} // namespace flangomp
-
-using namespace mlir;
-
-/// Add an operation to one of the output sets to be later rewritten.
-template <typename OpTy>
-static void collectRewrite(OpTy op, llvm::SetVector<OpTy> &rewrites) {
- rewrites.insert(op);
-}
-
-/// Add an \c omp.map.info operation and all its members recursively to the
-/// output set to be later rewritten.
-///
-/// Dependencies across \c omp.map.info are maintained by ensuring dependencies
-/// are added to the output sets before operations based on them.
-template <>
-void collectRewrite(omp::MapInfoOp mapOp,
- llvm::SetVector<omp::MapInfoOp> &rewrites) {
- for (Value member : mapOp.getMembers())
- collectRewrite(cast<omp::MapInfoOp>(member.getDefiningOp()), rewrites);
-
- rewrites.insert(mapOp);
-}
-
-/// Add the given value to a sorted set if it should be replaced by a
-/// placeholder when used as an operand that must remain for the device.
-///
-/// Values that are block arguments of \c func.func operations are skipped,
-/// since they will still be available after all rewrites are completed.
-static void collectRewrite(Value value, llvm::SetVector<Value> &rewrites) {
- if ((isa<BlockArgument>(value) &&
- isa<func::FuncOp>(
- cast<BlockArgument>(value).getOwner()->getParentOp())) ||
- rewrites.contains(value))
- return;
-
- rewrites.insert(value);
-}
-
-/// Provide the `device_type` of an `omp.declare_target` attribute, if defined.
-static std::optional<omp::DeclareTargetDeviceType>
-getDeclareTargetDevice(Operation &op) {
- auto declareTargetOp = dyn_cast<omp::DeclareTargetInterface>(op);
- if (declareTargetOp && declareTargetOp.isDeclareTarget())
- return declareTargetOp.getDeclareTargetDeviceType();
- return std::nullopt;
-}
-
-namespace {
-class HostOpFilteringPass
- : public flangomp::impl::HostOpFilteringPassBase<HostOpFilteringPass> {
-public:
- HostOpFilteringPass() = default;
-
- void runOnOperation() override {
- auto op = dyn_cast<omp::OffloadModuleInterface>(getOperation());
- if (!op || !op.getIsTargetDevice())
- return;
-
- op->walk<WalkOrder::PreOrder>([&](func::FuncOp funcOp) {
- omp::DeclareTargetDeviceType declareType =
- getDeclareTargetDevice(*funcOp.getOperation())
- .value_or(omp::DeclareTargetDeviceType::host);
-
- // Only process host function definitions.
- if (funcOp.isExternal() ||
- declareType != omp::DeclareTargetDeviceType::host)
- return WalkResult::advance();
-
- if (failed(rewriteHostFunction(funcOp))) {
- funcOp.emitOpError() << "could not filter host-only operations";
- return WalkResult::interrupt();
- }
- return WalkResult::advance();
- });
-
- // Make non-declare target globals internal for the device. They cannot be
- // deleted, because they are needed in order to properly lower map clauses.
- // However, no uses will remain in the device module, so we make them
- // internal to prevent link time issues.
- op->walk([&](fir::GlobalOp globalOp) {
- if (!getDeclareTargetDevice(*globalOp.getOperation()).has_value())
- globalOp.setLinkName("internal");
- });
- }
-
-private:
- /// Rewrite the given host device function containing \c omp.target
- /// operations, to remove host-only operations that are not used by device
- /// codegen.
- ///
- /// It is based on the expected form of the MLIR module as produced by Flang
- /// lowering, after HLFIR to FIR lowering, and it performs the following
- /// mutations:
- /// - Replace all values returned by the function with \c fir.undefined.
- /// - \c omp.target operations are moved to the end of the function. If they
- /// are nested inside of any other operations, they are hoisted out of
- /// them.
- /// - \c depend, \c device and \c if clauses are removed from these target
- /// functions. Values used to initialize other clauses are replaced by
- /// placeholders as follows:
- /// - Values defined by block arguments are replaced by placeholders only
- /// if they are not attached to the parent \c func.func operation. In
- /// that case, they are passed unmodified.
- /// - \c arith.constant and \c fir.address_of ops are maintained.
- /// - Values of type \c fir.boxchar are replaced with a combination of
- /// \c fir.alloca for a single bit and a \c fir.emboxchar.
- /// - Other values are replaced by a combination of an \c fir.alloca for a
- /// single bit and an \c fir.convert to the original type of the value.
- /// This can be done because the code eventually generated for these
- /// operations will be discarded, as they aren't runnable by the target
- /// device.
- /// - \c omp.map.info operations associated to these target regions are
- /// preserved. These are moved above all \c omp.target and sorted to
- /// satisfy dependencies among them.
- /// - \c bounds arguments are removed from \c omp.map.info operations.
- /// - \c var_ptr and \c var_ptr_ptr arguments of \c omp.map.info are
- /// handled as follows:
- /// - \c var_ptr_ptr is expected to be defined by a \c fir.box_offset
- /// operation which is preserved. Otherwise, the pass will fail.
- /// - \c var_ptr can be defined by a \c fir.declare which is also
- /// preserved. Its \c memref argument is replaced by a placeholder or
- /// maintained, similarly to non-map clauses of target operations
- /// described above. If it has \c shape or \c typeparams arguments, they
- /// are replaced by applicable constants. \c dummy_scope arguments
- /// are discarded.
- /// - Every other operation not located inside of an \c omp.target is
- /// removed.
- LogicalResult rewriteHostFunction(func::FuncOp funcOp) {
- Region ®ion = funcOp.getRegion();
-
- // Collect target operations inside of the function.
- llvm::SmallVector<omp::TargetOp> targetOps;
- region.walk<WalkOrder::PreOrder>([&](Operation *op) {
- // Skip the inside of omp.target regions, since these contain device code.
- if (auto targetOp = dyn_cast<omp::TargetOp>(op)) {
- targetOps.push_back(targetOp);
- return WalkResult::skip();
- }
-
- // Replace omp.target_data entry block argument uses with the value used
- // to initialize the associated omp.map.info operation. This way,
- // references are still valid once the omp.target operation has been
- // extracted out of the omp.target_data region.
- if (auto targetDataOp = dyn_cast<omp::TargetDataOp>(op)) {
- llvm::SmallVector<std::pair<Value, BlockArgument>> argPairs;
- cast<omp::BlockArgOpenMPOpInterface>(*targetDataOp)
- .getBlockArgsPairs(argPairs);
- for (auto [operand, blockArg] : argPairs) {
- auto mapInfo = cast<omp::MapInfoOp>(operand.getDefiningOp());
- Value varPtr = mapInfo.getVarPtr();
-
- // If the var_ptr operand of the omp.map.info op defining this entry
- // block argument is a fir.declare, the uses of all users of that
- // entry block argument that are themselves fir.declare are replaced
- // by the value produced by the outer one.
- //
- // This prevents this pass from producing chains of fir.declare of the
- // type:
- // %0 = ...
- // %1 = fir.declare %0
- // %2 = fir.declare %1...
- // %3 = omp.map.info var_ptr(%2 ...
- if (auto outerDeclare = varPtr.getDefiningOp<fir::DeclareOp>())
- for (Operation *user : blockArg.getUsers())
- if (isa<fir::DeclareOp>(user))
- user->replaceAllUsesWith(outerDeclare);
-
- // All remaining uses of the entry block argument are replaced with
- // the var_ptr initialization value.
- blockArg.replaceAllUsesWith(varPtr);
- }
- }
- return WalkResult::advance();
- });
-
- // Make a temporary clone of the parent operation with an empty region,
- // and update all references to entry block arguments to those of the new
- // region. Users will later either be moved to the new region or deleted
- // when the original region is replaced by the new.
- OpBuilder builder(&getContext());
- builder.setInsertionPointAfter(funcOp);
- Operation *newOp = builder.cloneWithoutRegions(funcOp);
- Block &block = newOp->getRegion(0).emplaceBlock();
-
- llvm::SmallVector<Location> locs;
- locs.reserve(region.getNumArguments());
- llvm::transform(region.getArguments(), std::back_inserter(locs),
- [](const BlockArgument &arg) { return arg.getLoc(); });
- block.addArguments(region.getArgumentTypes(), locs);
-
- for (auto [oldArg, newArg] :
- llvm::zip_equal(region.getArguments(), block.getArguments()))
- oldArg.replaceAllUsesWith(newArg);
-
- // Collect omp.map.info ops while satisfying interdependencies and remove
- // operands that aren't used by target device codegen.
- //
- // This logic must be updated whenever operands to omp.target change.
- llvm::SetVector<Value> rewriteValues;
- llvm::SetVector<omp::MapInfoOp> mapInfos;
- for (omp::TargetOp targetOp : targetOps) {
- assert(targetOp.getHostEvalVars().empty() &&
- "unexpected host_eval in target device module");
-
- // Variables unused by the device.
- targetOp.getDependVarsMutable().clear();
- targetOp.setDependKindsAttr(nullptr);
- targetOp.getDependIteratedMutable().clear();
- targetOp.setDependIteratedKindsAttr(nullptr);
- targetOp.getDeviceMutable().clear();
- targetOp.getDynGroupprivateSizeMutable().clear();
- targetOp.getIfExprMutable().clear();
-
- // TODO: Clear some of these operands rather than rewriting them,
- // depending on whether they are needed by device codegen once support for
- // them is fully implemented.
- for (Value allocVar : targetOp.getAllocateVars())
- collectRewrite(allocVar, rewriteValues);
- for (Value allocVar : targetOp.getAllocatorVars())
- collectRewrite(allocVar, rewriteValues);
- for (Value inReduction : targetOp.getInReductionVars())
- collectRewrite(inReduction, rewriteValues);
- for (Value isDevPtr : targetOp.getIsDevicePtrVars())
- collectRewrite(isDevPtr, rewriteValues);
- for (Value mapVar : targetOp.getHasDeviceAddrVars())
- collectRewrite(cast<omp::MapInfoOp>(mapVar.getDefiningOp()), mapInfos);
- for (Value mapVar : targetOp.getMapVars())
- collectRewrite(cast<omp::MapInfoOp>(mapVar.getDefiningOp()), mapInfos);
- for (Value privateVar : targetOp.getPrivateVars())
- collectRewrite(privateVar, rewriteValues);
- for (Value threadLimit : targetOp.getThreadLimitVars())
- collectRewrite(threadLimit, rewriteValues);
- }
-
- // Move omp.map.info ops to the new block and collect dependencies.
- llvm::SetVector<fir::DeclareOp> declareOps;
- llvm::SetVector<fir::BoxOffsetOp> boxOffsets;
- for (omp::MapInfoOp mapOp : mapInfos) {
- if (auto declareOp = dyn_cast_if_present<fir::DeclareOp>(
- mapOp.getVarPtr().getDefiningOp()))
- collectRewrite(declareOp, declareOps);
- else
- collectRewrite(mapOp.getVarPtr(), rewriteValues);
-
- if (Value varPtrPtr = mapOp.getVarPtrPtr()) {
- if (auto boxOffset = llvm::dyn_cast_if_present<fir::BoxOffsetOp>(
- varPtrPtr.getDefiningOp()))
- collectRewrite(boxOffset, boxOffsets);
- else
- return mapOp->emitOpError() << "var_ptr_ptr rewrite only supported "
- "if defined by fir.box_offset";
- }
-
- // Bounds are not used during target device codegen.
- mapOp.getBoundsMutable().clear();
- mapOp->moveBefore(&block, block.end());
- }
-
- // Create a temporary marker to simplify the op moving process below.
- builder.setInsertionPointToStart(&block);
- auto marker = fir::UndefOp::create(builder, builder.getUnknownLoc(),
- builder.getNoneType());
- builder.setInsertionPoint(marker);
-
- // Handle dependencies of fir.declare ops.
- for (fir::DeclareOp declareOp : declareOps) {
- collectRewrite(declareOp.getMemref(), rewriteValues);
-
- if (declareOp.getStorage())
- collectRewrite(declareOp.getStorage(), rewriteValues);
-
- // Shape and typeparams aren't needed for target device codegen, but
- // removing them would break verifiers.
- Value zero;
- if (declareOp.getShape() || !declareOp.getTypeparams().empty())
- zero = arith::ConstantOp::create(builder, declareOp.getLoc(),
- builder.getI64IntegerAttr(0));
-
- if (auto shape = declareOp.getShape()) {
- // The pre-cg rewrite pass requires the shape to be defined by one of
- // fir.shape, fir.shapeshift or fir.shift, so we need to make sure it's
- // still defined by one of these after this pass.
- Operation *shapeOp = shape.getDefiningOp();
- llvm::SmallVector<Value> extents(shapeOp->getNumOperands(), zero);
- Value newShape =
- llvm::TypeSwitch<Operation *, Value>(shapeOp)
- .Case([&](fir::ShapeOp op) {
- return fir::ShapeOp::create(builder, op.getLoc(), extents);
- })
- .Case([&](fir::ShapeShiftOp op) {
- auto type = fir::ShapeShiftType::get(op.getContext(),
- extents.size() / 2);
- return fir::ShapeShiftOp::create(builder, op.getLoc(), type,
- extents);
- })
- .Case([&](fir::ShiftOp op) {
- auto type =
- fir::ShiftType::get(op.getContext(), extents.size());
- return fir::ShiftOp::create(builder, op.getLoc(), type,
- extents);
- })
- .Default([](Operation *op) {
- op->emitOpError()
- << "fir.declare shape expected to be one of: "
- "fir.shape, fir.shapeshift or fir.shift";
- return nullptr;
- });
-
- if (!newShape)
- return failure();
-
- declareOp.getShapeMutable().assign(newShape);
- }
-
- for (OpOperand &typeParam : declareOp.getTypeparamsMutable())
- typeParam.assign(zero);
-
- declareOp.getDummyScopeMutable().clear();
- }
-
- // We don't actually need the proper initialization, but rather just
- // maintain the basic form of these operands. Generally, we create 1-bit
- // placeholder allocas that we "typecast" to the expected type and replace
- // all uses. Using fir.undefined here instead is not possible because these
- // ...
[truncated]
|
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The MLIR pass that removes operations exclusively intended for the host from OpenMP target offload modules is currently defined as part of Flang. However, this is a feature that would benefit from being reusable by other frontends, as removing such operations is a requirement for all OpenMP target device modules prior to LLVM IR translation.
By moving the
omp-host-op-filteringpass out of Flang, it had to be updated to work on a lower-level LLVM dialect-based representation, rather than FIR. This simplified some of the existing edge cases, such asfir.declareops andfir.boxchartype handling. In addition, new function arguments are introduced as placeholders and return values from host-only functions are removed, producing a cleaner result and simplifying the pass as compared to previously.As a result of a later execution of this pass, dynamic dispatch of host functions via dispatch table using
fir.dispatch,fir.type_infoandfir.dt_entryops would break due to the removal offir.dt_entryoperations pointing to deleted host functions, whilefir.dispatchops pointing to the same functions would remain. Theomp-function-filteringpass is updated to preventfir.dt_entryfrom being deleted and let them point to an undefined symbol. This lets FIR lowering to LLVM work well enough to get to the host op filtering stage, where any resulting operations for this host dynamic dispatch are cleaned up.