[MSFT] Add optional sub-path to PhysLocationAttr. (#2421)

Previously, a sub-path could be specified by encoding it in the name
of the attribute using a certain scheme. This is brittle and
non-standard. To make this more natural and robust, the sub-path is
added as a field of the attribute definition as a StringRef. It is not
required, in which case an empty StringRef can be used. The attribute
name for placement attributes is no longer used in any logic.
This commit is contained in:
mikeurbach 2022-01-05 18:39:54 -07:00 committed by GitHub
parent 811fa5fd28
commit c9c50d2889
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 67 additions and 65 deletions

View File

@ -17,7 +17,7 @@ def placement(subpath: Union[str, list[str]],
x: int,
y: int,
num: int = 0):
loc = PhysLocation(devtype, x, y, num)
if isinstance(subpath, list):
subpath = "|".join(subpath)
loc = PhysLocation(devtype, x, y, num, subpath)
return (f"loc:{subpath}", loc)

View File

@ -19,7 +19,8 @@ class PhysLocation:
prim_type: typing.Union[str, PrimitiveType],
x: int,
y: int,
num: typing.Union[int, None] = None):
num: typing.Union[int, None] = None,
sub_path: str = ""):
if isinstance(prim_type, str):
prim_type = getattr(PrimitiveType, prim_type)
@ -31,7 +32,7 @@ class PhysLocation:
assert isinstance(x, int)
assert isinstance(y, int)
assert isinstance(num, int)
self._loc = msft.PhysLocationAttr.get(prim_type, x, y, num)
self._loc = msft.PhysLocationAttr.get(prim_type, x, y, num, sub_path)
def __str__(self) -> str:
loc = self._loc

View File

@ -119,14 +119,12 @@ class Instance:
callback(inst)
inst.walk(callback)
def _attach_attribute(self, attr_key: str, attr: ir.Attribute):
def _attach_attribute(self, sub_path: str, attr: ir.Attribute):
if isinstance(attr, PhysLocation):
assert attr_key.startswith("loc:")
attr = attr._loc
db = self.root_instance.placedb._db
rc = db.add_placement(attr, self.path_attr, attr_key[4:],
self.instOp.operation)
rc = db.add_placement(attr, self.path_attr, sub_path, self.instOp.operation)
if not rc:
raise ValueError("Failed to place")
@ -137,7 +135,7 @@ class Instance:
global_ref = hw.GlobalRefOp(global_ref_symbol, path_attr)
# Attach the attribute to the global ref.
global_ref.attributes["loc:" + attr_key[4:]] = attr
global_ref.attributes["loc:" + sub_path] = attr
# Add references to the global ref for each instance through the hierarchy.
for instance in self.path:
@ -165,7 +163,7 @@ class Instance:
x: int,
y: int,
num: int = 0):
loc = msft.PhysLocationAttr.get(devtype, x, y, num)
if isinstance(subpath, list):
subpath = "|".join(subpath)
self._attach_attribute(f"loc:{subpath}", loc)
loc = msft.PhysLocationAttr.get(devtype, x, y, num, subpath)
self._attach_attribute(subpath, loc)

View File

