Skip to content
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 include/circt/Dialect/HW/HWOps.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "circt/Dialect/HW/HWOpInterfaces.h"
#include "circt/Dialect/HW/HWTypes.h"
#include "circt/Support/BuilderUtils.h"
#include "circt/Support/ProceduralRegionTrait.h"
#include "mlir/IR/BuiltinOps.h"
#include "mlir/IR/ImplicitLocOpBuilder.h"
#include "mlir/IR/OpImplementation.h"
Expand Down
6 changes: 4 additions & 2 deletions include/circt/Dialect/HW/HWStructure.td
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ include "circt/Dialect/HW/HWDialect.td"
include "circt/Dialect/HW/HWOpInterfaces.td"
include "circt/Dialect/HW/HWTypes.td"
include "circt/Dialect/Emit/EmitOpInterfaces.td"
include "circt/Support/ProceduralRegionTrait.td"
include "mlir/Interfaces/FunctionInterfaces.td"
include "mlir/IR/OpAsmInterface.td"
include "mlir/IR/OpBase.td"
Expand Down Expand Up @@ -114,7 +115,7 @@ class HWModuleOpBase<string mnemonic, list<Trait> traits = []> :
def HWModuleOp : HWModuleOpBase<"module",
[IsolatedFromAbove, RegionKindInterface,
SingleBlockImplicitTerminator<"OutputOp">,
HWEmittableModuleLike]>{
HWEmittableModuleLike, NonProceduralRegion]>{
let summary = "HW Module";
let description = [{
The "hw.module" operation represents a Verilog module, including a given
Expand Down Expand Up @@ -621,7 +622,8 @@ def EventControlAttr : I32EnumAttr<"EventControl", "edge control trigger",
}

def TriggeredOp : HWOp<"triggered", [
IsolatedFromAbove, SingleBlock, NoTerminator]> {
IsolatedFromAbove, SingleBlock, NoTerminator, NonProceduralOp,
ProceduralRegion]> {
let summary = "A procedural region with a trigger condition";
let description = [{
A procedural region that can be triggered by an event. The trigger
Expand Down
8 changes: 0 additions & 8 deletions include/circt/Dialect/SV/SV.td
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,6 @@ include "circt/Dialect/SV/SVDialect.td"
class SVOp<string mnemonic, list<Trait> traits = []> :
Op<SVDialect, mnemonic, traits>;

def ProceduralOp : NativeOpTrait<"ProceduralOp"> {
let cppNamespace = "::circt::sv";
}

def NonProceduralOp : NativeOpTrait<"NonProceduralOp"> {
let cppNamespace = "::circt::sv";
}

/// Mark an operation as being a vendor extension.
def VendorExtension : NativeOpTrait<"VendorExtension"> {
let cppNamespace = "::circt::sv";
Expand Down
1 change: 1 addition & 0 deletions include/circt/Dialect/SV/SVInOutOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
//===----------------------------------------------------------------------===//

include "circt/Dialect/HW/HWOpInterfaces.td"
include "circt/Support/ProceduralRegionTrait.td"
include "circt/Types.td"
include "mlir/Interfaces/InferTypeOpInterface.td"

Expand Down
26 changes: 0 additions & 26 deletions include/circt/Dialect/SV/SVOps.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,32 +166,6 @@ struct CaseInfo {
// Other Supporting Logic
//===----------------------------------------------------------------------===//

/// Return true if the specified operation is in a procedural region.
LogicalResult verifyInProceduralRegion(Operation *op);
/// Return true if the specified operation is not in a procedural region.
LogicalResult verifyInNonProceduralRegion(Operation *op);

/// This class verifies that the specified op is located in a procedural region.
template <typename ConcreteType>
class ProceduralOp
: public mlir::OpTrait::TraitBase<ConcreteType, ProceduralOp> {
public:
static LogicalResult verifyTrait(Operation *op) {
return verifyInProceduralRegion(op);
}
};

/// This class verifies that the specified op is not located in a procedural
/// region.
template <typename ConcreteType>
class NonProceduralOp
: public mlir::OpTrait::TraitBase<ConcreteType, NonProceduralOp> {
public:
static LogicalResult verifyTrait(Operation *op) {
return verifyInNonProceduralRegion(op);
}
};

/// This class provides a verifier for ops that are expecting their parent
/// to be one of the given parent ops
template <typename ConcreteType>
Expand Down
2 changes: 2 additions & 0 deletions include/circt/Dialect/SV/SVStatements.td
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ include "mlir/Interfaces/CallInterfaces.td"
include "mlir/Interfaces/ControlFlowInterfaces.td"
include "circt/Dialect/Emit/EmitOpInterfaces.td"
include "circt/Dialect/HW/HWOpInterfaces.td"
include "circt/Support/ProceduralRegionTrait.td"

//===----------------------------------------------------------------------===//
// Control flow like-operations
Expand Down Expand Up @@ -48,6 +49,7 @@ def OrderedOutputOp : SVOp<"ordered", [SingleBlock, NoTerminator, NoRegionArgume
def IfDefOp : SVOp<"ifdef", [
NoRegionArguments,
NonProceduralOp,
NonProceduralRegion,
GraphRegionNoTerminator,
DeclareOpInterfaceMethods<SymbolUserOpInterface>
]> {
Expand Down
1 change: 1 addition & 0 deletions include/circt/Dialect/Sim/SimOps.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "circt/Dialect/Sim/SimDialect.h"
#include "circt/Dialect/Sim/SimTypes.h"
#include "circt/Support/BuilderUtils.h"
#include "circt/Support/ProceduralRegionTrait.h"
#include "mlir/Bytecode/BytecodeOpInterface.h"
#include "mlir/IR/OpImplementation.h"
#include "mlir/IR/SymbolTable.h"
Expand Down
14 changes: 7 additions & 7 deletions include/circt/Dialect/Sim/SimOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ include "circt/Dialect/Seq/SeqTypes.td"
include "circt/Dialect/Sim/SimDialect.td"
include "circt/Dialect/Sim/SimOpInterfaces.td"
include "circt/Dialect/Sim/SimTypes.td"
include "circt/Support/ProceduralRegionTrait.td"
include "mlir/Interfaces/FunctionInterfaces.td"
include "mlir/Interfaces/InferTypeOpInterface.td"
include "mlir/Interfaces/SideEffectInterfaces.td"
Expand Down Expand Up @@ -546,7 +547,7 @@ def FormatStringConcatOp : SimOp<"fmt.concat", [Pure]> {
}];
}

def PrintFormattedOp : SimOp<"print"> {
def PrintFormattedOp : SimOp<"print", [NonProceduralOp]> {
let summary = "Print a formatted string on a given clock and condition";
let description = [{
Evaluate a format string and print it to the simulation console on the
Expand Down Expand Up @@ -577,7 +578,7 @@ def PrintFormattedOp : SimOp<"print"> {
let assemblyFormat = "$input `on` $clock `if` $condition (`to` $stream^)? attr-dict";
}

def PrintFormattedProcOp : SimOp<"proc.print"> {
def PrintFormattedProcOp : SimOp<"proc.print", [ProceduralOp]> {
let summary = "Print a formatted string within a procedural region";
let description = [{
Evaluate a format string and print it to the simulation console.
Expand All @@ -595,7 +596,6 @@ def PrintFormattedProcOp : SimOp<"proc.print"> {
build($_builder, $_state, input, mlir::Value());
}]>
];
let hasVerifier = true;
let hasCanonicalizeMethod = true;
let assemblyFormat = "$input (`to` $stream^)? attr-dict";
}
Expand Down Expand Up @@ -1026,7 +1026,7 @@ def QueueConcatOp : SimOp<"queue.concat", [Pure]> {
// Simulation Control
//===----------------------------------------------------------------------===//

def ClockedTerminateOp : SimOp<"clocked_terminate"> {
def ClockedTerminateOp : SimOp<"clocked_terminate", [NonProceduralOp]> {
let summary = "Terminate the simulation";
let description = [{
Implements the semantics of `sim.terminate` if the given condition is true
Expand All @@ -1045,7 +1045,7 @@ def ClockedTerminateOp : SimOp<"clocked_terminate"> {
}];
}

def ClockedPauseOp : SimOp<"clocked_pause"> {
def ClockedPauseOp : SimOp<"clocked_pause", [NonProceduralOp]> {
let summary = "Pause the simulation";
let description = [{
Implements the semantics of `sim.pause` if the given condition is true on
Expand All @@ -1062,7 +1062,7 @@ def ClockedPauseOp : SimOp<"clocked_pause"> {
}];
}

def TerminateOp : SimOp<"terminate"> {
def TerminateOp : SimOp<"terminate", [ProceduralOp]> {
let summary = "Terminate the simulation";
let description = [{
Terminate the simulation with a success or failure exit code. Depending on
Expand Down Expand Up @@ -1092,7 +1092,7 @@ def TerminateOp : SimOp<"terminate"> {
}];
}

def PauseOp : SimOp<"pause"> {
def PauseOp : SimOp<"pause", [ProceduralOp]> {
let summary = "Pause the simulation";
let description = [{
Interrupt the simulation and give control back to the user in case of an
Expand Down
45 changes: 44 additions & 1 deletion include/circt/Support/ProceduralRegionTrait.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
//
//===----------------------------------------------------------------------===//
//
// This header file defines the `ProceduralRegion` trait.
// This header file defines traits for (non-)procedural regions and operations.
//
//===----------------------------------------------------------------------===//

Expand All @@ -19,6 +19,20 @@

namespace circt {

/// Returns `success` if the operation has no closer surrounding parent
/// marked as procedural region than its closest parent marked as
/// non-procedural region.
/// Also returns `success` if no parent is marked as either procedural or
/// non-procedural region.
LogicalResult verifyNotInProceduralRegion(Operation *op);

/// Returns `success` if the operation has no closer surrounding parent
/// marked as non-procedural region than its closest parent marked as
/// procedural region.
/// Also returns `success` if no parent is marked as either procedural or
/// non-procedural region.
LogicalResult verifyNotInNonProceduralRegion(Operation *op);

/// Signals that an operation's regions are procedural.
template <typename ConcreteType>
class ProceduralRegion
Expand All @@ -28,6 +42,35 @@ class ProceduralRegion
}
};

/// Signals that an operation's regions are non-procedural.
template <typename ConcreteType>
class NonProceduralRegion
: public mlir::OpTrait::TraitBase<ConcreteType, NonProceduralRegion> {
static LogicalResult verifyTrait(Operation *op) {
return mlir::OpTrait::impl::verifyNRegions(op, 1);
}
};

/// Signals that an operation must not be in a non-procedural region.
template <typename ConcreteType>
class ProceduralOp
: public mlir::OpTrait::TraitBase<ConcreteType, ProceduralOp> {
public:
static LogicalResult verifyTrait(Operation *op) {
return verifyNotInNonProceduralRegion(op);
}
};

/// Signals that an operation must not be in a procedural region.
template <typename ConcreteType>
class NonProceduralOp
: public mlir::OpTrait::TraitBase<ConcreteType, NonProceduralOp> {
public:
static LogicalResult verifyTrait(Operation *op) {
return verifyNotInProceduralRegion(op);
}
};

} // namespace circt

#endif // CIRCT_SUPPORT_PROCEDURALREGIONTRAIT_H
16 changes: 16 additions & 0 deletions include/circt/Support/ProceduralRegionTrait.td
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,24 @@

include "mlir/IR/OpBase.td"

// Note: The procedural and non-procedural traits are intended to be mutually
// exclusive. If an operation or region is not strictly procedural or
// non-procedural, it should carry neither trait.

def ProceduralRegion : NativeOpTrait<"ProceduralRegion"> {
let cppNamespace = "::circt";
}

def NonProceduralRegion : NativeOpTrait<"NonProceduralRegion"> {
let cppNamespace = "::circt";
}

def ProceduralOp : NativeOpTrait<"ProceduralOp"> {
let cppNamespace = "::circt";
}

def NonProceduralOp : NativeOpTrait<"NonProceduralOp"> {
let cppNamespace = "::circt";
}

#endif // CIRCT_SUPPORT_PROCDURALREGIONTRAIT_TD
1 change: 1 addition & 0 deletions lib/Dialect/LLHD/IR/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ add_circt_dialect_library(CIRCTLLHD
LINK_LIBS PUBLIC
CIRCTHW
CIRCTComb
CIRCTSupport
MLIRIR
MLIRSideEffectInterfaces
MLIRControlFlowInterfaces
Expand Down
14 changes: 0 additions & 14 deletions lib/Dialect/SV/SVOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,20 +56,6 @@ bool sv::isExpression(Operation *op) {
MacroRefExprOp, MacroRefExprSEOp>(op);
}

LogicalResult sv::verifyInProceduralRegion(Operation *op) {
if (op->getParentOp()->hasTrait<ProceduralRegion>())
return success();
op->emitError() << op->getName() << " should be in a procedural region";
return failure();
}

LogicalResult sv::verifyInNonProceduralRegion(Operation *op) {
if (!op->getParentOp()->hasTrait<ProceduralRegion>())
return success();
op->emitError() << op->getName() << " should be in a non-procedural region";
return failure();
}

/// Returns the operation registered with the given symbol name with the regions
/// of 'symbolTableOp'. recurse through nested regions which don't contain the
/// symboltable trait. Returns nullptr if no valid symbol was found.
Expand Down
1 change: 1 addition & 0 deletions lib/Dialect/Sim/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ add_circt_dialect_library(CIRCTSim
LINK_LIBS PUBLIC
CIRCTHW
CIRCTSeq
CIRCTSupport
CIRCTSV
MLIRFuncDialect
MLIRLLVMDialect
Expand Down
23 changes: 0 additions & 23 deletions lib/Dialect/Sim/SimOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -626,29 +626,6 @@ LogicalResult PrintFormattedOp::canonicalize(PrintFormattedOp op,
return failure();
}

LogicalResult PrintFormattedProcOp::verify() {
// Check if we know for sure that the parent is not procedural.
auto *parentOp = getOperation()->getParentOp();

if (!parentOp)
return emitOpError("must be within a procedural region.");

if (isa_and_nonnull<hw::HWDialect>(parentOp->getDialect())) {
if (!isa<hw::TriggeredOp>(parentOp))
return emitOpError("must be within a procedural region.");
return success();
}

if (isa_and_nonnull<sv::SVDialect>(parentOp->getDialect())) {
if (!parentOp->hasTrait<ProceduralRegion>())
return emitOpError("must be within a procedural region.");
return success();
}

// Don't fail for dialects that are not explicitly handled.
return success();
}

LogicalResult PrintFormattedProcOp::canonicalize(PrintFormattedProcOp op,
PatternRewriter &rewriter) {
// Remove empty prints.
Expand Down
1 change: 1 addition & 0 deletions lib/Support/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ add_circt_library(CIRCTSupport
Path.cpp
PrettyPrinter.cpp
PrettyPrinterHelpers.cpp
ProceduralRegionTrait.cpp
SATSolver.cpp
SymCache.cpp
TruthTable.cpp
Expand Down
38 changes: 38 additions & 0 deletions lib/Support/ProceduralRegionTrait.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
//===----------------------------------------------------------------------===//
//
// 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 "circt/Support/ProceduralRegionTrait.h"

using namespace llvm;
using namespace mlir;

namespace circt {

LogicalResult verifyNotInProceduralRegion(Operation *op) {
auto *parent = op;
while ((parent = parent->getParentOp())) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The change in behavior regarding the transitivity of unknown operations makes sense to me 👍

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I'm a bit concerned about the potential performance impact added by recursing over the parent ops. But I hope if we consistently apply the ProceduralRegion/NonProceduralRegion trait to the operations that we control, it shouldn't be noticeable.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah I agree this is negligible for our dialects in tree. It's possible for some users to create deeply nested scf.if etc but I think that's their responsibility not to do that.

if (parent->hasTrait<NonProceduralRegion>())
return success();
if (parent->hasTrait<ProceduralRegion>())
return op->emitOpError("must not be in a procedural region");
}
return success();
}

LogicalResult verifyNotInNonProceduralRegion(Operation *op) {
auto *parent = op;
while ((parent = parent->getParentOp())) {
if (parent->hasTrait<ProceduralRegion>())
return success();
if (parent->hasTrait<NonProceduralRegion>())
return op->emitOpError("must not be in a non-procedural region");
}
return success();
}

} // namespace circt
Loading
Loading