Skip to content

[FIRRTL] Remove internalPaths from IR. Drop ref statements. #9154

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.

Sign up for GitHub

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

Merged
dtzSiFive merged 1 commit into from
Oct 29, 2025
Merged

[FIRRTL] Remove internalPaths from IR. Drop ref statements. #9154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 0 additions & 22 deletions include/circt/Dialect/FIRRTL/FIRRTLAttributes.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,28 +182,6 @@ def MemoryInitAttr : AttrDef<FIRRTLDialect, "MemoryInit"> {
let assemblyFormat = "`<` $filename `,` $isBinary `,` $isInline `>`";
}

/// An attribute holding internal path for ref-type ports.
def InternalPathAttr : AttrDef<FIRRTLDialect, "InternalPath"> {
let summary = "Internal path for ref-type ports";
let mnemonic = "internalpath";
let parameters = (ins OptionalParameter<"::mlir::StringAttr">:$path);
let assemblyFormat = "(`<` $path^ `>`)?";
let builders = [
AttrBuilderWithInferredContext<(ins "::mlir::StringAttr":$path), [{
return $_get(path.getContext(), path);
}]>,
AttrBuilder<(ins "::llvm::StringRef":$path), [{
return $_get($_ctxt, ::mlir::StringAttr::get($_ctxt, path));
}]>,
AttrBuilder<(ins), [{
return $_get($_ctxt, ::mlir::StringAttr());
}]>
];
}

def InternalPathArrayAttr
: TypedArrayAttrBase<InternalPathAttr, "InternalPath array attribute">;