@ -40,8 +40,9 @@ MLIR_CAPI_EXPORTED void mlirMSFTAddPhysLocationAttr(MlirOperation op,
MLIR_CAPI_EXPORTED bool
circtMSFTAttributeIsAPhysLocationAttribute(MlirAttribute);
MLIR_CAPI_EXPORTED MlirAttribute circtMSFTPhysLocationAttrGet(
MlirContext, CirctMSFTPrimitiveType, uint64_t x, uint64_t y, uint64_t num);
MLIR_CAPI_EXPORTED MlirAttribute
circtMSFTPhysLocationAttrGet(MlirContext, CirctMSFTPrimitiveType, uint64_t x,
uint64_t y, uint64_t num, MlirStringRef subPath);
MLIR_CAPI_EXPORTED CirctMSFTPrimitiveType
circtMSFTPhysLocationAttrGetPrimitiveType(MlirAttribute);
MLIR_CAPI_EXPORTED uint64_t circtMSFTPhysLocationAttrGetX(MlirAttribute);

View File

@ -37,7 +37,8 @@ def PhysLocation : MSFT_Attr<"PhysLocation"> {
let mnemonic = "physloc";
let parameters = (ins
"PrimitiveTypeAttr":$primitiveType,
"uint64_t":$x, "uint64_t":$y, "uint64_t":$num);
"uint64_t":$x, "uint64_t":$y, "uint64_t":$num,
StringRefParameter<"subPath">:$subPath);
}
def PhysicalBounds : MSFT_Attr<"PhysicalBounds"> {

View File

@ -43,8 +43,12 @@ with ir.Context() as ctx, ir.Location.unknown():
path = op.create("inst1")
minst = msft_mod.create("minst")
# CHECK: #msft.physloc<M20K, 2, 6, 1>
physAttr = msft.PhysLocationAttr.get(msft.M20K, x=2, y=6, num=1)
# CHECK: #msft.physloc<M20K, 2, 6, 1, "foo_subpath">
physAttr = msft.PhysLocationAttr.get(msft.M20K,
x=2,
y=6,
num=1,
sub_path="foo_subpath")
print(physAttr)
# CHECK: #msft.physloc<FF, 0, 0, 0>
@ -85,7 +89,7 @@ with ir.Context() as ctx, ir.Location.unknown():
place_rc = db.add_placement(physAttr, path, "foo_subpath", resolved_inst)
assert not place_rc
# ERR: error: 'msft.instance' op Could not apply placement #msft.physloc<M20K, 2, 6, 1>. Position already occupied by msft.instance @ext1 @MyExternMod
# ERR: error: 'msft.instance' op Could not apply placement #msft.physloc<M20K, 2, 6, 1, "foo_subpath">. Position already occupied by msft.instance @ext1 @MyExternMod
devdb = msft.PrimitiveDB()
assert not devdb.is_valid_location(physAttr)
@ -124,13 +128,13 @@ with ir.Context() as ctx, ir.Location.unknown():
print("=== Placements:")
seeded_pdb.walk_placements(print_placement)
# CHECK-LABEL: === Placements:
# CHECK: #msft.physloc<M20K, 2, 6, 1>, [#hw.innerNameRef<@top::@inst1>, #hw.innerNameRef<@MyWidget::@ext1>]
# CHECK: #msft.physloc<M20K, 2, 6, 1, "foo_subpath">, [#hw.innerNameRef<@top::@inst1>, #hw.innerNameRef<@MyWidget::@ext1>]
# CHECK: #msft.physloc<M20K, 2, 50, 1>
print("=== Placements (col 2):")
seeded_pdb.walk_placements(print_placement, bounds=(2, 2, None, None))
# CHECK-LABEL: === Placements (col 2):
# CHECK: #msft.physloc<M20K, 2, 6, 1>, [#hw.innerNameRef<@top::@inst1>, #hw.innerNameRef<@MyWidget::@ext1>]
# CHECK: #msft.physloc<M20K, 2, 6, 1, "foo_subpath">, [#hw.innerNameRef<@top::@inst1>, #hw.innerNameRef<@MyWidget::@ext1>]
# CHECK: #msft.physloc<M20K, 2, 50, 1>
print("=== Placements (col 2, row > 10):")

View File

@ -133,13 +133,14 @@ void circt::python::populateDialectMSFTSubmodule(py::module &m) {
.def_classmethod(
"get",
[](py::object cls, PrimitiveType devType, uint64_t x, uint64_t y,
uint64_t num, MlirContext ctxt) {
uint64_t num, std::string subPath, MlirContext ctxt) {
auto cSubPath = mlirStringRefCreateFromCString(subPath.c_str());
return cls(circtMSFTPhysLocationAttrGet(ctxt, (uint64_t)devType, x,
y, num));
y, num, cSubPath));
},
"Create a physical location attribute", py::arg(),
py::arg("dev_type"), py::arg("x"), py::arg("y"), py::arg("num"),
py::arg("ctxt") = py::none())
py::arg("sub_path") = py::str(""), py::arg("ctxt") = py::none())
.def_property_readonly(
"devtype",
[](MlirAttribute self) {

View File

@ -134,7 +134,7 @@ void mlirMSFTAddPhysLocationAttr(MlirOperation cOp, const char *entityName,
mlir::Operation *op = unwrap(cOp);
mlir::MLIRContext *ctxt = op->getContext();
PhysLocationAttr loc = PhysLocationAttr::get(
ctxt, PrimitiveTypeAttr::get(ctxt, type), x, y, num);
ctxt, PrimitiveTypeAttr::get(ctxt, type), x, y, num, entityName);
llvm::SmallString<64> entity("loc:");
entity.append(entityName);
op->setAttr(entity, loc);
@ -145,11 +145,12 @@ bool circtMSFTAttributeIsAPhysLocationAttribute(MlirAttribute attr) {
}
MlirAttribute circtMSFTPhysLocationAttrGet(MlirContext cCtxt,
CirctMSFTPrimitiveType devType,
uint64_t x, uint64_t y,
uint64_t num) {
auto ctxt = unwrap(cCtxt);
uint64_t x, uint64_t y, uint64_t num,
MlirStringRef subPath) {
auto *ctxt = unwrap(cCtxt);
return wrap(PhysLocationAttr::get(
ctxt, PrimitiveTypeAttr::get(ctxt, (PrimitiveType)devType), x, y, num));
ctxt, PrimitiveTypeAttr::get(ctxt, (PrimitiveType)devType), x, y, num,
unwrap(subPath)));
}
CirctMSFTPrimitiveType

