Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CIR][Interfaces] Implement LoopOpInterface #405

Merged
merged 5 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clang/include/clang/CIR/Dialect/IR/CIRDialect.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "clang/CIR/Dialect/IR/CIRTypes.h"

#include "clang/CIR/Interfaces/ASTAttrInterfaces.h"
#include "clang/CIR/Interfaces/CIRLoopOpInterface.h"
#include "clang/CIR/Interfaces/CIROpInterfaces.h"

namespace mlir {
Expand Down
23 changes: 22 additions & 1 deletion clang/include/clang/CIR/Dialect/IR/CIROps.td
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ include "clang/CIR/Dialect/IR/CIRAttrs.td"

include "clang/CIR/Interfaces/ASTAttrInterfaces.td"
include "clang/CIR/Interfaces/CIROpInterfaces.td"
include "clang/CIR/Interfaces/CIRLoopOpInterface.td"

include "mlir/Interfaces/ControlFlowInterfaces.td"
include "mlir/Interfaces/FunctionInterfaces.td"
Expand Down Expand Up @@ -1172,7 +1173,8 @@ def LoopOpKind : I32EnumAttr<
}

def LoopOp : CIR_Op<"loop",
[DeclareOpInterfaceMethods<LoopLikeOpInterface>,
[LoopOpInterface,
DeclareOpInterfaceMethods<LoopLikeOpInterface>,
DeclareOpInterfaceMethods<RegionBranchOpInterface>,
RecursivelySpeculatable, NoRegionArguments]> {
let summary = "Loop";
Expand Down Expand Up @@ -1236,6 +1238,25 @@ def LoopOp : CIR_Op<"loop",
];

let hasVerifier = 1;

let extraClassDeclaration = [{
Region *maybeGetStep() {
if (getKind() == LoopOpKind::For)
return &getStep();
return nullptr;
}

llvm::SmallVector<Region *> getRegionsInExecutionOrder() {
switch(getKind()) {
case LoopOpKind::For:
return llvm::SmallVector<Region *, 3>{&getCond(), &getBody(), &getStep()};
case LoopOpKind::While:
return llvm::SmallVector<Region *, 2>{&getCond(), &getBody()};
case LoopOpKind::DoWhile:
return llvm::SmallVector<Region *, 2>{&getBody(), &getCond()};
}
}
}];
}

//===----------------------------------------------------------------------===//
Expand Down
36 changes: 36 additions & 0 deletions clang/include/clang/CIR/Interfaces/CIRLoopOpInterface.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
//===- CIRLoopOpInterface.h - Interface for CIR loop-like ops --*- 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
//
//===---------------------------------------------------------------------===//
//
// Defines the interface to generically handle CIR loop operations.
//
//===----------------------------------------------------------------------===//

#ifndef CLANG_INTERFACES_CIR_CIRLOOPOPINTERFACE_H_
#define CLANG_INTERFACES_CIR_CIRLOOPOPINTERFACE_H_

#include "mlir/IR/BuiltinTypes.h"
#include "mlir/IR/OpDefinition.h"
#include "mlir/IR/Operation.h"
#include "mlir/Interfaces/ControlFlowInterfaces.h"
#include "mlir/Interfaces/LoopLikeInterface.h"

namespace mlir {
namespace cir {
namespace detail {

/// Verify invariants of the LoopOpInterface.
::mlir::LogicalResult verifyLoopOpInterface(::mlir::Operation *op);

} // namespace detail
} // namespace cir
} // namespace mlir

/// Include the tablegen'd interface declarations.
#include "clang/CIR/Interfaces/CIRLoopOpInterface.h.inc"

#endif // CLANG_INTERFACES_CIR_CIRLOOPOPINTERFACE_H_
100 changes: 100 additions & 0 deletions clang/include/clang/CIR/Interfaces/CIRLoopOpInterface.td
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
//===- CIRLoopOpInterface.td - Interface for CIR loop-like ops -*- 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
//
//===---------------------------------------------------------------------===//

#ifndef CLANG_CIR_INTERFACES_CIRLOOPOPINTERFACE
#define CLANG_CIR_INTERFACES_CIRLOOPOPINTERFACE