//===----------------------------------------------------------------------===//
// Miscellaneous attributes
//===----------------------------------------------------------------------===//
Expand Down
9 changes: 0 additions & 9 deletions include/circt/Dialect/FIRRTL/FIRRTLStructure.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,6 @@ def FExtModuleOp : FIRRTLModuleLike<"extmodule"> {
let description = [{
The "firrtl.extmodule" operation represents an external reference to a
Verilog module, including a given name and a list of ports.
LowerAnnotations can add RefType ports to it. Each RefType port
must have a corresponding entry in the internalPaths attribute.
The internalPaths attribute is used to represent opaque internal paths
into the external module, to be used for generating XMRs. Each RefType
port must be removed by LowerXMR pass.
}];
let arguments = (ins SymbolNameAttr:$sym_name, OptionalAttr<StrAttr>:$defname,
ParamDeclArrayAttr:$parameters, ConventionAttr:$convention,
Expand All @@ -206,7 +201,6 @@ def FExtModuleOp : FIRRTLModuleLike<"extmodule"> {
LayerArrayAttr:$knownLayers,
DefaultValuedAttr<AnnotationArrayAttr, "{}">:$annotations,
DefaultValuedAttr<LayerArrayAttr, "{}">:$layers,
OptionalAttr<InternalPathArrayAttr>:$internalPaths,
DefaultValuedAttr<ArrayRefAttr, "{}">:$domainInfo);
let results = (outs);
let regions = (region AnyRegion:$body);
Expand All @@ -217,7 +211,6 @@ def FExtModuleOp : FIRRTLModuleLike<"extmodule"> {
"ArrayAttr":$knownLayers, CArg<"StringRef", "StringRef()">:$defnamAttr,
CArg<"ArrayAttr", "ArrayAttr()">:$annotations,
CArg<"ArrayAttr", "ArrayAttr()">:$parameters,
CArg<"ArrayAttr", "ArrayAttr()">:$internalPaths,
CArg<"ArrayAttr", "ArrayAttr()">:$layers)>];

let extraModuleClassDeclaration = [{
Expand Down Expand Up @@ -246,7 +239,6 @@ def FIntModuleOp : FIRRTLModuleLike<"intmodule"> {
ArrayRefAttr:$portTypes,
DefaultValuedAttr<AnnotationArrayAttr, "ArrayAttr()">:$annotations,
DefaultValuedAttr<LayerArrayAttr, "{}">:$layers,
OptionalAttr<InternalPathArrayAttr>:$internalPaths,
DefaultValuedAttr<ArrayRefAttr, "{}">:$domainInfo
);
let results = (outs);
Expand All @@ -259,7 +251,6 @@ def FIntModuleOp : FIRRTLModuleLike<"intmodule"> {
"StringRef":$intrinsicNameStr,
CArg<"ArrayAttr", "ArrayAttr()">:$annotations,
CArg<"ArrayAttr", "ArrayAttr()">:$parameters,
CArg<"ArrayAttr", "ArrayAttr()">:$internalPaths,
CArg<"ArrayAttr", "ArrayAttr()">:$layers)>
];

Expand Down
101 changes: 6 additions & 95 deletions lib/Dialect/FIRRTL/FIRRTLOps.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -779,8 +779,7 @@ static Attribute fixDomainInfoInsertions(MLIRContext *context,
/// Insertion occurs in-order, such that ports with the same insertion index
/// appear in the module in the same order they appeared in the list.
static void insertPorts(FModuleLike op,
ArrayRef<std::pair<unsigned, PortInfo>> ports,
bool supportsInternalPaths = false) {
ArrayRef<std::pair<unsigned, PortInfo>> ports) {
if (ports.empty())
return;
unsigned oldNumArgs = op.getNumPorts();
Expand All @@ -795,27 +794,17 @@ static void insertPorts(FModuleLike op,
assert(existingNames.size() == oldNumArgs);
assert(existingTypes.size() == oldNumArgs);
assert(existingLocs.size() == oldNumArgs);
SmallVector<Attribute> internalPaths;
auto emptyInternalPath = InternalPathAttr::get(op.getContext());
if (supportsInternalPaths) {
if (auto internalPathsAttr = op->getAttrOfType<ArrayAttr>("internalPaths"))
llvm::append_range(internalPaths, internalPathsAttr);
else
internalPaths.resize(oldNumArgs, emptyInternalPath);
assert(internalPaths.size() == oldNumArgs);
}

SmallVector<bool> newDirections;
SmallVector<Attribute> newNames, newTypes, newDomains, newAnnos, newSyms,
newLocs, newInternalPaths;
newLocs;
newDirections.reserve(newNumArgs);
newNames.reserve(newNumArgs);
newTypes.reserve(newNumArgs);
newDomains.reserve(newNumArgs);
newAnnos.reserve(newNumArgs);
newSyms.reserve(newNumArgs);
newLocs.reserve(newNumArgs);
newInternalPaths.reserve(newNumArgs);

SmallVector<unsigned> indexMap(oldNumArgs);

Expand All @@ -833,8 +822,6 @@ static void insertPorts(FModuleLike op,
newAnnos.push_back(op.getAnnotationsAttrForPort(oldIdx));
newSyms.push_back(op.getPortSymbolAttr(oldIdx));
newLocs.push_back(existingLocs[oldIdx]);
if (supportsInternalPaths)
newInternalPaths.push_back(internalPaths[oldIdx]);

indexMap[oldIdx] = oldIdx + inserted;
++oldIdx;
Expand All @@ -854,8 +841,6 @@ static void insertPorts(FModuleLike op,
newAnnos.push_back(annos ? annos : emptyArray);
newSyms.push_back(port.sym);
newLocs.push_back(port.loc);
if (supportsInternalPaths)
newInternalPaths.push_back(emptyInternalPath);
++inserted;
}
migrateOldPorts(oldNumArgs);
Expand Down Expand Up @@ -888,17 +873,6 @@ static void insertPorts(FModuleLike op,
FModuleLike::fixupPortSymsArray(newSyms, op.getContext());
op.setPortSymbols(newSyms);
op->setAttr("portLocations", ArrayAttr::get(op.getContext(), newLocs));
if (supportsInternalPaths) {
// Drop if all-empty, otherwise set to new array.
auto empty = llvm::all_of(newInternalPaths, [](Attribute attr) {
return !cast<InternalPathAttr>(attr).getPath();
});
if (empty)
op->removeAttr("internalPaths");
else
op->setAttr("internalPaths",
ArrayAttr::get(op.getContext(), newInternalPaths));
}
}

// Return an Attribute with updated port domain information based on information
Expand Down Expand Up @@ -1031,34 +1005,12 @@ static void erasePorts(FModuleLike op, const llvm::BitVector &portIndices) {
portIndices, /*supportsEmptyAttr=*/true));
}

template <typename T>
static void eraseInternalPaths(T op, const llvm::BitVector &portIndices) {
// Fixup internalPaths array.
auto internalPaths = op.getInternalPaths();
if (!internalPaths)
return;

auto newPaths =
removeElementsAtIndices(internalPaths->getValue(), portIndices);

// Drop if all-empty, otherwise set to new array.
auto empty = llvm::all_of(newPaths, [](Attribute attr) {
return !cast<InternalPathAttr>(attr).getPath();
});
if (empty)
op.removeInternalPathsAttr();
else
op.setInternalPathsAttr(ArrayAttr::get(op.getContext(), newPaths));
}

void FExtModuleOp::erasePorts(const llvm::BitVector &portIndices) {
::erasePorts(cast<FModuleLike>((Operation *)*this), portIndices);
eraseInternalPaths(*this, portIndices);
}

void FIntModuleOp::erasePorts(const llvm::BitVector &portIndices) {
::erasePorts(cast<FModuleLike>((Operation *)*this), portIndices);
eraseInternalPaths(*this, portIndices);
}

void FMemModuleOp::erasePorts(const llvm::BitVector &portIndices) {
Expand Down Expand Up @@ -1087,13 +1039,11 @@ void FModuleOp::insertPorts(ArrayRef<std::pair<unsigned, PortInfo>> ports) {
}

void FExtModuleOp::insertPorts(ArrayRef<std::pair<unsigned, PortInfo>> ports) {
::insertPorts(cast<FModuleLike>((Operation *)*this), ports,
/*supportsInternalPaths=*/true);
::insertPorts(cast<FModuleLike>((Operation *)*this), ports);
}

void FIntModuleOp::insertPorts(ArrayRef<std::pair<unsigned, PortInfo>> ports) {
::insertPorts(cast<FModuleLike>((Operation *)*this), ports,
/*supportsInternalPaths=*/true);
::insertPorts(cast<FModuleLike>((Operation *)*this), ports);
}

/// Inserts the given ports. The insertion indices are expected to be in order.
Expand Down Expand Up @@ -1204,8 +1154,7 @@ void FExtModuleOp::build(OpBuilder &builder, OperationState &result,
StringAttr name, ConventionAttr convention,
ArrayRef<PortInfo> ports, ArrayAttr knownLayers,
StringRef defnameAttr, ArrayAttr annotations,
ArrayAttr parameters, ArrayAttr internalPaths,
ArrayAttr layers) {
ArrayAttr parameters, ArrayAttr layers) {
buildModule<FExtModuleOp>(builder, result, name, ports, annotations, layers);
auto &properties = result.getOrAddProperties<Properties>();
properties.setConvention(convention);
Expand All @@ -1217,23 +1166,18 @@ void FExtModuleOp::build(OpBuilder &builder, OperationState &result,
if (!parameters)
parameters = builder.getArrayAttr({});
properties.setParameters(parameters);
if (internalPaths && !internalPaths.empty())
properties.setInternalPaths(internalPaths);
}

void FIntModuleOp::build(OpBuilder &builder, OperationState &result,
StringAttr name, ArrayRef<PortInfo> ports,
StringRef intrinsicNameStr, ArrayAttr annotations,
ArrayAttr parameters, ArrayAttr internalPaths,
ArrayAttr layers) {
ArrayAttr parameters, ArrayAttr layers) {
buildModule<FIntModuleOp>(builder, result, name, ports, annotations, layers);
auto &properties = result.getOrAddProperties<Properties>();
properties.setIntrinsic(builder.getStringAttr(intrinsicNameStr));
if (!parameters)
parameters = builder.getArrayAttr({});
properties.setParameters(parameters);
if (internalPaths && !internalPaths.empty())
properties.setInternalPaths(internalPaths);
}

void FMemModuleOp::build(OpBuilder &builder, OperationState &result,
Expand Down Expand Up @@ -1819,37 +1763,7 @@ LogicalResult FModuleOp::verify() {
return success();
}

static LogicalResult
verifyInternalPaths(FModuleLike op,
std::optional<::mlir::ArrayAttr> internalPaths) {
if (!internalPaths)
return success();

// If internal paths are present, should cover all ports.
if (internalPaths->size() != op.getNumPorts())
return op.emitError("module has inconsistent internal path array with ")
<< internalPaths->size() << " entries for " << op.getNumPorts()
<< " ports";

// No internal paths for non-ref-type ports.
for (auto [idx, path, typeattr] : llvm::enumerate(
internalPaths->getAsRange<InternalPathAttr>(), op.getPortTypes())) {
if (path.getPath() &&
!type_isa<RefType>(cast<TypeAttr>(typeattr).getValue())) {
auto diag =
op.emitError("module has internal path for non-ref-type port ")
<< op.getPortNameAttr(idx);
return diag.attachNote(op.getPortLocation(idx)) << "this port";
}
}

return success();
}

LogicalResult FExtModuleOp::verify() {
if (failed(verifyInternalPaths(*this, getInternalPaths())))
return failure();

auto params = getParameters();

auto checkParmValue = [&](Attribute elt) -> bool {
Expand Down Expand Up @@ -1883,9 +1797,6 @@ LogicalResult FExtModuleOp::verify() {
}

LogicalResult FIntModuleOp::verify() {
if (failed(verifyInternalPaths(*this, getInternalPaths())))
return failure();

auto params = getParameters();
if (params.empty())
return success();
Expand Down
Loading