Skip to content

Commit

Permalink
[HW] Adopt prefixed accessors (#3533)
Browse files Browse the repository at this point in the history 
We cannot directly switch to prefixed only because of the use of duck-typing in HWOpInterfaces.td and SVOps.cpp. This means we need to generate both the prefixed and non-prefixed accessors until all dialects that make use of HWInstanceLike have switched. This includes HW, SV, FIRRTL, MSFT.
  • Loading branch information
@maerhart
maerhart committed Jul 14, 2022
1 parent 26226cb commit 3ab2c5a
Show file tree
Hide file tree
Showing 24 changed files with 229 additions and 225 deletions.
1 change: 1 addition & 0 deletions include/circt/Dialect/HW/HWDialect.td
View file
Expand Up @@ -25,6 +25,7 @@ def HWDialect : Dialect {

let hasConstantMaterializer = 1;
let useDefaultTypePrinterParser = 1;
int emitAccessorPrefix = kEmitAccessorPrefix_Both;

let extraClassDeclaration = [{
/// Register all HW types.
Expand Down
5 changes: 0 additions & 5 deletions include/circt/Dialect/HW/HWMiscOps.td
View file
Expand Up @@ -40,11 +40,6 @@ def ConstantOp
/// Build a ConstantOp from a prebuilt attribute.
OpBuilder<(ins "IntegerAttr":$value)>
];
let extraClassDeclaration = [{
APInt getValue() {
return (*this)->getAttrOfType<IntegerAttr>("value").getValue();
}
}];
let hasFolder = true;
let hasVerifier = 1;
}
Expand Down
10 changes: 5 additions & 5 deletions include/circt/Dialect/HW/HWStructure.td
View file
Expand Up @@ -115,7 +115,7 @@ def HWModuleOp : HWModuleOpBase<"module",

// TODO(mlir): FunctionLike shouldn't produce a getBody() helper, it is
// squatting on the name.
Block *getBodyBlock() { return &body().front(); }
Block *getBodyBlock() { return &getBody().front(); }

// Get the module's symbolic name as StringAttr.
StringAttr getNameAttr() {
Expand Down Expand Up @@ -430,12 +430,12 @@ def InstanceOp : HWOp<"instance", [

/// Change the names of all the input ports.
void setArgumentNames(ArrayAttr names) {
argNamesAttr(names);
setArgNamesAttr(names);
}

/// Change the names of all the result ports.
void setResultNames(ArrayAttr names) {
resultNamesAttr(names);
setResultNamesAttr(names);
}

/// Lookup the module or extmodule for the symbol. This returns null on
Expand All @@ -444,12 +444,12 @@ def InstanceOp : HWOp<"instance", [

/// Get the instances's name.
StringAttr getName() {
return instanceNameAttr();
return getInstanceNameAttr();
}

/// Set the instance's name.
void setName(StringAttr name) {
instanceNameAttr(name);
setInstanceNameAttr(name);
}

//===------------------------------------------------------------------===//
Expand Down
2 changes: 1 addition & 1 deletion include/circt/Dialect/HW/HWTypeDecls.td
View file
Expand Up @@ -30,7 +30,7 @@ def TypeScopeOp : HWOp<"type_scope",
let assemblyFormat = "$sym_name $body attr-dict";

let extraClassDeclaration = [{
Block *getBodyBlock() { return &body().front(); }
Block *getBodyBlock() { return &getBody().front(); }
}];
}

Expand Down
4 changes: 2 additions & 2 deletions include/circt/Dialect/MSFT/MSFTOpInterfaces.td
View file
Expand Up @@ -62,10 +62,10 @@ def DynInstDataOpInterface : OpInterface<"DynInstDataOpInterface"> {
$_op->emitOpError("could not find hw.globalRef ") << refSym;
return nullptr;
}
if (ref.namepath().empty())
if (ref.getNamepath().empty())
return nullptr;
auto modSym = FlatSymbolRefAttr::get(
ref.namepath()[0].cast<hw::InnerRefAttr>().getModule());
ref.getNamepath()[0].cast<hw::InnerRefAttr>().getModule());
return symCache.getDefinition(modSym);
}]
>
Expand Down
56 changes: 28 additions & 28 deletions lib/Conversion/ExportVerilog/ExportVerilog.cpp
View file
Expand Up @@ -152,7 +152,7 @@ static bool isDuplicatableExpression(Operation *op) {

// We only inline array_get with a constant index.
if (auto array = dyn_cast<hw::ArrayGetOp>(op))
return array.index().getDefiningOp<ConstantOp>();
return array.getIndex().getDefiningOp<ConstantOp>();

return false;
}
Expand Down Expand Up @@ -567,7 +567,7 @@ static bool isOkToBitSelectFrom(Value v) {
/// expressions in the sensitivity list of always blocks, etc.
static bool isExpressionUnableToInline(Operation *op) {
if (auto cast = dyn_cast<BitcastOp>(op))
if (!haveMatchingDims(cast.input().getType(), cast.result().getType(),
if (!haveMatchingDims(cast.getInput().getType(), cast.getResult().getType(),
op->getLoc()))
// Bitcasts rely on the type being assigned to, so we cannot inline.
return true;
Expand Down Expand Up @@ -1312,7 +1312,7 @@ static bool printPackedTypeImpl(Type type, raw_ostream &os, Location loc,
mlir::emitError(loc, "unresolvable type reference");
return false;
}
if (typedecl.type() != typeRef.getInnerType()) {
if (typedecl.getType() != typeRef.getInnerType()) {
mlir::emitError(loc, "declared type did not match aliased type");
return false;
}
Expand Down Expand Up @@ -2058,12 +2058,12 @@ SubExprInfo ExprEmitter::visitTypeOp(BitcastOp op) {
// their dimensions don't match. SystemVerilog uses the wire declaration to
// know what type this value is being casted to.
Type toType = op.getType();
if (!haveMatchingDims(toType, op.input().getType(), op.getLoc())) {
if (!haveMatchingDims(toType, op.getInput().getType(), op.getLoc())) {
os << "/*cast(bit";
emitter.emitTypeDims(toType, op.getLoc(), os);
os << ")*/";
}
return emitSubExpr(op.input(), LowestPrecedence);
return emitSubExpr(op.getInput(), LowestPrecedence);
}

SubExprInfo ExprEmitter::visitComb(ICmpOp op) {
Expand Down Expand Up @@ -2223,7 +2223,7 @@ SubExprInfo ExprEmitter::visitTypeOp(ParamValueOp op) {
if (hasSVAttributes(op))
emitError(op, "SV attributes emission is unimplemented for the op");

return emitter.printParamValue(op.value(), os, [&]() {
return emitter.printParamValue(op.getValue(), os, [&]() {
return op->emitOpError("invalid parameter use");
});
}
Expand All @@ -2234,19 +2234,19 @@ SubExprInfo ExprEmitter::visitTypeOp(ArraySliceOp op) {
if (hasSVAttributes(op))
emitError(op, "SV attributes emission is unimplemented for the op");

auto arrayPrec = emitSubExpr(op.input(), Selection);
auto arrayPrec = emitSubExpr(op.getInput(), Selection);

unsigned dstWidth = type_cast<ArrayType>(op.getType()).getSize();
os << '[';
emitSubExpr(op.lowIndex(), LowestPrecedence);
emitSubExpr(op.getLowIndex(), LowestPrecedence);
os << " +: " << dstWidth << ']';
return {Selection, arrayPrec.signedness};
}

SubExprInfo ExprEmitter::visitTypeOp(ArrayGetOp op) {
emitSubExpr(op.input(), Selection);
emitSubExpr(op.getInput(), Selection);
os << '[';
emitSubExpr(op.index(), LowestPrecedence);
emitSubExpr(op.getIndex(), LowestPrecedence);
os << ']';
emitSVAttributes(op);
return {Selection, IsUnsigned};
Expand All @@ -2258,7 +2258,7 @@ SubExprInfo ExprEmitter::visitTypeOp(ArrayCreateOp op) {
emitError(op, "SV attributes emission is unimplemented for the op");

os << '{';
llvm::interleaveComma(op.inputs(), os, [&](Value operand) {
llvm::interleaveComma(op.getInputs(), os, [&](Value operand) {
os << "{";
emitSubExpr(operand, LowestPrecedence);
os << "}";
Expand Down Expand Up @@ -2378,8 +2378,8 @@ SubExprInfo ExprEmitter::visitTypeOp(StructExtractOp op) {
if (hasSVAttributes(op))
emitError(op, "SV attributes emission is unimplemented for the op");

emitSubExpr(op.input(), Selection);
os << '.' << emitter.getVerilogStructFieldName(op.fieldAttr());
emitSubExpr(op.getInput(), Selection);
os << '.' << emitter.getVerilogStructFieldName(op.getFieldAttr());
return {Selection, IsUnsigned};
}

Expand All @@ -2392,10 +2392,10 @@ SubExprInfo ExprEmitter::visitTypeOp(StructInjectOp op) {
llvm::interleaveComma(
stype.getElements(), os, [&](const StructType::FieldInfo &field) {
os << emitter.getVerilogStructFieldName(field.name) << ": ";
if (field.name == op.field()) {
emitSubExpr(op.newValue(), Selection);
if (field.name == op.getField()) {
emitSubExpr(op.getNewValue(), Selection);
} else {
emitSubExpr(op.input(), Selection);
emitSubExpr(op.getInput(), Selection);
os << '.' << field.name.getValue();
}
});
Expand All @@ -2404,7 +2404,7 @@ SubExprInfo ExprEmitter::visitTypeOp(StructInjectOp op) {
}

SubExprInfo ExprEmitter::visitTypeOp(EnumConstantOp op) {
os << op.field().getField().getValue();
os << op.getField().getField().getValue();
return {Selection, IsUnsigned};
}

Expand Down Expand Up @@ -2963,10 +2963,10 @@ LogicalResult StmtEmitter::visitStmt(TypedeclOp op) {
emitError(op, "SV attributes emission is unimplemented for the op");

os << "typedef ";
emitter.printPackedType(stripUnpackedTypes(op.type()), os, op.getLoc(),
emitter.printPackedType(stripUnpackedTypes(op.getType()), os, op.getLoc(),
false);
os << ' ' << op.getPreferredName();
emitter.printUnpackedTypePostfix(op.type(), os);
emitter.printUnpackedTypePostfix(op.getType(), os);
os << ";\n";
return success();
}
Expand Down Expand Up @@ -3638,12 +3638,12 @@ LogicalResult StmtEmitter::visitStmt(InstanceOp op) {
indent() << getVerilogModuleName(moduleOp);

// If this is a parameterized module, then emit the parameters.
if (!op.parameters().empty()) {
if (!op.getParameters().empty()) {
// All the parameters may be defaulted -- don't print out an empty list if
// so.
bool printed = false;
for (auto params :
llvm::zip(op.parameters(),
llvm::zip(op.getParameters(),
moduleOp->getAttrOfType<ArrayAttr>("parameters"))) {
auto param = std::get<0>(params).cast<ParamDeclAttr>();
auto modParam = std::get<1>(params).cast<ParamDeclAttr>();
Expand Down Expand Up @@ -3689,7 +3689,7 @@ LogicalResult StmtEmitter::visitStmt(InstanceOp op) {
};

// Emit the argument and result ports.
auto opArgs = op.inputs();
auto opArgs = op.getInputs();
auto opResults = op.getResults();
bool isFirst = true; // True until we print a port.
bool isZeroWidth = false;
Expand Down Expand Up @@ -4134,7 +4134,7 @@ void ModuleEmitter::emitBind(BindOp op) {
}

// Emit the argument and result ports.
auto opArgs = inst.inputs();
auto opArgs = inst.getInputs();
auto opResults = inst.getResults();
bool isFirst = true; // True until we print a port.
for (auto &elt : childPortInfo) {
Expand Down Expand Up @@ -4261,7 +4261,7 @@ void ModuleEmitter::emitHWModule(HWModuleOp module) {
}

// Add all parameters to the name table.
for (auto param : module.parameters()) {
for (auto param : module.getParameters()) {
// Add the name to the name table so any conflicting wires are renamed.
StringRef verilogName = state.globalNames.getParameterVerilogName(
module, param.cast<ParamDeclAttr>().getName());
Expand All @@ -4271,15 +4271,15 @@ void ModuleEmitter::emitHWModule(HWModuleOp module) {
SmallPtrSet<Operation *, 8> moduleOpSet;
moduleOpSet.insert(module);

emitComment(module.commentAttr());
emitComment(module.getCommentAttr());

if (hasSVAttributes(module))
emitError(module, "SV attributes emission is unimplemented for the op");

os << "module " << getVerilogModuleName(module);

// If we have any parameters, print them on their own line.
if (!module.parameters().empty()) {
if (!module.getParameters().empty()) {
os << "\n #(";

auto printParamType = [&](Type type, Attribute defaultValue,
Expand Down Expand Up @@ -4319,7 +4319,7 @@ void ModuleEmitter::emitHWModule(HWModuleOp module) {
// Determine the max width of the parameter types so things are lined up.
size_t maxTypeWidth = 0;
SmallString<8> scratch;
for (auto param : module.parameters()) {
for (auto param : module.getParameters()) {
auto paramAttr = param.cast<ParamDeclAttr>();
// Measure the type length by printing it to a temporary string.
printParamType(paramAttr.getType().getValue(), paramAttr.getValue(),
Expand All @@ -4331,7 +4331,7 @@ void ModuleEmitter::emitHWModule(HWModuleOp module) {
maxTypeWidth += 1;

llvm::interleave(
module.parameters(), os,
module.getParameters(), os,
[&](Attribute param) {
auto paramAttr = param.cast<ParamDeclAttr>();
auto defaultValue = paramAttr.getValue(); // may be null if absent.
Expand Down
2 changes: 1 addition & 1 deletion lib/Conversion/ExportVerilog/LegalizeNames.cpp
View file
Expand Up @@ -179,7 +179,7 @@ void GlobalNameResolver::legalizeModuleNames(HWModuleOp module) {
}

// Legalize the parameter names.
for (auto param : module.parameters()) {
for (auto param : module.getParameters()) {
auto paramAttr = param.cast<ParamDeclAttr>();
auto newName = nameResolver.getLegalName(paramAttr.getName());
if (newName != paramAttr.getName().getValue())
Expand Down
15 changes: 8 additions & 7 deletions lib/Conversion/FIRRTLToHW/LowerToHW.cpp
View file
Expand Up @@ -1023,7 +1023,7 @@ FIRRTLModuleLowering::lowerModule(FModuleOp oldModule, Block *topLevelModule,
if (auto outputFile = oldModule->getAttr("output_file"))
newModule->setAttr("output_file", outputFile);
if (auto comment = oldModule->getAttrOfType<StringAttr>("comment"))
newModule.commentAttr(comment);
newModule.setCommentAttr(comment);

// If the circuit has an entry point, set all other modules private.
// Otherwise, mark all modules as public.
Expand All @@ -1042,7 +1042,8 @@ FIRRTLModuleLowering::lowerModule(FModuleOp oldModule, Block *topLevelModule,
newModule->setAttr("output_file", testBenchDir);
newModule->setAttr("firrtl.extract.do_not_extract",
builder.getUnitAttr());
newModule.commentAttr(builder.getStringAttr("VCS coverage exclude_file"));
newModule.setCommentAttr(
builder.getStringAttr("VCS coverage exclude_file"));
}

bool failed = false;
Expand Down Expand Up @@ -1251,7 +1252,7 @@ FIRRTLModuleLowering::lowerModuleBody(FModuleOp oldModule,
if (!newModule)
return success();

ImplicitLocOpBuilder bodyBuilder(oldModule.getLoc(), newModule.body());
ImplicitLocOpBuilder bodyBuilder(oldModule.getLoc(), newModule.getBody());

// Use a placeholder instruction be a cursor that indicates where we want to
// move the new function body to. This is important because we insert some
Expand Down Expand Up @@ -1283,7 +1284,7 @@ FIRRTLModuleLowering::lowerModuleBody(FModuleOp oldModule,
if (!port.isOutput() && !isZeroWidth) {
// Inputs and InOuts are modeled as arguments in the result, so we can
// just map them over. We model zero bit outputs as inouts.
Value newArg = newModule.body().getArgument(nextNewArg++);
Value newArg = newModule.getBody().getArgument(nextNewArg++);

// Cast the argument to the old type, reintroducing sign information in
// the hw.module body.
Expand Down Expand Up @@ -2048,7 +2049,7 @@ LogicalResult FIRRTLLowering::setPossiblyFoldedLowering(Value orig,
// If this is a constant, check to see if we have it in our unique mapping:
// it could have come from folding an operation.
if (auto cst = dyn_cast_or_null<hw::ConstantOp>(result.getDefiningOp())) {
auto &entry = hwConstantMap[cst.valueAttr()];
auto &entry = hwConstantMap[cst.getValueAttr()];
if (entry == cst) {
// We're already using an entry in the constant map, nothing to do.
} else if (entry) {
Expand Down Expand Up @@ -3036,10 +3037,10 @@ LogicalResult FIRRTLLowering::visitDecl(InstanceOp oldInstance) {
if (oldInstance.lowerToBind())
newInstance->setAttr("doNotPrint", builder.getBoolAttr(true));

if (newInstance.inner_symAttr())
if (newInstance.getInnerSymAttr())
if (auto forceName = circuitState.instanceForceNames.lookup(
{cast<hw::HWModuleOp>(newInstance->getParentOp()).getNameAttr(),
newInstance.inner_symAttr()}))
newInstance.getInnerSymAttr()}))
newInstance->setAttr("hw.verilogName", forceName);

// Now that we have the new hw.instance, we need to remap all of the users
Expand Down
2 changes: 1 addition & 1 deletion lib/Conversion/HWToLLHD/HWToLLHD.cpp
View file
Expand Up @@ -255,7 +255,7 @@ struct ConvertInstance : public OpConversionPattern<InstanceOp> {
// original instance for replacement with the new values probed from the
// signals attached to the LLHD instance.
rewriter.create<InstOp>(instance.getLoc(), instance.instanceName(),
instance.moduleName(), arguments, resultSigs);
instance.getModuleName(), arguments, resultSigs);
rewriter.replaceOp(instance, resultValues);

return success();
Expand Down

0 comments on commit 3ab2c5a

Please sign in to comment.