include "mlir/IR/OpBase.td"
include "mlir/Interfaces/ControlFlowInterfaces.td"
include "mlir/Interfaces/LoopLikeInterface.td"

def LoopOpInterface : OpInterface<"LoopOpInterface", [
DeclareOpInterfaceMethods<RegionBranchOpInterface>,
DeclareOpInterfaceMethods<LoopLikeOpInterface>
]> {
let description = [{
Contains helper functions to query properties and perform transformations
on a loop.
}];
let cppNamespace = "::mlir::cir";

let methods = [
InterfaceMethod<[{
Returns the loop's conditional region.
}],
/*retTy=*/"mlir::Region &",
/*methodName=*/"getCond"
>,
InterfaceMethod<[{
Returns the loop's body region.
}],
/*retTy=*/"mlir::Region &",
/*methodName=*/"getBody"
>,
InterfaceMethod<[{
Returns a pointer to the loop's step region or nullptr.
}],
/*retTy=*/"mlir::Region *",
/*methodName=*/"maybeGetStep",
/*args=*/(ins),
/*methodBody=*/"",
/*defaultImplementation=*/"return nullptr;"
>,
InterfaceMethod<[{
Returns the first region to be executed in the loop.
}],
/*retTy=*/"mlir::Region &",
/*methodName=*/"getEntry",
/*args=*/(ins),
/*methodBody=*/"",
/*defaultImplementation=*/"return $_op.getCond();"
>,
InterfaceMethod<[{
Returns a list of regions in order of execution.
}],
/*retTy=*/"llvm::SmallVector<mlir::Region *>",
/*methodName=*/"getRegionsInExecutionOrder",
/*args=*/(ins),
/*methodBody=*/"",
/*defaultImplementation=*/[{
return llvm::SmallVector<mlir::Region *, 2>{&$_op.getRegion(0), &$_op.getRegion(1)};
}]
>,
InterfaceMethod<[{
Recursively walks the body of the loop in pre-order while skipping
nested loops and executing a callback on every other operation.
}],
/*retTy=*/"mlir::WalkResult",
/*methodName=*/"walkBodySkippingNestedLoops",
/*args=*/(ins "::llvm::function_ref<void (Operation *)>":$callback),
/*methodBody=*/"",
/*defaultImplementation=*/[{
return $_op.getBody().template walk<WalkOrder::PreOrder>([&](Operation *op) {
if (isa<LoopOpInterface>(op))
return mlir::WalkResult::skip();
callback(op);
return mlir::WalkResult::advance();
});
}]
>
];

let extraClassDeclaration = [{
/// Generic method to retrieve the successors of a LoopOpInterface operation.
static void getLoopOpSuccessorRegions(
::mlir::cir::LoopOpInterface op, ::mlir::RegionBranchPoint point,
::mlir::SmallVectorImpl<::mlir::RegionSuccessor> &regions);
}];

let verify = [{
/// Verify invariants of the LoopOpInterface.
return detail::verifyLoopOpInterface($_op);
}];
}

#endif // CLANG_CIR_INTERFACES_CIRLOOPOPINTERFACE
1 change: 1 addition & 0 deletions clang/include/clang/CIR/Interfaces/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@ endfunction()

