[Mlir-commits] [mlir] 09ae1bf - [flang] Added OperationMoveOpInterface for controlling LICM. (#175108)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Fri Jan 16 08:32:43 PST 2026
Author: Slava Zakharin
Date: 2026-01-16T08:32:38-08:00
New Revision: 09ae1bf8b7175962169448565cef0addb4f43a2b
URL: https://github.com/llvm/llvm-project/commit/09ae1bf8b7175962169448565cef0addb4f43a2b
DIFF: https://github.com/llvm/llvm-project/commit/09ae1bf8b7175962169448565cef0addb4f43a2b.diff
LOG: [flang] Added OperationMoveOpInterface for controlling LICM. (#175108)
In #173438 I added a FIR specific loop invariant code motion pass.
During the review, Tom pointed out certain limitations about OpenMP
dialect operations that should be taken into consideration during
transformations such as LICM:
https://github.com/llvm/llvm-project/pull/173438#discussion_r2657612148
I also found issues with hoisting operations out of `acc.loop`
operations in certain conditions (see the added test in `licm.fir`).
I am proposing a new operation interface that will allow to control
movement of operations during MLIR transformations. In particular, I
propose two methods (there might be more):
* op.canMoveOutOf(cand) - returns true, if it is allowed to move 'cand'
operation out of 'op'.
* op.canMoveFromDescendant(descendant, cand) - return true, if it is
allowed to move 'cand' out of 'descendant' and into 'op'.
I used the new interface to get rid of explicit OpenMP interfaces checks
in Flang's LICM, and I also used it for `acc.loop` operation (though, I
provided conservative initial implementation).
The new interface is part of FIR dialect, but I think it would better
fit into the core MLIR set of interfaces so that the checks that I make
in Flang's LICM are actually done in
`mlir::moveLoopInvariantCode`. Moreover, other code movement
transformations that may appear in MLIR may also need to use such an
interface.
I would like to get some feedback on whether it is reasonable to move
the interface to core MLIR.
Added:
flang/include/flang/Optimizer/Dialect/FIROperationMoveOpInterface.h
flang/include/flang/Optimizer/Dialect/FIROperationMoveOpInterface.td
flang/lib/Optimizer/Dialect/FIROperationMoveOpInterface.cpp
flang/lib/Optimizer/OpenMP/Support/FIROpenMPOpsInterfaces.cpp
Modified:
flang/include/flang/Optimizer/Dialect/CMakeLists.txt
flang/include/flang/Optimizer/OpenACC/Support/FIROpenACCOpsInterfaces.h
flang/include/flang/Optimizer/OpenMP/Support/RegisterOpenMPExtensions.h
flang/lib/Optimizer/Dialect/CMakeLists.txt
flang/lib/Optimizer/OpenACC/Support/FIROpenACCOpsInterfaces.cpp
flang/lib/Optimizer/OpenACC/Support/RegisterOpenACCExtensions.cpp
flang/lib/Optimizer/OpenMP/Support/CMakeLists.txt
flang/lib/Optimizer/OpenMP/Support/RegisterOpenMPExtensions.cpp
flang/lib/Optimizer/Transforms/LoopInvariantCodeMotion.cpp
flang/test/Transforms/licm.fir
mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h
Removed:
################################################################################
diff --git a/flang/include/flang/Optimizer/Dialect/CMakeLists.txt b/flang/include/flang/Optimizer/Dialect/CMakeLists.txt
index 7b1a12932276a..eacebfd1e204a 100644
--- a/flang/include/flang/Optimizer/Dialect/CMakeLists.txt
+++ b/flang/include/flang/Optimizer/Dialect/CMakeLists.txt
@@ -35,6 +35,11 @@ mlir_tablegen(SafeTempArrayCopyAttrInterface.h.inc -gen-attr-interface-decls)
mlir_tablegen(SafeTempArrayCopyAttrInterface.cpp.inc -gen-attr-interface-defs)
add_public_tablegen_target(FIRSafeTempArrayCopyAttrInterfaceIncGen)
+set(LLVM_TARGET_DEFINITIONS FIROperationMoveOpInterface.td)
+mlir_tablegen(FIROperationMoveOpInterface.h.inc -gen-op-interface-decls)
+mlir_tablegen(FIROperationMoveOpInterface.cpp.inc -gen-op-interface-defs)
+add_public_tablegen_target(FIROperationMoveOpInterfaceIncGen)
+
set(LLVM_TARGET_DEFINITIONS CanonicalizationPatterns.td)
mlir_tablegen(CanonicalizationPatterns.inc -gen-rewriters)
add_public_tablegen_target(CanonicalizationPatternsIncGen)
diff --git a/flang/include/flang/Optimizer/Dialect/FIROperationMoveOpInterface.h b/flang/include/flang/Optimizer/Dialect/FIROperationMoveOpInterface.h
new file mode 100644
index 0000000000000..30a8fa110a5e4
--- /dev/null
+++ b/flang/include/flang/Optimizer/Dialect/FIROperationMoveOpInterface.h
@@ -0,0 +1,42 @@
+//===- FIROperationMoveOpInterface.h ----------------------------*- C++ -*-===//
+//
+// 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 declares methods used by OperationMoveOpInterface.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef FORTRAN_OPTIMIZER_DIALECT_FIR_OPERATION_MOVE_OP_INTERFACE_H
+#define FORTRAN_OPTIMIZER_DIALECT_FIR_OPERATION_MOVE_OP_INTERFACE_H
+
+#include "mlir/IR/OpDefinition.h"
+#include "mlir/IR/Operation.h"
+#include "llvm/Support/LogicalResult.h"
+
+namespace fir::detail {
+/// Verify invariants of OperationMoveOpInterface.
+llvm::LogicalResult verifyOperationMoveOpInterface(mlir::Operation *op);
+} // namespace fir::detail
+
+#include "flang/Optimizer/Dialect/FIROperationMoveOpInterface.h.inc"
+
+namespace fir {
+/// Returns true if it is allowed to move the given 'candidate'
+/// operation from the 'descendant' operation into operation 'op'.
+/// If 'candidate' is nullptr, then the caller is querying whether
+/// any operation from any descendant can be moved into this operation.
+bool canMoveFromDescendant(mlir::Operation *op, mlir::Operation *descendant,
+ mlir::Operation *candidate);
+
+/// Returns true if it is allowed to move the given 'candidate'
+/// operation out of operation 'op'. If 'candidate' is nullptr,
+/// then the caller is querying whether any operation can be moved
+/// out of this operation.
+bool canMoveOutOf(mlir::Operation *op, mlir::Operation *candidate);
+} // namespace fir
+
+#endif // FORTRAN_OPTIMIZER_DIALECT_FIR_OPERATION_MOVE_OP_INTERFACE_H
diff --git a/flang/include/flang/Optimizer/Dialect/FIROperationMoveOpInterface.td b/flang/include/flang/Optimizer/Dialect/FIROperationMoveOpInterface.td
new file mode 100644
index 0000000000000..359808012b5e4
--- /dev/null
+++ b/flang/include/flang/Optimizer/Dialect/FIROperationMoveOpInterface.td
@@ -0,0 +1,44 @@
+//===-- FIROperationMoveOpInterface.td ---------------------*- tablegen -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+include "mlir/IR/Interfaces.td"
+
+def OperationMoveOpInterface : OpInterface<"OperationMoveOpInterface"> {
+ let description = [{
+ Provides methods to control movement of operations during transformations.
+ For example, some LoopLikeOpInterface operations may disallow
+ hoisting speculatable operations (e.g. even Pure ones) under some
+ conditions. Operations may control when the hoisting
+ (or rather a general movement) is allowed by implementing this interface.
+ }];
+ let cppNamespace = "::fir";
+ let verify = [{ return detail::verifyOperationMoveOpInterface($_op); }];
+ let methods = [InterfaceMethod<
+ /*desc=*/[{
+ Returns true if it is allowed to move the given 'candidate'
+ operation from the 'descendant' operation into this operation.
+ If 'candidate' is nullptr, then the caller is querying whether
+ any operation from any descendant can be moved into this operation.
+ }],
+ /*returnType=*/"bool",
+ /*methodName=*/"canMoveFromDescendant",
+ /*args=*/
+ (ins "::mlir::Operation *":$descendant,
+ "::mlir::Operation *":$candidate)>,
+ InterfaceMethod<
+ /*desc=*/[{
+ Returns true if it is allowed to move the given 'candidate'
+ operation out of this operation. If 'candidate' is nullptr,
+ then the caller is querying whether any operation can be moved
+ out of this operation.
+ }],
+ /*returnType=*/"bool",
+ /*methodName=*/"canMoveOutOf",
+ /*args=*/(ins "::mlir::Operation *":$candidate)>,
+ ];
+}
diff --git a/flang/include/flang/Optimizer/OpenACC/Support/FIROpenACCOpsInterfaces.h b/flang/include/flang/Optimizer/OpenACC/Support/FIROpenACCOpsInterfaces.h
index b0252fee7b5a6..4847f3920eec1 100644
--- a/flang/include/flang/Optimizer/OpenACC/Support/FIROpenACCOpsInterfaces.h
+++ b/flang/include/flang/Optimizer/OpenACC/Support/FIROpenACCOpsInterfaces.h
@@ -13,6 +13,7 @@
#ifndef FLANG_OPTIMIZER_OPENACC_FIROPENACC_OPS_INTERFACES_H_
#define FLANG_OPTIMIZER_OPENACC_FIROPENACC_OPS_INTERFACES_H_
+#include "flang/Optimizer/Dialect/FIROperationMoveOpInterface.h"
#include "mlir/Dialect/OpenACC/OpenACC.h"
namespace fir {
@@ -95,6 +96,27 @@ struct OffloadRegionModel
: public mlir::acc::OffloadRegionOpInterface::ExternalModel<
OffloadRegionModel<Op>, Op> {};
+/// External model for fir::OperationMoveOpInterface.
+/// This interface provides methods to identify whether
+/// operations can be moved (e.g. by LICM, CSE, etc.) from/into
+/// OpenACC dialect operations.
+template <typename Op>
+struct OperationMoveModel : public fir::OperationMoveOpInterface::ExternalModel<
+ OperationMoveModel<Op>, Op> {
+ // Returns true if it is allowed to move the given 'candidate'
+ // operation from the 'descendant' operation into 'op' operation.
+ // If 'candidate' is nullptr, then the caller is querying whether
+ // any operation from any descendant can be moved into 'op' operation.
+ bool canMoveFromDescendant(mlir::Operation *op, mlir::Operation *descendant,
+ mlir::Operation *candidate) const;
+
+ // Returns true if it is allowed to move the given 'candidate'
+ // operation out of 'op' operation. If 'candidate' is nullptr,
+ // then the caller is querying whether any operation can be moved
+ // out of 'op' operation.
+ bool canMoveOutOf(mlir::Operation *op, mlir::Operation *candidate) const;
+};
+
} // namespace fir::acc
#endif // FLANG_OPTIMIZER_OPENACC_FIROPENACC_OPS_INTERFACES_H_
diff --git a/flang/include/flang/Optimizer/OpenMP/Support/RegisterOpenMPExtensions.h b/flang/include/flang/Optimizer/OpenMP/Support/RegisterOpenMPExtensions.h
index b0fa1d16cb2bd..12bdd614c2b82 100644
--- a/flang/include/flang/Optimizer/OpenMP/Support/RegisterOpenMPExtensions.h
+++ b/flang/include/flang/Optimizer/OpenMP/Support/RegisterOpenMPExtensions.h
@@ -25,6 +25,9 @@ void registerAttrsExtensions(mlir::DialectRegistry ®istry);
void registerTransformationalAttrsDependentDialects(
mlir::DialectRegistry ®istry);
+/// Register external models for FIR operation interfaces related to OpenMP.
+void registerOpInterfacesExtensions(mlir::DialectRegistry ®istry);
+
} // namespace fir::omp
#endif // FLANG_OPTIMIZER_OPENMP_SUPPORT_REGISTEROPENMPEXTENSIONS_H_
diff --git a/flang/lib/Optimizer/Dialect/CMakeLists.txt b/flang/lib/Optimizer/Dialect/CMakeLists.txt
index 65d1f2c027548..f81989a541990 100644
--- a/flang/lib/Optimizer/Dialect/CMakeLists.txt
+++ b/flang/lib/Optimizer/Dialect/CMakeLists.txt
@@ -6,6 +6,7 @@ add_subdirectory(MIF)
add_flang_library(FIRDialect
FIRAttr.cpp
FIRDialect.cpp
+ FIROperationMoveOpInterface.cpp
FIROps.cpp
FIRType.cpp
FirAliasTagOpInterface.cpp
@@ -15,6 +16,7 @@ add_flang_library(FIRDialect
DEPENDS
CanonicalizationPatternsIncGen
+ FIROperationMoveOpInterfaceIncGen
FIROpsIncGen
FIRSafeTempArrayCopyAttrInterfaceIncGen
CUFAttrsIncGen
diff --git a/flang/lib/Optimizer/Dialect/FIROperationMoveOpInterface.cpp b/flang/lib/Optimizer/Dialect/FIROperationMoveOpInterface.cpp
new file mode 100644
index 0000000000000..dcf532356783a
--- /dev/null
+++ b/flang/lib/Optimizer/Dialect/FIROperationMoveOpInterface.cpp
@@ -0,0 +1,49 @@
+//===-- FIROperationMoveOpInterface.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
+//
+//===----------------------------------------------------------------------===//
+//
+// Coding style: https://mlir.llvm.org/getting_started/DeveloperGuide/
+//
+//===----------------------------------------------------------------------===//
+
+#include "flang/Optimizer/Dialect/FIROperationMoveOpInterface.h"
+
+#include "flang/Optimizer/Dialect/FIROperationMoveOpInterface.cpp.inc"
+
+llvm::LogicalResult
+fir::detail::verifyOperationMoveOpInterface(mlir::Operation *op) {
+ // It does not make sense to use this interface for operations
+ // without any regions.
+ if (op->getNumRegions() == 0)
+ return op->emitOpError("must contain at least one region");
+ return llvm::success();
+}
+
+bool fir::canMoveFromDescendant(mlir::Operation *op,
+ mlir::Operation *descendant,
+ mlir::Operation *candidate) {
+ // Perform some sanity checks.
+ assert(op->isProperAncestor(descendant) &&
+ "op must be an ancestor of descendant");
+ if (candidate)
+ assert(descendant->isProperAncestor(candidate) &&
+ "descendant must be an ancestor of candidate");
+ if (auto iface = mlir::dyn_cast<OperationMoveOpInterface>(op))
+ return iface.canMoveFromDescendant(descendant, candidate);
+
+ return true;
+}
+
+bool fir::canMoveOutOf(mlir::Operation *op, mlir::Operation *candidate) {
+ if (candidate)
+ assert(op->isProperAncestor(candidate) &&
+ "op must be an ancestor of candidate");
+ if (auto iface = mlir::dyn_cast<OperationMoveOpInterface>(op))
+ return iface.canMoveOutOf(candidate);
+
+ return true;
+}
diff --git a/flang/lib/Optimizer/OpenACC/Support/FIROpenACCOpsInterfaces.cpp b/flang/lib/Optimizer/OpenACC/Support/FIROpenACCOpsInterfaces.cpp
index 5671af91f6ea5..4e48f16dd1144 100644
--- a/flang/lib/Optimizer/OpenACC/Support/FIROpenACCOpsInterfaces.cpp
+++ b/flang/lib/Optimizer/OpenACC/Support/FIROpenACCOpsInterfaces.cpp
@@ -184,4 +184,30 @@ void IndirectGlobalAccessModel<fir::TypeDescOp>::getReferencedSymbols(
symbolTable);
}
+template <>
+bool OperationMoveModel<mlir::acc::LoopOp>::canMoveFromDescendant(
+ mlir::Operation *op, mlir::Operation *descendant,
+ mlir::Operation *candidate) const {
+ // It should be always allowed to move operations from descendants
+ // of acc.loop into the acc.loop.
+ return true;
+}
+
+template <>
+bool OperationMoveModel<mlir::acc::LoopOp>::canMoveOutOf(
+ mlir::Operation *op, mlir::Operation *candidate) const {
+ // TODO: disallow moving operations, which have operands that are referenced
+ // in the data operands (e.g. in [first]private() etc.) of the acc.loop.
+ // For example:
+ // %17 = acc.private var(%16 : !fir.box<!fir.array<?xf32>>)
+ // acc.loop private(%17 : !fir.box<!fir.array<?xf32>>) ... {
+ // %19 = fir.box_addr %17
+ // }
+ // We cannot hoist %19 without violating assumptions that OpenACC
+ // transformations rely on.
+ //
+ // Always return false in the initial implementation.
+ return false;
+}
+
} // namespace fir::acc
diff --git a/flang/lib/Optimizer/OpenACC/Support/RegisterOpenACCExtensions.cpp b/flang/lib/Optimizer/OpenACC/Support/RegisterOpenACCExtensions.cpp
index 4c514599df414..d0f323684b759 100644
--- a/flang/lib/Optimizer/OpenACC/Support/RegisterOpenACCExtensions.cpp
+++ b/flang/lib/Optimizer/OpenACC/Support/RegisterOpenACCExtensions.cpp
@@ -91,6 +91,13 @@ void registerOpenACCExtensions(mlir::DialectRegistry ®istry) {
cuf::KernelOp::attachInterface<OffloadRegionModel<cuf::KernelOp>>(*ctx);
});
+ // Attach FIR dialect interfaces to OpenACC operations.
+ registry.addExtension(+[](mlir::MLIRContext *ctx,
+ mlir::acc::OpenACCDialect *dialect) {
+ mlir::acc::LoopOp::attachInterface<OperationMoveModel<mlir::acc::LoopOp>>(
+ *ctx);
+ });
+
registerAttrsExtensions(registry);
}
diff --git a/flang/lib/Optimizer/OpenMP/Support/CMakeLists.txt b/flang/lib/Optimizer/OpenMP/Support/CMakeLists.txt
index dee35e4f2c5a4..004753ddee7ba 100644
--- a/flang/lib/Optimizer/OpenMP/Support/CMakeLists.txt
+++ b/flang/lib/Optimizer/OpenMP/Support/CMakeLists.txt
@@ -2,6 +2,7 @@ get_property(dialect_libs GLOBAL PROPERTY MLIR_DIALECT_LIBS)
add_flang_library(FIROpenMPSupport
FIROpenMPAttributes.cpp
+ FIROpenMPOpsInterfaces.cpp
RegisterOpenMPExtensions.cpp
DEPENDS
diff --git a/flang/lib/Optimizer/OpenMP/Support/FIROpenMPOpsInterfaces.cpp b/flang/lib/Optimizer/OpenMP/Support/FIROpenMPOpsInterfaces.cpp
new file mode 100644
index 0000000000000..a396ef00dc004
--- /dev/null
+++ b/flang/lib/Optimizer/OpenMP/Support/FIROpenMPOpsInterfaces.cpp
@@ -0,0 +1,102 @@
+//===-- FIROpenMPOpsInterfaces.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
+//
+//===----------------------------------------------------------------------===//
+/// \file
+/// This file implements FIR operation interfaces, which may be attached
+/// to OpenMP dialect operations.
+//===----------------------------------------------------------------------===//
+
+#include "flang/Optimizer/Dialect/FIROperationMoveOpInterface.h"
+#include "flang/Optimizer/OpenMP/Support/RegisterOpenMPExtensions.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
+
+namespace {
+/// Helper template that must be specialized for each operation.
+/// The methods are declared just for documentation.
+template <typename OP, typename Enable = void>
+struct OperationMoveModel {
+ // Returns true if it is allowed to move the given 'candidate'
+ // operation from the 'descendant' operation into operation 'op'.
+ // If 'candidate' is nullptr, then the caller is querying whether
+ // any operation from any descendant can be moved into 'op' operation.
+ bool canMoveFromDescendant(mlir::Operation *op, mlir::Operation *descendant,
+ mlir::Operation *candidate) const;
+
+ // Returns true if it is allowed to move the given 'candidate'
+ // operation out of operation 'op'. If 'candidate' is nullptr,
+ // then the caller is querying whether any operation can be moved
+ // out of 'op' operation.
+ bool canMoveOutOf(mlir::Operation *op, mlir::Operation *candidate) const;
+};
+
+// Helpers to check if T is one of Ts.
+template <typename T, typename... Ts>
+struct is_any_type : std::disjunction<std::is_same<T, Ts>...> {};
+
+template <typename T, typename... Ts>
+struct is_any_omp_op
+ : std::integral_constant<
+ bool, is_any_type<typename std::remove_cv<T>::type, Ts...>::value> {};
+
+template <typename T, typename... Ts>
+constexpr bool is_any_omp_op_v = is_any_omp_op<T, Ts...>::value;
+
+/// OperationMoveModel specialization for OMP_LOOP_WRAPPER_OPS.
+template <typename OP>
+struct OperationMoveModel<
+ OP,
+ typename std::enable_if<is_any_omp_op_v<OP, OMP_LOOP_WRAPPER_OPS>>::type>
+ : public fir::OperationMoveOpInterface::ExternalModel<
+ OperationMoveModel<OP>, OP> {
+ bool canMoveFromDescendant(mlir::Operation *op, mlir::Operation *descendant,
+ mlir::Operation *candidate) const {
+ // Operations cannot be moved from descendants of LoopWrapperInterface
+ // operation into the LoopWrapperInterface operation.
+ return false;
+ }
+ bool canMoveOutOf(mlir::Operation *op, mlir::Operation *candidate) const {
+ // The LoopWrapperInterface operations are only supposed to contain
+ // a loop operation, and it is probably okay to move operations
+ // from the descendant loop operation out of the LoopWrapperInterface
+ // operation. For now, return false to be conservative.
+ return false;
+ }
+};
+
+/// OperationMoveModel specialization for OMP_OUTLINEABLE_OPS.
+template <typename OP>
+struct OperationMoveModel<
+ OP, typename std::enable_if<is_any_omp_op_v<OP, OMP_OUTLINEABLE_OPS>>::type>
+ : public fir::OperationMoveOpInterface::ExternalModel<
+ OperationMoveModel<OP>, OP> {
+ bool canMoveFromDescendant(mlir::Operation *op, mlir::Operation *descendant,
+ mlir::Operation *candidate) const {
+ // Operations can be moved from descendants of OutlineableOpenMPOpInterface
+ // operation into the OutlineableOpenMPOpInterface operation.
+ return true;
+ }
+ bool canMoveOutOf(mlir::Operation *op, mlir::Operation *candidate) const {
+ // Operations cannot be moved out of OutlineableOpenMPOpInterface operation.
+ return false;
+ }
+};
+
+// Helper to call attachInterface<OperationMoveModel> for all Ts
+// (types of operations).
+template <typename... Ts>
+void attachInterfaces(mlir::MLIRContext *ctx) {
+ (Ts::template attachInterface<OperationMoveModel<Ts>>(*ctx), ...);
+}
+} // anonymous namespace
+
+void fir::omp::registerOpInterfacesExtensions(mlir::DialectRegistry ®istry) {
+ registry.addExtension(
+ +[](mlir::MLIRContext *ctx, mlir::omp::OpenMPDialect *dialect) {
+ attachInterfaces<OMP_LOOP_WRAPPER_OPS>(ctx);
+ attachInterfaces<OMP_OUTLINEABLE_OPS>(ctx);
+ });
+}
diff --git a/flang/lib/Optimizer/OpenMP/Support/RegisterOpenMPExtensions.cpp b/flang/lib/Optimizer/OpenMP/Support/RegisterOpenMPExtensions.cpp
index 2495d54bf0174..de4906ec6fd26 100644
--- a/flang/lib/Optimizer/OpenMP/Support/RegisterOpenMPExtensions.cpp
+++ b/flang/lib/Optimizer/OpenMP/Support/RegisterOpenMPExtensions.cpp
@@ -15,6 +15,7 @@
namespace fir::omp {
void registerOpenMPExtensions(mlir::DialectRegistry ®istry) {
registerAttrsExtensions(registry);
+ registerOpInterfacesExtensions(registry);
}
} // namespace fir::omp
diff --git a/flang/lib/Optimizer/Transforms/LoopInvariantCodeMotion.cpp b/flang/lib/Optimizer/Transforms/LoopInvariantCodeMotion.cpp
index cb19cc2fa485b..8ebb8982936e8 100644
--- a/flang/lib/Optimizer/Transforms/LoopInvariantCodeMotion.cpp
+++ b/flang/lib/Optimizer/Transforms/LoopInvariantCodeMotion.cpp
@@ -13,11 +13,11 @@
//===----------------------------------------------------------------------===//
#include "flang/Optimizer/Analysis/AliasAnalysis.h"
+#include "flang/Optimizer/Dialect/FIROperationMoveOpInterface.h"
#include "flang/Optimizer/Dialect/FIROpsSupport.h"
#include "flang/Optimizer/Dialect/FortranVariableInterface.h"
#include "flang/Optimizer/HLFIR/HLFIROps.h"
#include "flang/Optimizer/Transforms/Passes.h"
-#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
#include "mlir/Pass/Pass.h"
#include "mlir/Transforms/LoopInvariantCodeMotionUtils.h"
#include "llvm/ADT/TypeSwitch.h"
@@ -272,21 +272,29 @@ void LoopInvariantCodeMotion::runOnOperation() {
};
getOperation()->walk([&](LoopLikeOpInterface loopLike) {
- if (isa<omp::OutlineableOpenMPOpInterface>(loopLike.getOperation())) {
- LDBG() << "Skipping omp::OutlineableOpenMPOpInterface operation";
+ if (!fir::canMoveOutOf(loopLike, nullptr)) {
+ LDBG() << "Cannot hoist anything out of loop operation: ";
+ LDBG_OS([&](llvm::raw_ostream &os) {
+ loopLike->print(os, OpPrintingFlags().skipRegions());
+ });
return;
}
// We always hoist operations to the parent operation of the loopLike.
// Check that the parent operation allows the hoisting, e.g.
// omp::LoopWrapperInterface operations assume tight nesting
// of the inner maybe loop-like operations, so hoisting
- // to such a parent would be invalid.
+ // to such a parent would be invalid. We rely on
+ // fir::canMoveFromDescendant() to identify whether the hoisting
+ // is allowed.
Operation *parentOp = loopLike->getParentOp();
if (!parentOp) {
LDBG() << "Skipping top-level loop-like operation?";
return;
- } else if (isa<omp::LoopWrapperInterface>(parentOp)) {
- LDBG() << "Skipping omp::LoopWrapperInterface operation";
+ } else if (!fir::canMoveFromDescendant(parentOp, loopLike, nullptr)) {
+ LDBG() << "Cannot hoist anything into operation: ";
+ LDBG_OS([&](llvm::raw_ostream &os) {
+ parentOp->print(os, OpPrintingFlags().skipRegions());
+ });
return;
}
moveLoopInvariantCode(
@@ -297,6 +305,14 @@ void LoopInvariantCodeMotion::runOnOperation() {
},
/*shouldMoveOutOfRegion=*/
[&](Operation *op, Region *) {
+ if (!fir::canMoveOutOf(loopLike, op)) {
+ LDBG() << "Cannot hoist " << *op << " out of the loop";
+ return false;
+ }
+ if (!fir::canMoveFromDescendant(parentOp, loopLike, op)) {
+ LDBG() << "Cannot hoist " << *op << " into the parent of the loop";
+ return false;
+ }
return shouldMoveOutOfLoop(op, loopLike);
},
/*moveOutOfRegion=*/
diff --git a/flang/test/Transforms/licm.fir b/flang/test/Transforms/licm.fir
index fbcb928620b3b..b568f62b6fca6 100644
--- a/flang/test/Transforms/licm.fir
+++ b/flang/test/Transforms/licm.fir
@@ -1659,3 +1659,71 @@ func.func @test_if_hoisting(%arg0: !fir.ref<!fir.array<?xi32>> {fir.bindc_name =
fir.store %11 to %2 : !fir.ref<i32>
return
}
+
+// -----
+// Check that fir.box_addr applied to the private box is not hoisted
+// out of acc.loop. This breaks the assumptions taken by OpenACC
+// transformations that the results of acc.private operations,
+// that are present as the private() operands of acc.loop,
+// are only used inside the acc.loop.
+// subroutine test_acc_loop_with_private(a,n)
+// integer :: n,i
+// real :: b(n)
+// !$acc parallel private(b)
+// !$acc loop private(b)
+// do i = 1, n
+// b(i) = 1.0
+// enddo
+// !$acc end parallel
+// end subroutine
+// CHECK-LABEL: func.func @_QPtest_acc_loop_with_private(
+// CHECK-SAME: %[[ARG0:[0-9]+|[a-zA-Z$._-][a-zA-Z0-9$._-]*]]: !fir.ref<f32> {fir.bindc_name = "a"},
+// CHECK-SAME: %[[ARG1:[0-9]+|[a-zA-Z$._-][a-zA-Z0-9$._-]*]]: !fir.ref<i32> {fir.bindc_name = "n"}) {
+// CHECK: acc.parallel private(%{{.*}} : !fir.box<!fir.array<?xf32>>) {
+// CHECK: %[[PRIVATE_1:.*]] = acc.private var(%{{.*}} : !fir.box<!fir.array<?xf32>>) recipe(@{{.*}}) -> !fir.box<!fir.array<?xf32>> {name = "b"}
+// CHECK: %[[PRIVATE_2:.*]] = acc.private varPtr(%{{.*}} : !fir.ref<i32>) recipe(@{{.*}}) -> !fir.ref<i32> {implicit = true, name = "i"}
+// CHECK-NOT: fir.box_addr %[[PRIVATE_1]]
+// CHECK: acc.loop private(%[[PRIVATE_1]], %[[PRIVATE_2]] : !fir.box<!fir.array<?xf32>>, !fir.ref<i32>) {{.*}} {
+// CHECK: %[[BOX_ADDR_1:.*]] = fir.box_addr %[[PRIVATE_1]] : (!fir.box<!fir.array<?xf32>>) -> !fir.ref<!fir.array<?xf32>>
+// CHECK: %[[DECLARE_5:.*]] = fir.declare %[[BOX_ADDR_1]](%{{.*}}) {uniq_name = "_QFtest_acc_loop_with_privateEb"} : (!fir.ref<!fir.array<?xf32>>, !fir.shape<1>) -> !fir.ref<!fir.array<?xf32>>
+func.func @_QPtest_acc_loop_with_private(%arg0: !fir.ref<f32> {fir.bindc_name = "a"}, %arg1: !fir.ref<i32> {fir.bindc_name = "n"}) {
+ %cst = arith.constant 1.000000e+00 : f32
+ %c10_i32 = arith.constant 10 : i32
+ %c1_i32 = arith.constant 1 : i32
+ %c0 = arith.constant 0 : index
+ %0 = fir.dummy_scope : !fir.dscope
+ %1 = fir.declare %arg0 dummy_scope %0 arg 1 {uniq_name = "_QFtest_acc_loop_with_privateEa"} : (!fir.ref<f32>, !fir.dscope) -> !fir.ref<f32>
+ %2 = fir.declare %arg1 dummy_scope %0 arg 2 {uniq_name = "_QFtest_acc_loop_with_privateEn"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32>
+ %3 = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFtest_acc_loop_with_privateEi"}
+ %4 = fir.declare %3 {uniq_name = "_QFtest_acc_loop_with_privateEi"} : (!fir.ref<i32>) -> !fir.ref<i32>
+ %5 = fir.load %2 : !fir.ref<i32>
+ %6 = fir.convert %5 : (i32) -> index
+ %7 = arith.cmpi sgt, %6, %c0 : index
+ %8 = arith.select %7, %6, %c0 : index
+ %9 = fir.alloca !fir.array<?xf32>, %8 {bindc_name = "b", uniq_name = "_QFtest_acc_loop_with_privateEb"}
+ %10 = fir.shape %8 : (index) -> !fir.shape<1>
+ %11 = fir.declare %9(%10) {uniq_name = "_QFtest_acc_loop_with_privateEb"} : (!fir.ref<!fir.array<?xf32>>, !fir.shape<1>) -> !fir.ref<!fir.array<?xf32>>
+ %12 = fir.embox %11(%10) : (!fir.ref<!fir.array<?xf32>>, !fir.shape<1>) -> !fir.box<!fir.array<?xf32>>
+ %13 = acc.private var(%12 : !fir.box<!fir.array<?xf32>>) recipe(@privatization_box_Uxf32) -> !fir.box<!fir.array<?xf32>> {name = "b"}
+ acc.parallel private(%13 : !fir.box<!fir.array<?xf32>>) {
+ %14 = fir.box_addr %13 : (!fir.box<!fir.array<?xf32>>) -> !fir.ref<!fir.array<?xf32>>
+ %15 = fir.declare %14(%10) {uniq_name = "_QFtest_acc_loop_with_privateEb"} : (!fir.ref<!fir.array<?xf32>>, !fir.shape<1>) -> !fir.ref<!fir.array<?xf32>>
+ %16 = fir.embox %15(%10) : (!fir.ref<!fir.array<?xf32>>, !fir.shape<1>) -> !fir.box<!fir.array<?xf32>>
+ %17 = acc.private var(%16 : !fir.box<!fir.array<?xf32>>) recipe(@privatization_box_Uxf32) -> !fir.box<!fir.array<?xf32>> {name = "b"}
+ %18 = acc.private varPtr(%4 : !fir.ref<i32>) recipe(@privatization_ref_i32) -> !fir.ref<i32> {implicit = true, name = "i"}
+ acc.loop private(%17, %18 : !fir.box<!fir.array<?xf32>>, !fir.ref<i32>) control(%arg2 : i32) = (%c1_i32 : i32) to (%c10_i32 : i32) step (%c1_i32 : i32) {
+ %19 = fir.box_addr %17 : (!fir.box<!fir.array<?xf32>>) -> !fir.ref<!fir.array<?xf32>>
+ %20 = fir.declare %19(%10) {uniq_name = "_QFtest_acc_loop_with_privateEb"} : (!fir.ref<!fir.array<?xf32>>, !fir.shape<1>) -> !fir.ref<!fir.array<?xf32>>
+ %21 = fir.embox %20(%10) : (!fir.ref<!fir.array<?xf32>>, !fir.shape<1>) -> !fir.box<!fir.array<?xf32>>
+ %22 = fir.declare %18 {uniq_name = "_QFtest_acc_loop_with_privateEi"} : (!fir.ref<i32>) -> !fir.ref<i32>
+ fir.store %arg2 to %22 : !fir.ref<i32>
+ %23 = fir.load %22 : !fir.ref<i32>
+ %24 = fir.convert %23 : (i32) -> i64
+ %25 = fir.array_coor %20(%10) %24 : (!fir.ref<!fir.array<?xf32>>, !fir.shape<1>, i64) -> !fir.ref<f32>
+ fir.store %cst to %25 : !fir.ref<f32>
+ acc.yield
+ } attributes {inclusiveUpperbound = array<i1: true>, independent = [#acc.device_type<none>]}
+ acc.yield
+ }
+ return
+}
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h b/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h
index 7cf738352ba47..8b32dcdaf47a8 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h
@@ -37,6 +37,16 @@
#define GET_OP_CLASSES
#include "mlir/Dialect/OpenMP/OpenMPOps.h.inc"
+/// Operations implementing LoopWrapperInterface.
+#define OMP_LOOP_WRAPPER_OPS \
+ mlir::omp::WorkshareLoopWrapperOp, mlir::omp::LoopOp, mlir::omp::WsloopOp, \
+ mlir::omp::SimdOp, mlir::omp::DistributeOp, mlir::omp::TaskloopOp
+
+/// Operations implementing OutlineableOpenMPOpInterface.
+#define OMP_OUTLINEABLE_OPS \
+ mlir::omp::ParallelOp, mlir::omp::TeamsOp, mlir::omp::TaskOp, \
+ mlir::omp::TargetOp
+
namespace mlir::omp {
/// Find the omp.new_cli, generator, and consumer of a canonical loop info.
std::tuple<NewCliOp, OpOperand *, OpOperand *> decodeCli(mlir::Value cli);
More information about the Mlir-commits
mailing list