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

[MSFT] Add multicycle path op #6262

Merged
merged 2 commits into from
Oct 17, 2023
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
8 changes: 7 additions & 1 deletion include/circt/Dialect/MSFT/MSFTOpInterfaces.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,13 @@

namespace circt {
namespace msft {
LogicalResult verifyDynInstData(Operation *);
LogicalResult verifyUnaryDynInstDataOp(Operation *);

/// Returns the top-level module which the given HierPathOp that defines
/// pathSym, refers to.
Operation *getHierPathTopModule(Location loc,
circt::hw::HWSymbolCache &symCache,
FlatSymbolRefAttr pathSym);
class InstanceHierarchyOp;
} // namespace msft
} // namespace circt
Expand Down
48 changes: 20 additions & 28 deletions include/circt/Dialect/MSFT/MSFTOpInterfaces.td
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,31 @@
include "mlir/IR/OpBase.td"

def DynInstDataOpInterface : OpInterface<"DynInstDataOpInterface"> {
let description = [{
Interface for ops that define dynamic instance data.
}];
let cppNamespace = "::circt::msft";

let methods = [
InterfaceMethod<
/*desc=*/[{
Get the top module op to which this op is providing data for.
}],
/*retTy=*/"Operation *",
/*methodName=*/"getTopModule",
/*args=*/(ins "circt::hw::HWSymbolCache &":$symCache)
>
];
}

def UnaryDynInstDataOpInterface : OpInterface<"UnaryDynInstDataOpInterface",
[DynInstDataOpInterface]> {
let description = [{
Interface for anything which needs to refer to a HierPathOp.
}];
let cppNamespace = "::circt::msft";
let verify = [{
return ::circt::msft::verifyDynInstData($_op);
return ::circt::msft::verifyUnaryDynInstDataOp($_op);
}];

let methods = [
Expand All @@ -41,33 +60,6 @@ def DynInstDataOpInterface : OpInterface<"DynInstDataOpInterface"> {
/*defaultImplementation=*/[{
return $_op.getRefAttr();
}]
>,
InterfaceMethod<
/*desc=*/[{
Get the top module op to which the HierPathOp which this op is referring.
}],
/*retTy=*/"Operation *",
/*methodName=*/"getTopModule",
/*args=*/(ins "circt::hw::HWSymbolCache &":$symCache),
/*methodBody=*/[{}],
/*defaultImplementation=*/[{
FlatSymbolRefAttr refSym = $_op.getPathSym();
if (!refSym) {
$_op->emitOpError("must run dynamic instance lowering first");
return nullptr;
}
auto ref = dyn_cast_or_null<hw::HierPathOp>(
symCache.getDefinition(refSym));
if (!ref) {
$_op->emitOpError("could not find hw.hierpath ") << refSym;
return nullptr;
}
if (ref.getNamepath().empty())
return nullptr;
auto modSym = FlatSymbolRefAttr::get(
ref.getNamepath()[0].cast<hw::InnerRefAttr>().getModule());
return symCache.getDefinition(modSym);
}]
>
];
}
51 changes: 47 additions & 4 deletions include/circt/Dialect/MSFT/MSFTPDOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def DeclPhysicalRegionOp : MSFTOp<"physical_region",
}

