[Mlir-commits] [flang] [mlir] [flang] Added OperationMoveOpInterface for controlling LICM. (PR #175108)
Slava Zakharin
llvmlistbot at llvm.org
Thu Jan 15 20:04:38 PST 2026
https://github.com/vzakhari updated https://github.com/llvm/llvm-project/pull/175108
>From f2292456df9dd0da9f81f23becdcc98d753422d0 Mon Sep 17 00:00:00 2001
From: Slava Zakharin <szakharin at nvidia.com>
Date: Thu, 8 Jan 2026 17:02:16 -0800
Subject: [PATCH 1/2] [flang] Added OperationMoveOpInterface for controlling
LICM.
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.
---
.../flang/Optimizer/Dialect/CMakeLists.txt | 5 +
.../Dialect/FIROperationMoveOpInterface.h | 38 +++++++
.../Dialect/FIROperationMoveOpInterface.td | 75 +++++++++++++
.../OpenACC/Support/FIROpenACCOpsInterfaces.h | 23 ++++
.../OpenMP/Support/RegisterOpenMPExtensions.h | 3 +
flang/lib/Optimizer/Dialect/CMakeLists.txt | 2 +
.../Dialect/FIROperationMoveOpInterface.cpp | 46 ++++++++
.../Support/FIROpenACCOpsInterfaces.cpp | 26 +++++
.../Support/RegisterOpenACCExtensions.cpp | 7 ++
.../Optimizer/OpenMP/Support/CMakeLists.txt | 1 +
.../OpenMP/Support/FIROpenMPOpsInterfaces.cpp | 105 ++++++++++++++++++
.../Support/RegisterOpenMPExtensions.cpp | 1 +
.../Transforms/LoopInvariantCodeMotion.cpp | 28 ++++-
flang/test/Transforms/licm.fir | 68 ++++++++++++
.../mlir/Dialect/OpenMP/OpenMPDialect.h | 10 ++
15 files changed, 432 insertions(+), 6 deletions(-)
create mode 100644 flang/include/flang/Optimizer/Dialect/FIROperationMoveOpInterface.h
create mode 100644 flang/include/flang/Optimizer/Dialect/FIROperationMoveOpInterface.td
create mode 100644 flang/lib/Optimizer/Dialect/FIROperationMoveOpInterface.cpp
create mode 100644 flang/lib/Optimizer/OpenMP/Support/FIROpenMPOpsInterfaces.cpp
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..cbb25b6ebad94
--- /dev/null
+++ b/flang/include/flang/Optimizer/Dialect/FIROperationMoveOpInterface.h
@@ -0,0 +1,38 @@
+//===- 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);
+
+/// A wrapper around canMoveFromDescendantImpl().
+/// The wrapper asserts certain assumptions about the passed
+/// arguments.
+bool canMoveFromDescendant(mlir::Operation *op, mlir::Operation *descendant,
+ mlir::Operation *candidate);
+
+/// A wrapper around canMoveOutOfImpl().
+/// The wrapper asserts certain assumptions about the passed
+/// arguments.
+bool canMoveOutOf(mlir::Operation *op, mlir::Operation *candidate);
+} // namespace fir::detail
+
+#include "flang/Optimizer/Dialect/FIROperationMoveOpInterface.h.inc"
+
+#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..1964342fe00f8
--- /dev/null
+++ b/flang/include/flang/Optimizer/Dialect/FIROperationMoveOpInterface.td
@@ -0,0 +1,75 @@
+//===-- 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.
+
+ The implementations of OperationMoveOpInterface should not provide
+ implementation of this method, in general. The default implementation
+ applies some verification and relays to canMoveFromDescendantImpl()
+ method, which the implementations must provide.
+ }],
+ /*returnType=*/"bool",
+ /*methodName=*/"canMoveFromDescendant",
+ /*args=*/
+ (ins "::mlir::Operation *":$descendant,
+ "::mlir::Operation *":$candidate),
+ /*methodBody=*/[{}],
+ /*defaultImplementation=*/[{
+ return detail::canMoveFromDescendant($_op, descendant, candidate);
+ }]>,
+ InterfaceMethod<
+ /*desc=*/"",
+ /*returnType=*/"bool",
+ /*methodName=*/"canMoveFromDescendantImpl",
+ /*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.
+
+ The implementations of OperationMoveOpInterface should not provide
+ implementation of this method, in general. The default implementation
+ applies some verification and relays to canMoveOutOfImpl() method,
+ which the implementations must provide.
+ }],
+ /*returnType=*/"bool",
+ /*methodName=*/"canMoveOutOf",
+ /*args=*/(ins "::mlir::Operation *":$candidate),
+ /*methodBody=*/[{}],
+ /*defaultImplementation=*/[{
+ return detail::canMoveOutOf($_op, candidate);
+ }]>,
+ InterfaceMethod<
+ /*desc=*/"Implementation of canMoveOutOf() for concrete operation",
+ /*returnType=*/"bool",
+ /*methodName=*/"canMoveOutOfImpl",
+ /*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 c6f52bbd0c64b..3f36f2e712c99 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 {
@@ -94,6 +95,28 @@ 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) 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 canMoveFromDescendantImpl(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 canMoveOutOfImpl(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..692a7acc7e89b
--- /dev/null
+++ b/flang/lib/Optimizer/Dialect/FIROperationMoveOpInterface.cpp
@@ -0,0 +1,46 @@
+//===-- 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::detail::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");
+ auto iface = mlir::cast<OperationMoveOpInterface>(op);
+ return iface.canMoveFromDescendantImpl(descendant, candidate);
+}
+
+bool fir::detail::canMoveOutOf(mlir::Operation *op,
+ mlir::Operation *candidate) {
+ if (candidate)
+ assert(op->isProperAncestor(candidate) &&
+ "op must be an ancestor of candidate");
+ auto iface = mlir::cast<OperationMoveOpInterface>(op);
+ return iface.canMoveOutOfImpl(candidate);
+}
diff --git a/flang/lib/Optimizer/OpenACC/Support/FIROpenACCOpsInterfaces.cpp b/flang/lib/Optimizer/OpenACC/Support/FIROpenACCOpsInterfaces.cpp
index dacafb1eeb4b2..669bfdaf1d5a5 100644
--- a/flang/lib/Optimizer/OpenACC/Support/FIROpenACCOpsInterfaces.cpp
+++ b/flang/lib/Optimizer/OpenACC/Support/FIROpenACCOpsInterfaces.cpp
@@ -177,4 +177,30 @@ void IndirectGlobalAccessModel<fir::TypeDescOp>::getReferencedSymbols(
symbolTable);
}
+template <>
+bool OperationMoveModel<mlir::acc::LoopOp>::canMoveFromDescendantImpl(
+ 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>::canMoveOutOfImpl(
+ 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..d5f0d8cac9ca8
--- /dev/null
+++ b/flang/lib/Optimizer/OpenMP/Support/FIROpenMPOpsInterfaces.cpp
@@ -0,0 +1,105 @@
+//===-- 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 'op' operation.
+ // If 'candidate' is nullptr, then the caller is querying whether
+ // any operation from any descendant can be moved into 'op' operation.
+ bool canMoveFromDescendantImpl(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 canMoveOutOfImpl(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 canMoveFromDescendantImpl(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 canMoveOutOfImpl(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 canMoveFromDescendantImpl(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 canMoveOutOfImpl(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..980d10a1a15df 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,28 @@ void LoopInvariantCodeMotion::runOnOperation() {
};
getOperation()->walk([&](LoopLikeOpInterface loopLike) {
- if (isa<omp::OutlineableOpenMPOpInterface>(loopLike.getOperation())) {
- LDBG() << "Skipping omp::OutlineableOpenMPOpInterface operation";
+ auto loopMoveIface =
+ dyn_cast<fir::OperationMoveOpInterface>(loopLike.getOperation());
+ if (loopMoveIface && !loopMoveIface.canMoveOutOf(nullptr)) {
+ LDBG()
+ << "Cannot hoist anything out of OperationMoveOpInterface operation";
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 assume that all such
+ // operations properly implement fir::OperationMoveOpInterface.
Operation *parentOp = loopLike->getParentOp();
+ auto parentMoveIface =
+ dyn_cast_or_null<fir::OperationMoveOpInterface>(parentOp);
if (!parentOp) {
LDBG() << "Skipping top-level loop-like operation?";
return;
- } else if (isa<omp::LoopWrapperInterface>(parentOp)) {
- LDBG() << "Skipping omp::LoopWrapperInterface operation";
+ } else if (parentMoveIface &&
+ !parentMoveIface.canMoveFromDescendant(loopLike, nullptr)) {
+ LDBG() << "Cannot hoist anything into OperationMoveOpInterface operation";
return;
}
moveLoopInvariantCode(
@@ -297,6 +304,15 @@ void LoopInvariantCodeMotion::runOnOperation() {
},
/*shouldMoveOutOfRegion=*/
[&](Operation *op, Region *) {
+ if (loopMoveIface && !loopMoveIface.canMoveOutOf(op)) {
+ LDBG() << "Cannot hoist " << *op << " out of the loop";
+ return false;
+ }
+ if (parentMoveIface &&
+ !parentMoveIface.canMoveFromDescendant(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);
>From b7d12d33a161a205d2dc35f8b34e69380e07261b Mon Sep 17 00:00:00 2001
From: Slava Zakharin <szakharin at nvidia.com>
Date: Mon, 12 Jan 2026 10:01:00 -0800
Subject: [PATCH 2/2] Addressed review comments.
---
.../Dialect/FIROperationMoveOpInterface.h | 22 +++++---
.../Dialect/FIROperationMoveOpInterface.td | 55 ++++---------------
.../OpenACC/Support/FIROpenACCOpsInterfaces.h | 9 ++-
.../Dialect/FIROperationMoveOpInterface.cpp | 21 ++++---
.../Support/FIROpenACCOpsInterfaces.cpp | 4 +-
.../OpenMP/Support/FIROpenMPOpsInterfaces.cpp | 25 ++++-----
.../Transforms/LoopInvariantCodeMotion.cpp | 30 +++++-----
7 files changed, 69 insertions(+), 97 deletions(-)
diff --git a/flang/include/flang/Optimizer/Dialect/FIROperationMoveOpInterface.h b/flang/include/flang/Optimizer/Dialect/FIROperationMoveOpInterface.h
index cbb25b6ebad94..30a8fa110a5e4 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROperationMoveOpInterface.h
+++ b/flang/include/flang/Optimizer/Dialect/FIROperationMoveOpInterface.h
@@ -20,19 +20,23 @@
namespace fir::detail {
/// Verify invariants of OperationMoveOpInterface.
llvm::LogicalResult verifyOperationMoveOpInterface(mlir::Operation *op);
+} // namespace fir::detail
+
+#include "flang/Optimizer/Dialect/FIROperationMoveOpInterface.h.inc"
-/// A wrapper around canMoveFromDescendantImpl().
-/// The wrapper asserts certain assumptions about the passed
-/// arguments.
+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);
-/// A wrapper around canMoveOutOfImpl().
-/// The wrapper asserts certain assumptions about the passed
-/// arguments.
+/// 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::detail
-
-#include "flang/Optimizer/Dialect/FIROperationMoveOpInterface.h.inc"
+} // 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
index 1964342fe00f8..359808012b5e4 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROperationMoveOpInterface.td
+++ b/flang/include/flang/Optimizer/Dialect/FIROperationMoveOpInterface.td
@@ -18,58 +18,27 @@ def OperationMoveOpInterface : OpInterface<"OperationMoveOpInterface"> {
}];
let cppNamespace = "::fir";
let verify = [{ return detail::verifyOperationMoveOpInterface($_op); }];
- let methods =
- [InterfaceMethod<
- /*desc=*/[{
+ 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.
-
- The implementations of OperationMoveOpInterface should not provide
- implementation of this method, in general. The default implementation
- applies some verification and relays to canMoveFromDescendantImpl()
- method, which the implementations must provide.
}],
- /*returnType=*/"bool",
- /*methodName=*/"canMoveFromDescendant",
- /*args=*/
- (ins "::mlir::Operation *":$descendant,
- "::mlir::Operation *":$candidate),
- /*methodBody=*/[{}],
- /*defaultImplementation=*/[{
- return detail::canMoveFromDescendant($_op, descendant, candidate);
- }]>,
- InterfaceMethod<
- /*desc=*/"",
- /*returnType=*/"bool",
- /*methodName=*/"canMoveFromDescendantImpl",
- /*args=*/
- (ins "::mlir::Operation *":$descendant,
- "::mlir::Operation *":$candidate)>,
- InterfaceMethod<
- /*desc=*/[{
+ /*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.
-
- The implementations of OperationMoveOpInterface should not provide
- implementation of this method, in general. The default implementation
- applies some verification and relays to canMoveOutOfImpl() method,
- which the implementations must provide.
}],
- /*returnType=*/"bool",
- /*methodName=*/"canMoveOutOf",
- /*args=*/(ins "::mlir::Operation *":$candidate),
- /*methodBody=*/[{}],
- /*defaultImplementation=*/[{
- return detail::canMoveOutOf($_op, candidate);
- }]>,
- InterfaceMethod<
- /*desc=*/"Implementation of canMoveOutOf() for concrete operation",
- /*returnType=*/"bool",
- /*methodName=*/"canMoveOutOfImpl",
- /*args=*/(ins "::mlir::Operation *":$candidate)>,
+ /*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 3f36f2e712c99..a51c3bf582f8e 100644
--- a/flang/include/flang/Optimizer/OpenACC/Support/FIROpenACCOpsInterfaces.h
+++ b/flang/include/flang/Optimizer/OpenACC/Support/FIROpenACCOpsInterfaces.h
@@ -97,7 +97,7 @@ struct OffloadRegionModel
/// External model for fir::OperationMoveOpInterface.
/// This interface provides methods to identify whether
-/// operations can be moved (e.g. by LICM) from/into
+/// operations can be moved (e.g. by LICM, CSE, etc.) from/into
/// OpenACC dialect operations.
template <typename Op>
struct OperationMoveModel : public fir::OperationMoveOpInterface::ExternalModel<
@@ -106,15 +106,14 @@ struct OperationMoveModel : public fir::OperationMoveOpInterface::ExternalModel<
// 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 canMoveFromDescendantImpl(mlir::Operation *op,
- mlir::Operation *descendant,
- mlir::Operation *candidate) const;
+ 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 canMoveOutOfImpl(mlir::Operation *op, mlir::Operation *candidate) const;
+ bool canMoveOutOf(mlir::Operation *op, mlir::Operation *candidate) const;
};
} // namespace fir::acc
diff --git a/flang/lib/Optimizer/Dialect/FIROperationMoveOpInterface.cpp b/flang/lib/Optimizer/Dialect/FIROperationMoveOpInterface.cpp
index 692a7acc7e89b..dcf532356783a 100644
--- a/flang/lib/Optimizer/Dialect/FIROperationMoveOpInterface.cpp
+++ b/flang/lib/Optimizer/Dialect/FIROperationMoveOpInterface.cpp
@@ -23,24 +23,27 @@ fir::detail::verifyOperationMoveOpInterface(mlir::Operation *op) {
return llvm::success();
}
-bool fir::detail::canMoveFromDescendant(mlir::Operation *op,
- mlir::Operation *descendant,
- mlir::Operation *candidate) {
+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");
- auto iface = mlir::cast<OperationMoveOpInterface>(op);
- return iface.canMoveFromDescendantImpl(descendant, candidate);
+ if (auto iface = mlir::dyn_cast<OperationMoveOpInterface>(op))
+ return iface.canMoveFromDescendant(descendant, candidate);
+
+ return true;
}
-bool fir::detail::canMoveOutOf(mlir::Operation *op,
- mlir::Operation *candidate) {
+bool fir::canMoveOutOf(mlir::Operation *op, mlir::Operation *candidate) {
if (candidate)
assert(op->isProperAncestor(candidate) &&
"op must be an ancestor of candidate");
- auto iface = mlir::cast<OperationMoveOpInterface>(op);
- return iface.canMoveOutOfImpl(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 669bfdaf1d5a5..ace5f29338c8e 100644
--- a/flang/lib/Optimizer/OpenACC/Support/FIROpenACCOpsInterfaces.cpp
+++ b/flang/lib/Optimizer/OpenACC/Support/FIROpenACCOpsInterfaces.cpp
@@ -178,7 +178,7 @@ void IndirectGlobalAccessModel<fir::TypeDescOp>::getReferencedSymbols(
}
template <>
-bool OperationMoveModel<mlir::acc::LoopOp>::canMoveFromDescendantImpl(
+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
@@ -187,7 +187,7 @@ bool OperationMoveModel<mlir::acc::LoopOp>::canMoveFromDescendantImpl(
}
template <>
-bool OperationMoveModel<mlir::acc::LoopOp>::canMoveOutOfImpl(
+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.
diff --git a/flang/lib/Optimizer/OpenMP/Support/FIROpenMPOpsInterfaces.cpp b/flang/lib/Optimizer/OpenMP/Support/FIROpenMPOpsInterfaces.cpp
index d5f0d8cac9ca8..a396ef00dc004 100644
--- a/flang/lib/Optimizer/OpenMP/Support/FIROpenMPOpsInterfaces.cpp
+++ b/flang/lib/Optimizer/OpenMP/Support/FIROpenMPOpsInterfaces.cpp
@@ -20,18 +20,17 @@ namespace {
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 'op' operation.
+ // 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 canMoveFromDescendantImpl(mlir::Operation *op,
- mlir::Operation *descendant,
- mlir::Operation *candidate) const;
+ 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,
+ // 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 canMoveOutOfImpl(mlir::Operation *op, mlir::Operation *candidate) const;
+ bool canMoveOutOf(mlir::Operation *op, mlir::Operation *candidate) const;
};
// Helpers to check if T is one of Ts.
@@ -53,14 +52,13 @@ struct OperationMoveModel<
typename std::enable_if<is_any_omp_op_v<OP, OMP_LOOP_WRAPPER_OPS>>::type>
: public fir::OperationMoveOpInterface::ExternalModel<
OperationMoveModel<OP>, OP> {
- bool canMoveFromDescendantImpl(mlir::Operation *op,
- mlir::Operation *descendant,
- mlir::Operation *candidate) const {
+ 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 canMoveOutOfImpl(mlir::Operation *op, mlir::Operation *candidate) const {
+ 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
@@ -75,14 +73,13 @@ 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 canMoveFromDescendantImpl(mlir::Operation *op,
- mlir::Operation *descendant,
- mlir::Operation *candidate) const {
+ 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 canMoveOutOfImpl(mlir::Operation *op, mlir::Operation *candidate) const {
+ bool canMoveOutOf(mlir::Operation *op, mlir::Operation *candidate) const {
// Operations cannot be moved out of OutlineableOpenMPOpInterface operation.
return false;
}
diff --git a/flang/lib/Optimizer/Transforms/LoopInvariantCodeMotion.cpp b/flang/lib/Optimizer/Transforms/LoopInvariantCodeMotion.cpp
index 980d10a1a15df..8ebb8982936e8 100644
--- a/flang/lib/Optimizer/Transforms/LoopInvariantCodeMotion.cpp
+++ b/flang/lib/Optimizer/Transforms/LoopInvariantCodeMotion.cpp
@@ -272,28 +272,29 @@ void LoopInvariantCodeMotion::runOnOperation() {
};
getOperation()->walk([&](LoopLikeOpInterface loopLike) {
- auto loopMoveIface =
- dyn_cast<fir::OperationMoveOpInterface>(loopLike.getOperation());
- if (loopMoveIface && !loopMoveIface.canMoveOutOf(nullptr)) {
- LDBG()
- << "Cannot hoist anything out of OperationMoveOpInterface 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. We assume that all such
- // operations properly implement fir::OperationMoveOpInterface.
+ // to such a parent would be invalid. We rely on
+ // fir::canMoveFromDescendant() to identify whether the hoisting
+ // is allowed.
Operation *parentOp = loopLike->getParentOp();
- auto parentMoveIface =
- dyn_cast_or_null<fir::OperationMoveOpInterface>(parentOp);
if (!parentOp) {
LDBG() << "Skipping top-level loop-like operation?";
return;
- } else if (parentMoveIface &&
- !parentMoveIface.canMoveFromDescendant(loopLike, nullptr)) {
- LDBG() << "Cannot hoist anything into OperationMoveOpInterface 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(
@@ -304,12 +305,11 @@ void LoopInvariantCodeMotion::runOnOperation() {
},
/*shouldMoveOutOfRegion=*/
[&](Operation *op, Region *) {
- if (loopMoveIface && !loopMoveIface.canMoveOutOf(op)) {
+ if (!fir::canMoveOutOf(loopLike, op)) {
LDBG() << "Cannot hoist " << *op << " out of the loop";
return false;
}
- if (parentMoveIface &&
- !parentMoveIface.canMoveFromDescendant(loopLike, op)) {
+ if (!fir::canMoveFromDescendant(parentOp, loopLike, op)) {
LDBG() << "Cannot hoist " << *op << " into the parent of the loop";
return false;
}
More information about the Mlir-commits
mailing list