Skip to content

Commit

Permalink
[MSFT] Finally move away from GlobalRefOp (#6168)
Browse files Browse the repository at this point in the history 
Switch to HierPathOp. Closes #5183.
  • Loading branch information
@teqdruid
teqdruid authored Sep 21, 2023
1 parent 960195f commit ff1f72d
Show file tree
Hide file tree
Showing 12 changed files with 59 additions and 133 deletions.
6 changes: 3 additions & 3 deletions include/circt/Dialect/MSFT/ExportTcl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ class TclEmitter {
LogicalResult emit(Operation *hwMod, StringRef outputFile);

Operation *getDefinition(FlatSymbolRefAttr);
const DenseSet<hw::GlobalRefOp> &getRefsUsed() { return refsUsed; }
void usedRef(hw::GlobalRefOp ref) { refsUsed.insert(ref); }
const DenseSet<hw::HierPathOp> &getRefsUsed() { return refsUsed; }
void usedRef(hw::HierPathOp ref) { refsUsed.insert(ref); }

private:
mlir::ModuleOp topLevel;
Expand All @@ -46,7 +46,7 @@ class TclEmitter {
DenseMap<Operation *,
llvm::MapVector<StringAttr, SmallVector<DynInstDataOpInterface, 0>>>
tclOpsForModInstance;
DenseSet<hw::GlobalRefOp> refsUsed;
DenseSet<hw::HierPathOp> refsUsed;

LogicalResult populate();
};
Expand Down
20 changes: 10 additions & 10 deletions include/circt/Dialect/MSFT/MSFTOpInterfaces.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ include "mlir/IR/OpBase.td"

def DynInstDataOpInterface : OpInterface<"DynInstDataOpInterface"> {
let description = [{
Interface for anything which needs to refer to a GlobalRefOp.
Interface for anything which needs to refer to a HierPathOp.
}];
let cppNamespace = "::circt::msft";
let verify = [{
Expand All @@ -20,22 +20,22 @@ def DynInstDataOpInterface : OpInterface<"DynInstDataOpInterface"> {
let methods = [
InterfaceMethod<
/*desc=*/[{
Set the GlobalRefOp to which this op is referring.
Set the HierPathOp to which this op is referring.
}],
/*retTy=*/"void",
/*methodName=*/"setGlobalRef",
/*args=*/(ins "::circt::hw::GlobalRefOp":$ref),
/*methodName=*/"setPathOp",
/*args=*/(ins "::circt::hw::HierPathOp":$ref),
/*methodBody=*/[{}],
/*defaultImplementation=*/[{
$_op.setRefAttr(FlatSymbolRefAttr::get(ref));
}]
>,
InterfaceMethod<
/*desc=*/[{
Get the symbol of the GlobalRefOp to which this op is referring.
Get the symbol of the HierPathOp to which this op is referring.
}],
/*retTy=*/"FlatSymbolRefAttr",
/*methodName=*/"getGlobalRefSym",
/*methodName=*/"getPathSym",
/*args=*/(ins),
/*methodBody=*/[{}],
/*defaultImplementation=*/[{
Expand All @@ -44,22 +44,22 @@ def DynInstDataOpInterface : OpInterface<"DynInstDataOpInterface"> {
>,
InterfaceMethod<
/*desc=*/[{
Get the top module op to which the GlobalRefOp which this op is referring.
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.getGlobalRefSym();
FlatSymbolRefAttr refSym = $_op.getPathSym();
if (!refSym) {
$_op->emitOpError("must run dynamic instance lowering first");
return nullptr;
}
auto ref = dyn_cast_or_null<hw::GlobalRefOp>(
auto ref = dyn_cast_or_null<hw::HierPathOp>(
symCache.getDefinition(refSym));
if (!ref) {
$_op->emitOpError("could not find hw.globalRef ") << refSym;
$_op->emitOpError("could not find hw.hierpath ") << refSym;
return nullptr;
}
if (ref.getNamepath().empty())
Expand Down
12 changes: 6 additions & 6 deletions include/circt/Dialect/MSFT/MSFTPDOps.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def PDPhysLocationOp : MSFTOp<"pd.location",
Supports specifying the location of a subpath for extern modules and device
primitives. Intended to live as a child of `instance.dynamic` initially
without the `ref` field. The dynamic instance lowering will fill in `ref`
with the symol of the `hw.globalref` op corresponding to the lowered dynamic
with the symol of the `hw.hierpath` op corresponding to the lowered dynamic
instance.
}];
let arguments = (ins PhysLocation:$loc,
Expand Down Expand Up @@ -110,14 +110,14 @@ def DynamicInstanceOp : MSFTOp<"instance.dynamic",
Represents an instance (as in instance in the instance hierarchy) referred
to henceforth as a dynamic instance. Specified with a path through the
instance hierarchy (which in the future will be replaced with an AppID).
Lowers to a `hw.globalref` but unlike a global ref, does not require all of
the ops participating in the globalref to contain a back pointer attribute.
Lowers to a `hw.hierpath` but unlike a global ref, does not require all of
the ops participating in the hierpath to contain a back pointer attribute.
Allows users to efficiently add placements to a large number of dynamic
instances which happen to map to a small number of static instances by
bulk-adding the necessary `hw.globalref` attributes.
bulk-adding the necessary `hw.hierpath` attributes.

During the lowering, moves the operations in the body to the top level and
gives them the symbol of the globalref which was created to replace the
gives them the symbol of the hierpath which was created to replace the
dynamic instance.
}];
let arguments = (ins InnerRefAttr:$instanceRef);
Expand All @@ -128,7 +128,7 @@ def DynamicInstanceOp : MSFTOp<"instance.dynamic",
}];

let extraClassDeclaration = [{
::mlir::ArrayAttr globalRefPath();
::mlir::ArrayAttr getPath();
}];
}

Expand Down
6 changes: 2 additions & 4 deletions lib/Dialect/MSFT/DeviceDB.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,7 @@ LogicalResult PlacementDB::insertPlacement(DynInstDataOpInterface op,
if (leaf->locOp != nullptr)
return op->emitOpError("Could not apply placement ")
<< loc << ". Position already occupied by "
<< cast<DynamicInstanceOp>(leaf->locOp->getParentOp())
.globalRefPath();
<< cast<DynamicInstanceOp>(leaf->locOp->getParentOp()).getPath();

leaf->locOp = op;
return success();
Expand Down Expand Up @@ -279,8 +278,7 @@ LogicalResult PlacementDB::movePlacementCheck(DynInstDataOpInterface op,
if (newLeaf->locOp)
return op.emitError(
"cannot move to new location since location is occupied by ")
<< cast<DynamicInstanceOp>(newLeaf->locOp->getParentOp())
.globalRefPath();
<< cast<DynamicInstanceOp>(newLeaf->locOp->getParentOp()).getPath();
return success();
}

Expand Down
19 changes: 9 additions & 10 deletions lib/Dialect/MSFT/ExportQuartusTcl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,19 +103,18 @@ struct TclOutputState {
LogicalResult emit(PDRegPhysLocationOp);
LogicalResult emit(DynamicInstanceVerbatimAttrOp attr);

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

/// Get the GlobalRefOp to which the given operation is pointing. Add it to
/// Get the HierPathOp to which the given operation is pointing. Add it to
/// the set of used global refs.
GlobalRefOp getRefOp(DynInstDataOpInterface op) {
auto ref = dyn_cast_or_null<hw::GlobalRefOp>(
emitter.getDefinition(op.getGlobalRefSym()));
HierPathOp getRefOp(DynInstDataOpInterface op) {
auto ref = dyn_cast_or_null<hw::HierPathOp>(
emitter.getDefinition(op.getPathSym()));
if (ref)
emitter.usedRef(ref);
else
op.emitOpError("could not find hw.globalRef named ")
<< op.getGlobalRefSym();
op.emitOpError("could not find hw.hierpath named ") << op.getPathSym();
return ref;
}
};
Expand All @@ -130,7 +129,7 @@ void TclOutputState::emitInnerRefPart(hw::InnerRefAttr innerRef) {
symbolRefs.push_back(innerRef);
}

void TclOutputState::emitPath(hw::GlobalRefOp ref,
void TclOutputState::emitPath(hw::HierPathOp ref,
std::optional<StringRef> subpath) {
// Traverse each part of the path.
auto parts = ref.getNamepathAttr().getAsRange<hw::InnerRefAttr>();
Expand Down Expand Up @@ -211,7 +210,7 @@ LogicalResult TclOutputState::emit(PDRegPhysLocationOp locs) {
/// Emit tcl in the form of:
/// "set_global_assignment -name NAME VALUE -to $parent|fooInst|entityName"
LogicalResult TclOutputState::emit(DynamicInstanceVerbatimAttrOp attr) {
GlobalRefOp ref = getRefOp(attr);
HierPathOp ref = getRefOp(attr);
indent() << "set_instance_assignment -name " << attr.getName() << " "
<< attr.getValue();

Expand All @@ -228,7 +227,7 @@ LogicalResult TclOutputState::emit(DynamicInstanceVerbatimAttrOp attr) {
/// set_instance_assignment -name CORE_ONLY_PLACE_REGION ON -to $parent|a|b|c
/// set_instance_assignment -name REGION_NAME test_region -to $parent|a|b|c
LogicalResult TclOutputState::emit(PDPhysRegionOp region) {
GlobalRefOp ref = getRefOp(region);
HierPathOp ref = getRefOp(region);

auto physicalRegion = dyn_cast_or_null<DeclPhysicalRegionOp>(
emitter.getDefinition(region.getPhysRegionRefAttr()));
Expand Down
7 changes: 3 additions & 4 deletions lib/Dialect/MSFT/MSFTOpInterfaces.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,12 @@ using namespace msft;

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

if (inst && globalRef)
if (inst && pathRef)
return op->emitOpError("cannot both have a global ref symbol and be a "
"child of a dynamic instance op");
if (!inst && !globalRef)
if (!inst && !pathRef)
return op->emitOpError("must have either a global ref symbol of belong to "
"a dynamic instance op");
return success();
Expand Down
2 changes: 1 addition & 1 deletion lib/Dialect/MSFT/MSFTOps.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ static void printModuleLikeOp(hw::HWModuleLike moduleLike, OpAsmPrinter &p,
// DynamicInstanceOp
//===----------------------------------------------------------------------===//

ArrayAttr DynamicInstanceOp::globalRefPath() {
ArrayAttr DynamicInstanceOp::getPath() {
SmallVector<Attribute, 16> path;
DynamicInstanceOp next = *this;
do {
Expand Down
7 changes: 3 additions & 4 deletions lib/Dialect/MSFT/Transforms/MSFTExportTcl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,10 @@ void ExportTclPass::runOnOperation() {
if (failed(applyPartialConversion(top, target, std::move(patterns))))
signalPassFailure();

target.addDynamicallyLegalOp<hw::GlobalRefOp>([&](hw::GlobalRefOp ref) {
return !emitter.getRefsUsed().contains(ref);
});
target.addDynamicallyLegalOp<hw::HierPathOp>(
[&](hw::HierPathOp ref) { return !emitter.getRefsUsed().contains(ref); });
patterns.clear();
patterns.insert<RemoveOpLowering<hw::GlobalRefOp>>(ctxt);
patterns.insert<RemoveOpLowering<hw::HierPathOp>>(ctxt);
if (failed(applyPartialConversion(top, target, std::move(patterns))))
signalPassFailure();
}
Expand Down
81 changes: 7 additions & 74 deletions lib/Dialect/MSFT/Transforms/MSFTLowerInstances.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,43 +37,19 @@ struct LowerInstancesPass : public LowerInstancesBase<LowerInstancesPass> {
LogicalResult lower(DynamicInstanceOp inst, InstanceHierarchyOp hier,
OpBuilder &b);

// Aggregation of the global ref attributes populated as a side-effect of the
// conversion.
DenseMap<Operation *, SmallVector<hw::GlobalRefAttr, 0>> globalRefsToApply;

// Cache the top-level symbols. Insert the new ones we're creating for new
// global ref ops.
// HierPathOps.
SymbolCache topSyms;

// In order to be efficient, cache the "symbols" in each module.
DenseMap<hw::HWModuleLike, SymbolCache> perModSyms;
// Accessor for `perModSyms` which lazily constructs each cache.
const SymbolCache &getSyms(hw::HWModuleLike mod);
};
} // anonymous namespace

const SymbolCache &LowerInstancesPass::getSyms(hw::HWModuleLike mod) {
auto symsFound = perModSyms.find(mod);
if (symsFound != perModSyms.end())
return symsFound->getSecond();

// Build the cache.
SymbolCache &syms = perModSyms[mod];
hw::InnerSymbolTable::walkSymbols(
mod, [&](StringAttr symName, hw::InnerSymTarget target) {
if (!target.isPort())
syms.addDefinition(symName, target.getOp());
});
return syms;
}

LogicalResult LowerInstancesPass::lower(DynamicInstanceOp inst,
InstanceHierarchyOp hier,
OpBuilder &b) {

hw::GlobalRefOp ref = nullptr;
hw::HierPathOp ref = nullptr;

// If 'inst' doesn't contain any ops which use a global ref op, don't create
// If 'inst' doesn't contain any ops which use a hierpath op, don't create
// one.
if (llvm::any_of(inst.getOps(), [](Operation &op) {
return isa<DynInstDataOpInterface>(op);
Expand All @@ -87,40 +63,12 @@ LogicalResult LowerInstancesPass::lower(DynamicInstanceOp inst,
refSym = StringAttr::get(&getContext(),
origRefSym.getValue() + "_" + Twine(++ctr));

// Create a global ref to replace us.
ArrayAttr globalRefPath = inst.globalRefPath();
ref = b.create<hw::GlobalRefOp>(inst.getLoc(), refSym, globalRefPath);
auto refAttr = hw::GlobalRefAttr::get(ref);
// Create a hierpath to replace us.
ArrayAttr hierPath = inst.getPath();
ref = b.create<hw::HierPathOp>(inst.getLoc(), refSym, hierPath);

// Add the new symbol to the symbol cache.
topSyms.addDefinition(refSym, ref);

// For each level of `globalRef`, find the static operation which needs a
// back reference to the global ref which is replacing us.
bool symNotFound = false;
for (auto innerRef : globalRefPath.getAsRange<hw::InnerRefAttr>()) {
auto mod =
cast<hw::HWModuleLike>(topSyms.getDefinition(innerRef.getModule()));
const SymbolCache &modSyms = getSyms(mod);
Operation *tgtOp = modSyms.getDefinition(innerRef.getName());
if (!tgtOp) {
symNotFound = true;
inst.emitOpError("Could not find ")
<< innerRef.getName() << " in module " << innerRef.getModule();
continue;
}
// Add the backref to the list of attributes to apply.
globalRefsToApply[tgtOp].push_back(refAttr);

// Since GlobalRefOp uses the `inner_sym` attribute, assign the
// 'inner_sym' attribute if it's not already assigned.
if (!tgtOp->hasAttr("inner_sym")) {
tgtOp->setAttr("inner_sym", hw::InnerSymAttr::get(innerRef.getName()));
}
}
if (symNotFound)
return inst.emitOpError(
"Could not find operation corresponding to instance reference");
}

// Relocate all my children.
Expand All @@ -134,7 +82,7 @@ LogicalResult LowerInstancesPass::lower(DynamicInstanceOp inst,
// Assign a ref for ops which need it.
if (auto specOp = dyn_cast<DynInstDataOpInterface>(op)) {
assert(ref);
specOp.setGlobalRef(ref);
specOp.setPathOp(ref);
}
}

Expand Down Expand Up @@ -166,21 +114,6 @@ void LowerInstancesPass::runOnOperation() {
}
if (numFailed)
signalPassFailure();

// Since applying a large number of attributes is very expensive in MLIR (both
// in terms of time and memory), bulk-apply the attributes necessary for
// `hw.globalref`s.
for (auto opRefPair : globalRefsToApply) {
ArrayRef<hw::GlobalRefAttr> refArr = opRefPair.getSecond();
SmallVector<Attribute> newGlobalRefs(
llvm::map_range(refArr, [](hw::GlobalRefAttr ref) { return ref; }));
Operation *op = opRefPair.getFirst();
if (auto refArr =
op->getAttrOfType<ArrayAttr>(hw::GlobalRefAttr::DialectAttrName))
newGlobalRefs.append(refArr.getValue().begin(), refArr.getValue().end());
op->setAttr(hw::GlobalRefAttr::DialectAttrName,
ArrayAttr::get(ctxt, newGlobalRefs));
}
}

namespace circt {
Expand Down
8 changes: 4 additions & 4 deletions test/Dialect/MSFT/dynamic_instance.mlir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ msft.instance.hierarchy @deeper {
}
}
}
// CHECK: hw.globalRef @instref [#hw.innerNameRef<@deeper::@branch>, #hw.innerNameRef<@shallow::@leaf>, #hw.innerNameRef<@leaf::@module>]
// CHECK: hw.hierpath @instref [@deeper::@branch, @shallow::@leaf, @leaf::@module]
// CHECK: msft.pd.location @instref M20K x: 15 y: 9 n: 3 path : "|memBank2"

msft.instance.hierarchy @shallow {
Expand All @@ -22,7 +22,7 @@ msft.instance.hierarchy @shallow {
}
}
}
// CHECK: hw.globalRef @instref_1 [#hw.innerNameRef<@shallow::@leaf>, #hw.innerNameRef<@leaf::@module>]
// CHECK: hw.hierpath @instref_1 [@shallow::@leaf, @leaf::@module]
// CHECK: msft.pd.location @instref_1 M20K x: 8 y: 19 n: 1 path : "|memBank2"

msft.instance.hierarchy @reg "foo" {
Expand All @@ -35,9 +35,9 @@ msft.instance.hierarchy @reg "bar" {
msft.pd.reg_location i4 [<3,4,5>, *, *, *]
}
}
// CHECK: hw.globalRef @instref_2 [#hw.innerNameRef<@reg::@reg>]
// CHECK: hw.hierpath @instref_2 [@reg::@reg]
// CHECK-DAG: msft.pd.reg_location ref @instref_2 i4 [*, <1, 2, 3>, <1, 2, 4>, <1, 2, 5>]
// CHECK: hw.globalRef @instref_3 [#hw.innerNameRef<@reg::@reg>]
// CHECK: hw.hierpath @instref_3 [@reg::@reg]
// CHECK-DAG: msft.pd.reg_location ref @instref_3 i4 [<3, 4, 5>, *, *, *]


Expand Down
Loading

1 comment on commit ff1f72d

@teqdruid
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Typo in the issue number. Should be #5182.

Please sign in to comment.