def PDPhysLocationOp : MSFTOp<"pd.location",
[DeclareOpInterfaceMethods<DynInstDataOpInterface>]> {
[DeclareOpInterfaceMethods<UnaryDynInstDataOpInterface>]> {
let summary = "Specify a location for an instance";
let description = [{
Used to specify a specific location on an FPGA to place a dynamic instance.
Expand All @@ -38,10 +38,16 @@ def PDPhysLocationOp : MSFTOp<"pd.location",
let assemblyFormat = [{
($ref^)? custom<PhysLoc>($loc) (`path` `:` $subPath^)? attr-dict
}];

let extraClassDeclaration = [{
Operation* getTopModule(circt::hw::HWSymbolCache &symCache) {
return getHierPathTopModule(getOperation()->getLoc(), symCache, getPathSym());
}
}];
}

def PDRegPhysLocationOp : MSFTOp<"pd.reg_location",
[DeclareOpInterfaceMethods<DynInstDataOpInterface>]> {
[DeclareOpInterfaceMethods<UnaryDynInstDataOpInterface>]> {
let summary = "Specify register locations";
let description = [{
A version of "PDPhysLocationOp" specialized for registers, which have one
Expand All @@ -52,10 +58,34 @@ def PDRegPhysLocationOp : MSFTOp<"pd.reg_location",
let assemblyFormat = [{
(`ref` $ref^)? custom<ListOptionalRegLocList>($locs) attr-dict
}];

let extraClassDeclaration = [{
Operation* getTopModule(circt::hw::HWSymbolCache &symCache) {
return getHierPathTopModule(getOperation()->getLoc(), symCache, getPathSym());
}
}];
}

def PDMulticycleOp : MSFTOp<"pd.multicycle",
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: dunno if I'd consider a multi-cycle path a PD optimization. If it just generates tcl (as this appears to do), it's needed for correctness (I think), in which case it's definitely not a PD optimization. Just a necessary tcl command which is being abstracted here.

I would suggest (longer term) creating a MCP op in the pipeline dialect which both results in tcl and omits the registers.

[DeclareOpInterfaceMethods<DynInstDataOpInterface>]> {
let summary = "Specify a multicycle constraint";
let description = [{
Specifies a multicycle constraint in between two registers.
`source` and `dest` symbols reference `HierPathOp` symbols denoting the
exact registers in the instance hierarchy to which the constraint applies.
}];
let arguments = (ins
FlatSymbolRefAttr:$source,
FlatSymbolRefAttr:$dest,
ConfinedAttr<I32Attr, [IntMinValue<1>]>:$cycles
);
let assemblyFormat = [{
$cycles $source `->` $dest attr-dict
}];
}

def PDPhysRegionOp : MSFTOp<"pd.physregion",
[DeclareOpInterfaceMethods<DynInstDataOpInterface>]> {
[DeclareOpInterfaceMethods<UnaryDynInstDataOpInterface>]> {
let summary = "Specify a physical region for an instance";
let description = [{
Annotate a particular entity within an op with the region of the devices
Expand All @@ -68,6 +98,12 @@ def PDPhysRegionOp : MSFTOp<"pd.physregion",
let assemblyFormat = [{
($ref^)? $physRegionRef (`path` `:` $subPath^)? attr-dict
}];

let extraClassDeclaration = [{
Operation* getTopModule(circt::hw::HWSymbolCache &symCache) {
return getHierPathTopModule(getOperation()->getLoc(), symCache, getPathSym());
}
}];
}

def InstanceHierarchyOp : MSFTOp<"instance.hierarchy",
Expand Down Expand Up @@ -123,7 +159,8 @@ def DynamicInstanceOp : MSFTOp<"instance.dynamic",
}

def DynamicInstanceVerbatimAttrOp : MSFTOp<
"instance.verb_attr", [DeclareOpInterfaceMethods<DynInstDataOpInterface>]> {
"instance.verb_attr", [
DeclareOpInterfaceMethods<UnaryDynInstDataOpInterface>]> {
let summary = "Specify an arbitrary attribute attached to a dynamic instance";
let description = [{
Allows a user to specify a custom attribute name and value which is attached
Expand All @@ -139,4 +176,10 @@ def DynamicInstanceVerbatimAttrOp : MSFTOp<
let assemblyFormat = [{
($ref^)? `name` `:` $name `value` `:` $value (`path` `:` $subPath^)? attr-dict
}];

let extraClassDeclaration = [{
Operation* getTopModule(circt::hw::HWSymbolCache &symCache) {
return getHierPathTopModule(getOperation()->getLoc(), symCache, getPathSym());
}
}];
}
32 changes: 26 additions & 6 deletions lib/Dialect/MSFT/ExportQuartusTcl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,27 +94,33 @@ struct TclOutputState {
SmallVector<Attribute> symbolRefs;

void emit(PhysLocationAttr);
LogicalResult emitLocationAssignment(DynInstDataOpInterface refOp,
LogicalResult emitLocationAssignment(UnaryDynInstDataOpInterface refOp,
PhysLocationAttr,
std::optional<StringRef> subpath);

LogicalResult emit(PDPhysRegionOp region);
LogicalResult emit(PDPhysLocationOp loc);
LogicalResult emit(PDRegPhysLocationOp);
LogicalResult emit(DynamicInstanceVerbatimAttrOp attr);
LogicalResult emit(PDMulticycleOp op);

void emitPath(hw::HierPathOp ref, std::optional<StringRef> subpath);
void emitInnerRefPart(hw::InnerRefAttr innerRef);

/// Get the HierPathOp to which the given operation is pointing. Add it to
/// the set of used global refs.
HierPathOp getRefOp(DynInstDataOpInterface op) {
auto ref = dyn_cast_or_null<hw::HierPathOp>(
emitter.getDefinition(op.getPathSym()));
HierPathOp getRefOp(UnaryDynInstDataOpInterface op) {
return getRefOp(op.getLoc(), op.getPathSym());
}

/// Get the HierPathOp to which a given value is pointing. Add it to the
/// set of used global refs.
HierPathOp getRefOp(Location loc, FlatSymbolRefAttr pathSym) {
auto ref = dyn_cast_or_null<hw::HierPathOp>(emitter.getDefinition(pathSym));
if (ref)
emitter.usedRef(ref);
else
op.emitOpError("could not find hw.hierpath named ") << op.getPathSym();
emitError(loc, "could not find hw.hierpath named ") << pathSym;
return ref;
}
};
Expand Down Expand Up @@ -166,7 +172,7 @@ void TclOutputState::emit(PhysLocationAttr pla) {
/// "set_location_assignment MPDSP_X34_Y285_N0 -to
/// $parent|fooInst|entityName(subpath)"
LogicalResult
TclOutputState::emitLocationAssignment(DynInstDataOpInterface refOp,
TclOutputState::emitLocationAssignment(UnaryDynInstDataOpInterface refOp,
PhysLocationAttr loc,
std::optional<StringRef> subpath) {
indent() << "set_location_assignment ";
Expand Down Expand Up @@ -199,6 +205,19 @@ LogicalResult TclOutputState::emit(PDRegPhysLocationOp locs) {
return success();
}

LogicalResult TclOutputState::emit(PDMulticycleOp op) {
indent() << "set_multicycle_path ";
os << "-hold 1 ";
Copy link
Contributor

Choose a reason for hiding this comment

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

Previous we emitted the hold and setup on different lines of SDC. Have you checked that this works with quartus?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have not, but based on the documentation i see no reason how this should not work.

os << "-setup " << op.getCycles() << " ";
os << "-from [get_registers {$parent|";
emitPath(getRefOp(op.getLoc(), op.getSourceAttr()), std::nullopt);
os << "}] ";
os << "-to [get_registers {$parent|";
emitPath(getRefOp(op.getLoc(), op.getDestAttr()), std::nullopt);
os << "}]\n";
return success();
}

/// Emit tcl in the form of:
/// "set_global_assignment -name NAME VALUE -to $parent|fooInst|entityName"
LogicalResult TclOutputState::emit(DynamicInstanceVerbatimAttrOp attr) {
Expand Down Expand Up @@ -299,6 +318,7 @@ LogicalResult TclEmitter::emit(Operation *hwMod, StringRef outputFile) {
.Case([&](PDPhysLocationOp op) { return state.emit(op); })
.Case([&](PDRegPhysLocationOp op) { return state.emit(op); })
.Case([&](PDPhysRegionOp op) { return state.emit(op); })
.Case([&](PDMulticycleOp op) { return state.emit(op); })
.Case([&](DynamicInstanceVerbatimAttrOp op) {
return state.emit(op);
})
Expand Down
21 changes: 19 additions & 2 deletions lib/Dialect/MSFT/MSFTOpInterfaces.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@
using namespace circt;
using namespace msft;

LogicalResult circt::msft::verifyDynInstData(Operation *op) {
LogicalResult circt::msft::verifyUnaryDynInstDataOp(Operation *op) {
auto inst = dyn_cast<DynamicInstanceOp>(op->getParentOp());
FlatSymbolRefAttr pathRef = cast<DynInstDataOpInterface>(op).getPathSym();
FlatSymbolRefAttr pathRef =
cast<UnaryDynInstDataOpInterface>(op).getPathSym();

if (inst && pathRef)
return op->emitOpError("cannot both have a global ref symbol and be a "
Expand All @@ -25,6 +26,22 @@ LogicalResult circt::msft::verifyDynInstData(Operation *op) {
return success();
}

Operation *circt::msft::getHierPathTopModule(Location loc,
circt::hw::HWSymbolCache &symCache,
FlatSymbolRefAttr pathSym) {
assert(pathSym && "pathSym must be non-null");
auto ref = dyn_cast_or_null<hw::HierPathOp>(symCache.getDefinition(pathSym));
if (!ref) {
emitError(loc) << "could not find hw.hierpath " << pathSym;
return nullptr;
}
if (ref.getNamepath().empty())
return nullptr;
auto modSym = FlatSymbolRefAttr::get(
ref.getNamepath()[0].cast<hw::InnerRefAttr>().getModule());
return symCache.getDefinition(modSym);
}

namespace circt {
namespace msft {
#include "circt/Dialect/MSFT/MSFTOpInterfaces.cpp.inc"
Expand Down
17 changes: 17 additions & 0 deletions lib/Dialect/MSFT/MSFTOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,5 +248,22 @@ LogicalResult LinearOp::verify() {
return success();
}

//===----------------------------------------------------------------------===//
// PDMulticycleOp
//===----------------------------------------------------------------------===//

Operation *PDMulticycleOp::getTopModule(hw::HWSymbolCache &cache) {
// Both symbols should reference the same top-level module in their respective
// HierPath ops.
Operation *srcTop = getHierPathTopModule(getLoc(), cache, getSourceAttr());
Operation *dstTop = getHierPathTopModule(getLoc(), cache, getDestAttr());
if (srcTop != dstTop) {
emitOpError("source and destination paths must refer to the same top-level "
"module.");
return nullptr;
}
return srcTop;
}

#define GET_OP_CLASSES
#include "circt/Dialect/MSFT/MSFT.cpp.inc"
2 changes: 1 addition & 1 deletion lib/Dialect/MSFT/Transforms/MSFTLowerInstances.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ LogicalResult LowerInstancesPass::lower(DynamicInstanceOp inst,
hierBlock.insert(&op);

// Assign a ref for ops which need it.
if (auto specOp = dyn_cast<DynInstDataOpInterface>(op)) {
if (auto specOp = dyn_cast<UnaryDynInstDataOpInterface>(op)) {
assert(ref);
specOp.setPathOp(ref);
}
Expand Down
10 changes: 9 additions & 1 deletion test/Dialect/MSFT/dynamic_instance.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@ msft.instance.hierarchy @reg "bar" {
// CHECK: hw.hierpath @instref_3 [@reg::@reg]
// CHECK-DAG: msft.pd.reg_location ref @instref_3 i4 [<3, 4, 5>, *, *, *]


hw.hierpath @reg.reg [@reg::@reg]
hw.hierpath @reg.reg2 [@reg::@reg2]
msft.instance.hierarchy @reg "multicycle" {
msft.pd.multicycle 2 @reg.reg -> @reg.reg2
}

hw.module.extern @Foo()

Expand Down Expand Up @@ -76,6 +80,7 @@ hw.module @deeper () {

hw.module @reg (in %input : i4, in %clk : !seq.clock) {
%reg = seq.compreg sym @reg %input, %clk : i4
%reg2 = seq.compreg sym @reg2 %input, %clk : i4
}
// TCL-LABEL: proc reg_0_foo_config
// TCL: set_location_assignment FF_X1_Y2_N3 -to $parent|reg_0[1]
Expand All @@ -84,3 +89,6 @@ hw.module @reg (in %input : i4, in %clk : !seq.clock) {

// TCL-LABEL: proc reg_0_bar_config
// TCL: set_location_assignment FF_X3_Y4_N5 -to $parent|reg_0[0]

// TCL-LABEL: proc reg_0_multicycle_config { parent } {
// TCL: set_multicycle_path -hold 1 -setup 2 -from [get_registers {$parent|reg_0}] -to [get_registers {$parent|reg2}]