View File

@ -56,7 +56,7 @@ void PrimitiveDB::foreach (
for (auto n : y.second)
for (auto p : n.second)
callback(PhysLocationAttr::get(ctxt, PrimitiveTypeAttr::get(ctxt, p),
x.first, y.first, n.first));
x.first, y.first, n.first, ""));
}
//===----------------------------------------------------------------------===//
@ -126,22 +126,14 @@ size_t PlacementDB::addPlacements(FlatSymbolRefAttr rootMod,
size_t numFailed = 0;
for (NamedAttribute attr : op->getAttrs()) {
StringRef attrName = attr.getName();
llvm::TypeSwitch<Attribute>(attr.getValue())
// Handle PhysLocationAttr.
.Case([&](PhysLocationAttr physLoc) {
// PhysLoc has a subpath, which comes from the attribute name.
// TODO(mikeurbach): make this an inherent attribute of the PhysLoc.
if (!attrName.startswith("loc:")) {
op->emitOpError(
"PhysLoc attributes must have names starting with 'loc'");
++numFailed;
return;
}
LogicalResult added = addPlacement(
physLoc, PlacedInstance{instPath, attrName.substr(4), op});
// PhysLoc has a subpath.
auto subPath = physLoc.getSubPath();
LogicalResult added =
addPlacement(physLoc, PlacedInstance{instPath, subPath, op});
if (failed(added))
++numFailed;
})
@ -265,8 +257,9 @@ void PlacementDB::walkPlacements(
PlacedInstance inst = devF->getSecond();
// Marshall and run the callback.
PhysLocationAttr loc = PhysLocationAttr::get(
ctxt, PrimitiveTypeAttr::get(ctxt, devtype), x, y, num);
PhysLocationAttr loc =
PhysLocationAttr::get(ctxt, PrimitiveTypeAttr::get(ctxt, devtype),
x, y, num, inst.subpath);
callback(loc, inst);
}
}

View File

@ -25,11 +25,21 @@ using namespace msft;
Attribute PhysLocationAttr::parse(AsmParser &p, Type type) {
llvm::SMLoc loc = p.getCurrentLocation();
std::string subPath;
StringRef devTypeStr;
uint64_t x, y, num;
if (p.parseLess() || p.parseKeyword(&devTypeStr) || p.parseComma() ||
p.parseInteger(x) || p.parseComma() || p.parseInteger(y) ||
p.parseComma() || p.parseInteger(num) || p.parseGreater())
p.parseComma() || p.parseInteger(num))
return Attribute();
// Parse an optional subPath.
if (succeeded(p.parseOptionalComma()))
if (p.parseString(&subPath))
return Attribute();
if (p.parseGreater())
return Attribute();
Optional<PrimitiveType> devType = symbolizePrimitiveType(devTypeStr);
@ -39,13 +49,20 @@ Attribute PhysLocationAttr::parse(AsmParser &p, Type type) {
}
PrimitiveTypeAttr devTypeAttr =
PrimitiveTypeAttr::get(p.getContext(), *devType);
auto phy = PhysLocationAttr::get(p.getContext(), devTypeAttr, x, y, num);
auto phy =
PhysLocationAttr::get(p.getContext(), devTypeAttr, x, y, num, subPath);
return phy;
}
void PhysLocationAttr::print(AsmPrinter &p) const {
p << "<" << stringifyPrimitiveType(getPrimitiveType().getValue()) << ", "
<< getX() << ", " << getY() << ", " << getNum() << '>';
<< getX() << ", " << getY() << ", " << getNum();
// Print an optional subPath.
if (!getSubPath().empty())
p << ", \"" << getSubPath() << '"';
p << '>';
}
Attribute PhysicalRegionRefAttr::parse(AsmParser &p, Type type) {

View File

@ -195,7 +195,7 @@ public:
matchAndRewrite(hw::GlobalRefOp op, OpAdaptor adaptor,
ConversionPatternRewriter &rewriter) const final {
for (auto attr : op->getAttrs()) {
if (attr.getName().getValue().startswith("loc")) {
if (isa<MSFTDialect>(attr.getValue().getDialect())) {
rewriter.eraseOp(op);
return success();
}
@ -237,7 +237,7 @@ void LowerToHWPass::runOnOperation() {
target.addLegalDialect<sv::SVDialect>();
target.addDynamicallyLegalOp<hw::GlobalRefOp>([](hw::GlobalRefOp op) {
for (auto attr : op->getAttrs())
if (attr.getName().getValue().startswith("loc"))
if (isa<MSFTDialect>(attr.getValue().getDialect()))
return false;
return true;
});

View File

@ -3,19 +3,19 @@
// RUN: circt-opt %s --lower-msft-to-hw=tops=shallow,deeper,regions,reg --lower-seq-to-sv --export-verilog | FileCheck %s --check-prefix=TCL
hw.globalRef @ref1 [#hw.innerNameRef<@deeper::@branch>, #hw.innerNameRef<@shallow::@leaf>, #hw.innerNameRef<@leaf::@module>] {
"loc:memBank2" = #msft.physloc<M20K, 15, 9, 3>
"foo" = #msft.physloc<M20K, 15, 9, 3, "memBank2">
}
hw.globalRef @ref2 [#hw.innerNameRef<@shallow::@leaf>, #hw.innerNameRef<@leaf::@module>] {
"loc:memBank2" = #msft.physloc<M20K, 8, 19, 1>
"bar" = #msft.physloc<M20K, 8, 19, 1, "memBank2">
}
hw.globalRef @ref3 [#hw.innerNameRef<@regions::@module>] {
"loc:" = #msft.physical_region_ref<@region1>
"baz" = #msft.physical_region_ref<@region1>
}
hw.globalRef @ref4 [#hw.innerNameRef<@reg::@reg>] {
"loc:" = #msft.physloc<FF, 0, 0, 0>
"qux" = #msft.physloc<FF, 0, 0, 0>
}
hw.module.extern @Foo()

View File

@ -12,18 +12,3 @@ msft.module @top {} () -> () {
msft.instance @foo1 @Foo() {circt.globalRef = [#hw.globalNameRef<@ref>], inner_sym = "foo1"} : () -> ()
msft.output
}
// -----
hw.module.extern @Foo()
// expected-error @+1 {{'hw.globalRef' op PhysLoc attributes must have names starting with 'loc'}}
hw.globalRef @ref [#hw.innerNameRef<@top::@foo1>] {
"phys:" = #msft.physloc<DSP, 0, 0, 0>
}
// expected-error @+1 {{Could not place 1 instances}}
msft.module @top {} () -> () {
msft.instance @foo1 @Foo() {circt.globalRef = [#hw.globalNameRef<@ref>], inner_sym = "foo1"} : () -> ()
msft.output
}