add_clang_mlir_attr_interface(ASTAttrInterfaces)
add_clang_mlir_op_interface(CIROpInterfaces)
add_clang_mlir_op_interface(CIRLoopOpInterface)
1 change: 1 addition & 0 deletions clang/lib/CIR/CodeGen/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ add_clang_library(clangCIR
MLIRCIROpsIncGen
MLIRCIRASTAttrInterfacesIncGen
MLIRCIROpInterfacesIncGen
MLIRCIRLoopOpInterfaceIncGen
${dialect_libs}

LINK_LIBS
Expand Down
37 changes: 7 additions & 30 deletions clang/lib/CIR/Dialect/IR/CIRDialect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "clang/CIR/Dialect/IR/CIRAttrs.h"
#include "clang/CIR/Dialect/IR/CIROpsEnums.h"
#include "clang/CIR/Dialect/IR/CIRTypes.h"
#include "clang/CIR/Interfaces/CIRLoopOpInterface.h"
#include "llvm/Support/ErrorHandling.h"
#include <optional>

Expand Down Expand Up @@ -232,7 +233,7 @@ void AllocaOp::build(::mlir::OpBuilder &odsBuilder,
//===----------------------------------------------------------------------===//

LogicalResult BreakOp::verify() {
if (!getOperation()->getParentOfType<LoopOp>() &&
if (!getOperation()->getParentOfType<LoopOpInterface>() &&
!getOperation()->getParentOfType<SwitchOp>())
return emitOpError("must be within a loop or switch");
return success();
Expand All @@ -251,7 +252,7 @@ void ConditionOp::getSuccessorRegions(
// down its list of possible successors.

// Parent is a loop: condition may branch to the body or to the parent op.
if (auto loopOp = dyn_cast<LoopOp>(getOperation()->getParentOp())) {
if (auto loopOp = dyn_cast<LoopOpInterface>(getOperation()->getParentOp())) {
regions.emplace_back(&loopOp.getBody(), loopOp.getBody().getArguments());
regions.emplace_back(loopOp->getResults());
}
Expand All @@ -269,7 +270,7 @@ ConditionOp::getMutableSuccessorOperands(RegionBranchPoint point) {
}

LogicalResult ConditionOp::verify() {
if (!isa<LoopOp, AwaitOp>(getOperation()->getParentOp()))
if (!isa<LoopOpInterface, AwaitOp>(getOperation()->getParentOp()))
return emitOpError("condition must be within a conditional region");
return success();
}
Expand Down Expand Up @@ -365,7 +366,7 @@ OpFoldResult ConstantOp::fold(FoldAdaptor /*adaptor*/) { return getValue(); }
//===----------------------------------------------------------------------===//

LogicalResult ContinueOp::verify() {
if (!this->getOperation()->getParentOfType<LoopOp>())
if (!this->getOperation()->getParentOfType<LoopOpInterface>())
return emitOpError("must be within a loop");
return success();
}
Expand Down Expand Up @@ -1264,38 +1265,14 @@ void LoopOp::build(OpBuilder &builder, OperationState &result,
stepBuilder(builder, result.location);
}

/// Given the region at `index`, or the parent operation if `index` is None,
/// return the successor regions. These are the regions that may be selected
/// during the flow of control. `operands` is a set of optional attributes
/// that correspond to a constant value for each operand, or null if that
/// operand is not a constant.
void LoopOp::getSuccessorRegions(mlir::RegionBranchPoint point,
SmallVectorImpl<RegionSuccessor> &regions) {
// If any index all the underlying regions branch back to the parent
// operation.
if (!point.isParent()) {
regions.push_back(RegionSuccessor());
return;
}

// FIXME: we want to look at cond region for getting more accurate results
// if the other regions will get a chance to execute.
regions.push_back(RegionSuccessor(&this->getCond()));
regions.push_back(RegionSuccessor(&this->getBody()));
regions.push_back(RegionSuccessor(&this->getStep()));
LoopOpInterface::getLoopOpSuccessorRegions(*this, point, regions);
}

llvm::SmallVector<Region *> LoopOp::getLoopRegions() { return {&getBody()}; }

LogicalResult LoopOp::verify() {
if (getCond().empty())
return emitOpError() << "cond region must not be empty";

if (!llvm::isa<ConditionOp>(getCond().back().getTerminator()))
return emitOpError() << "cond region terminate with 'cir.condition'";

return success();
}
LogicalResult LoopOp::verify() { return success(); }

//===----------------------------------------------------------------------===//
// GlobalOp
Expand Down
1 change: 1 addition & 0 deletions clang/lib/CIR/Dialect/IR/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ add_clang_library(MLIRCIR
MLIRSymbolInterfacesIncGen
MLIRCIRASTAttrInterfacesIncGen
MLIRCIROpInterfacesIncGen
MLIRCIRLoopOpInterfaceIncGen

LINK_LIBS PUBLIC
MLIRIR
Expand Down
26 changes: 5 additions & 21 deletions clang/lib/CIR/Dialect/Transforms/LifetimeCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "clang/CIR/Dialect/IR/CIRDialect.h"
#include "clang/CIR/Dialect/Passes.h"

#include "clang/CIR/Interfaces/CIRLoopOpInterface.h"
#include "llvm/ADT/SetOperations.h"
#include "llvm/ADT/SmallSet.h"

Expand Down Expand Up @@ -674,27 +675,10 @@ void LifetimeCheckPass::checkLoop(LoopOp loopOp) {
SmallVector<Region *, 4> regionsToCheck;

auto setupLoopRegionsToCheck = [&](bool isSubsequentTaken = false) {
regionsToCheck.clear();
switch (loopOp.getKind()) {
case LoopOpKind::For: {
regionsToCheck.push_back(&loopOp.getCond());
regionsToCheck.push_back(&loopOp.getBody());
if (!isSubsequentTaken)
regionsToCheck.push_back(&loopOp.getStep());
break;
}
case LoopOpKind::While: {
regionsToCheck.push_back(&loopOp.getCond());
regionsToCheck.push_back(&loopOp.getBody());
break;
}
case LoopOpKind::DoWhile: {
// Note this is the reverse order from While above.
regionsToCheck.push_back(&loopOp.getBody());
regionsToCheck.push_back(&loopOp.getCond());
break;
}
}
regionsToCheck = loopOp.getRegionsInExecutionOrder();
// Drop step if it exists and we are not checking the subsequent taken.
if (loopOp.maybeGetStep() && !isSubsequentTaken)
regionsToCheck.pop_back();
};

// From 2.4.9 "Note":
Expand Down
1 change: 1 addition & 0 deletions clang/lib/CIR/FrontendAction/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ add_clang_library(clangCIRFrontendAction
MLIRCIROpsIncGen
MLIRCIRASTAttrInterfacesIncGen
MLIRCIROpInterfacesIncGen
MLIRCIRLoopOpInterfaceIncGen
MLIRBuiltinLocationAttributesIncGen
MLIRBuiltinTypeInterfacesIncGen
MLIRFunctionInterfacesIncGen
Expand Down
54 changes: 54 additions & 0 deletions clang/lib/CIR/Interfaces/CIRLoopOpInterface.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
//===- CIRLoopOpInterface.cpp - Interface for CIR loop-like ops *- 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
//
//===---------------------------------------------------------------------===//

#include "clang/CIR/Interfaces/CIRLoopOpInterface.h"

#include "clang/CIR/Dialect/IR/CIRDialect.h"
#include "clang/CIR/Interfaces/CIRLoopOpInterface.cpp.inc"
#include "llvm/Support/ErrorHandling.h"

namespace mlir {
namespace cir {

void LoopOpInterface::getLoopOpSuccessorRegions(
LoopOpInterface op, RegionBranchPoint point,
SmallVectorImpl<RegionSuccessor> &regions) {
assert(point.isParent() || point.getRegionOrNull());

// Branching to first region: go to condition or body (do-while).
if (point.isParent()) {
regions.emplace_back(&op.getEntry(), op.getEntry().getArguments());
}
// Branching from condition: go to body or exit.
else if (&op.getCond() == point.getRegionOrNull()) {
regions.emplace_back(RegionSuccessor(op->getResults()));
regions.emplace_back(&op.getBody(), op.getBody().getArguments());
}
// Branching from body: go to step (for) or condition.
else if (&op.getBody() == point.getRegionOrNull()) {
// FIXME(cir): Should we consider break/continue statements here?
auto *afterBody = (op.maybeGetStep() ? op.maybeGetStep() : &op.getCond());
regions.emplace_back(afterBody, afterBody->getArguments());
}
// Branching from step: go to condition.
else if (op.maybeGetStep() == point.getRegionOrNull()) {
regions.emplace_back(&op.getCond(), op.getCond().getArguments());
}
}

/// Verify invariants of the LoopOpInterface.
LogicalResult detail::verifyLoopOpInterface(Operation *op) {
auto loopOp = cast<LoopOpInterface>(op);
if (!isa<ConditionOp>(loopOp.getCond().back().getTerminator()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sitio-couto this case causes MLIRCIR and MLIRCIRInterfaces to have cyclic dependencies. I disabled it in a opt out commit on tip of main right now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! @YazZz1k caught that in #410, but I'm yet to fix it.

return op->emitOpError(
"expected condition region to terminate with 'cir.condition'");
return success();
}

} // namespace cir
} // namespace mlir
Loading