Skip to content

Commit

Permalink
[Moore] Drop named_constant op in favor of dbg.variable
Browse files Browse the repository at this point in the history 
Remove the `NamedConstantOp` and replace its uses with `VariableOp` from
the debug dialect.

The op was originally added to track the value of constant parameters,
localparams, and specparams in the IR. In ImportVerilog, such parameters
would generate a corresponding `named_constant` op and all references to
the parameter by name would be replaced with the `named_constant`'s
result.

This doesn't really work well for parameters defined outside a module,
such as in packages or at the root of the Verilog source file. (Modules
are isolated from above, preventing the use of `named_constant`s from
outside the module.) Therefore expressions would generally fall back to
materializing constants directly where they were used.

Since the named constant ops are only there to track a constant value in
the IR for the user's debugging convenience, using the debug dialect
directly feels a lot more appropriate.
  • Loading branch information
@fabianschuiki
fabianschuiki committed Sep 24, 2024
1 parent 76f3ca5 commit 18c8c5c
Show file tree
Hide file tree
Showing 9 changed files with 163 additions and 210 deletions.
37 changes: 5 additions & 32 deletions include/circt/Dialect/Moore/MooreOps.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ def DetectEventOp : MooreOp<"detect_event", [
}

//===----------------------------------------------------------------------===//
// Expressions
// Constants
//===----------------------------------------------------------------------===//

def ConstantOp : MooreOp<"constant", [Pure, ConstantLike]> {
Expand All @@ -476,37 +476,6 @@ def ConstantOp : MooreOp<"constant", [Pure, ConstantLike]> {
];
}

def Parameter : I32EnumAttrCase<"Parameter", 0, "parameter">;
def LocalParameter : I32EnumAttrCase<"LocalParameter", 1, "localparam">;
def SpecParameter : I32EnumAttrCase<"SpecParameter", 2, "specparam">;

def NamedConstAttr : I32EnumAttr<"NamedConst", "elaboration-time constants",
[Parameter, LocalParameter, SpecParameter]>{
let cppNamespace = "circt::moore";
}

def NamedConstantOp : MooreOp<"named_constant", [
SameOperandsAndResultType,
DeclareOpInterfaceMethods<OpAsmOpInterface, ["getAsmResultNames"]>
]>{
let summary = "An elaboration time constant expression";
let description = [{
Constants are named data objects that never change. SystemVerilog provides
three elaboration-time constants: parameter, localparam, and specparam.

See IEEE 1800-2017 § 6.20 "Constants".
}];
let arguments = (ins StrAttr:$name,
NamedConstAttr:$kind,
IntType:$value);
let results = (outs IntType:$result);
let assemblyFormat = [{
$kind
``custom<ImplicitSSAName>($name)
$value `:` type($result) attr-dict
}];
}

def StringConstantOp : MooreOp<"string_constant", [Pure]> {
let summary = "Produce a constant string value";
let description = [{
Expand All @@ -522,6 +491,10 @@ def StringConstantOp : MooreOp<"string_constant", [Pure]> {
let assemblyFormat = "$value attr-dict `:` type($result)";
}

//===----------------------------------------------------------------------===//
// Expressions
//===----------------------------------------------------------------------===//

def ConversionOp : MooreOp<"conversion", [Pure]> {
let summary = "A type conversion";
let description = [{
Expand Down
1 change: 1 addition & 0 deletions lib/Conversion/ImportVerilog/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ add_circt_translation_library(CIRCTImportVerilog
slang_slang

LINK_LIBS PUBLIC
CIRCTDebug
CIRCTHW
CIRCTMoore
MLIRFuncDialect
Expand Down
5 changes: 3 additions & 2 deletions lib/Conversion/ImportVerilog/ImportVerilog.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,8 +264,9 @@ LogicalResult ImportDriver::importVerilog(ModuleOp module) {
return success();

// Traverse the parsed Verilog AST and map it to the equivalent CIRCT ops.
mlirContext->loadDialect<moore::MooreDialect, hw::HWDialect,
cf::ControlFlowDialect, func::FuncDialect>();
mlirContext
->loadDialect<moore::MooreDialect, hw::HWDialect, cf::ControlFlowDialect,
func::FuncDialect, debug::DebugDialect>();
auto conversionTimer = ts.nest("Verilog to dialect mapping");
Context context(options, *compilation, module, driver.sourceManager,
bufferFilePaths);
Expand Down
1 change: 1 addition & 0 deletions lib/Conversion/ImportVerilog/ImportVerilogInternals.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#define CONVERSION_IMPORTVERILOG_IMPORTVERILOGINTERNALS_H

#include "circt/Conversion/ImportVerilog.h"
#include "circt/Dialect/Debug/DebugOps.h"
#include "circt/Dialect/HW/HWOps.h"
#include "circt/Dialect/Moore/MooreOps.h"
#include "mlir/Dialect/ControlFlow/IR/ControlFlowOps.h"
Expand Down
143 changes: 65 additions & 78 deletions lib/Conversion/ImportVerilog/Structure.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,21 @@
using namespace circt;
using namespace ImportVerilog;

//===----------------------------------------------------------------------===//
// Utilities
//===----------------------------------------------------------------------===//

static void guessNamespacePrefix(const slang::ast::Symbol &symbol,
SmallString<64> &prefix) {
if (symbol.kind != slang::ast::SymbolKind::Package)
return;
guessNamespacePrefix(symbol.getParentScope()->asSymbol(), prefix);
if (!symbol.name.empty()) {
prefix += symbol.name;
prefix += "::";
}
}

//===----------------------------------------------------------------------===//
// Base Visitor
//===----------------------------------------------------------------------===//
Expand All @@ -20,6 +35,13 @@ namespace {
/// Base visitor which ignores AST nodes that are handled by Slang's name
/// resolution and type checking.
struct BaseVisitor {
Context &context;
Location loc;
OpBuilder &builder;

BaseVisitor(Context &context, Location loc)
: context(context), loc(loc), builder(context.builder) {}

// Skip semicolons.
LogicalResult visit(const slang::ast::EmptyMemberSymbol &) {
return success();
Expand All @@ -41,6 +63,46 @@ struct BaseVisitor {
LogicalResult visit(const slang::ast::WildcardImportSymbol &) {
return success();
}

// Skip type parameters. The Slang AST is already monomorphized.
LogicalResult visit(const slang::ast::TypeParameterSymbol &) {
return success();
}

// Handle parameters.
LogicalResult visit(const slang::ast::ParameterSymbol &param) {
visitParameter(param);
return success();
}

LogicalResult visit(const slang::ast::SpecparamSymbol &param) {
visitParameter(param);
return success();
}

template <class Node>
void visitParameter(const Node &param) {
// If debug info is enabled, try to materialize the parameter's constant
// value on a best-effort basis and create a `dbg.variable` to track the
// value.
if (!context.options.debugInfo)
return;
auto value =
context.materializeConstant(param.getValue(), param.getType(), loc);
if (!value)
return;
if (builder.getInsertionBlock()->getParentOp() == context.intoModuleOp)
context.orderedRootOps.insert({param.location, value.getDefiningOp()});

// Prefix the parameter name with the surrounding namespace to create
// somewhat sane names in the IR.
SmallString<64> paramName;
guessNamespacePrefix(param.getParentScope()->asSymbol(), paramName);
paramName += param.name;

builder.create<debug::VariableOp>(loc, builder.getStringAttr(paramName),
value, Value{});
}
};
} // namespace

Expand All @@ -50,15 +112,9 @@ struct BaseVisitor {

namespace {
struct RootVisitor : public BaseVisitor {
using BaseVisitor::BaseVisitor;
using BaseVisitor::visit;

Context &context;
Location loc;
OpBuilder &builder;

RootVisitor(Context &context, Location loc)
: context(context), loc(loc), builder(context.builder) {}

// Handle packages.
LogicalResult visit(const slang::ast::PackageSymbol &package) {
return context.convertPackage(package);
Expand All @@ -85,22 +141,9 @@ struct RootVisitor : public BaseVisitor {

namespace {
struct PackageVisitor : public BaseVisitor {
using BaseVisitor::BaseVisitor;
using BaseVisitor::visit;

Context &context;
Location loc;
OpBuilder &builder;

PackageVisitor(Context &context, Location loc)
: context(context), loc(loc), builder(context.builder) {}

// Ignore parameters. These are materialized on-the-fly as `ConstantOp`s.
LogicalResult visit(const slang::ast::ParameterSymbol &) { return success(); }
LogicalResult visit(const slang::ast::SpecparamSymbol &) { return success(); }
LogicalResult visit(const slang::ast::TypeParameterSymbol &) {
return success();
}

// Handle functions and tasks.
LogicalResult visit(const slang::ast::SubroutineSymbol &subroutine) {
return context.convertFunction(subroutine);
Expand Down Expand Up @@ -177,15 +220,9 @@ static moore::NetKind convertNetKind(slang::ast::NetType::NetKind kind) {

namespace {
struct ModuleVisitor : public BaseVisitor {
using BaseVisitor::BaseVisitor;
using BaseVisitor::visit;

Context &context;
Location loc;
OpBuilder &builder;

ModuleVisitor(Context &context, Location loc)
: context(context), loc(loc), builder(context.builder) {}

// Skip ports which are already handled by the module itself.
LogicalResult visit(const slang::ast::PortSymbol &) { return success(); }
LogicalResult visit(const slang::ast::MultiPortSymbol &) { return success(); }
Expand Down Expand Up @@ -453,45 +490,6 @@ struct ModuleVisitor : public BaseVisitor {
return success();
}

// Handle parameters.
LogicalResult visit(const slang::ast::ParameterSymbol &paramNode) {
auto type = cast<moore::IntType>(context.convertType(paramNode.getType()));
if (!type)
return failure();

auto valueInt = paramNode.getValue().integer().as<uint64_t>().value();
Value value = builder.create<moore::ConstantOp>(loc, type, valueInt);

auto namedConstantOp = builder.create<moore::NamedConstantOp>(
loc, type, builder.getStringAttr(paramNode.name),
paramNode.isLocalParam()
? moore::NamedConstAttr::get(context.getContext(),
moore::NamedConst::LocalParameter)
: moore::NamedConstAttr::get(context.getContext(),
moore::NamedConst::Parameter),
value);
context.valueSymbols.insert(&paramNode, namedConstantOp);
return success();
}

// Handle specparam.
LogicalResult visit(const slang::ast::SpecparamSymbol &spNode) {
auto type = cast<moore::IntType>(context.convertType(spNode.getType()));
if (!type)
return failure();

auto valueInt = spNode.getValue().integer().as<uint64_t>().value();
Value value = builder.create<moore::ConstantOp>(loc, type, valueInt);

auto namedConstantOp = builder.create<moore::NamedConstantOp>(
loc, type, builder.getStringAttr(spNode.name),
moore::NamedConstAttr::get(context.getContext(),
moore::NamedConst::SpecParameter),
value);
context.valueSymbols.insert(&spNode, namedConstantOp);
return success();
}

// Handle generate block.
LogicalResult visit(const slang::ast::GenerateBlockSymbol &genNode) {
if (!genNode.isUninstantiated) {
Expand Down Expand Up @@ -794,17 +792,6 @@ Context::convertPackage(const slang::ast::PackageSymbol &package) {
return success();
}

static void guessNamespacePrefix(const slang::ast::Symbol &symbol,
SmallString<64> &prefix) {
if (symbol.kind == slang::ast::SymbolKind::Root)
return;
guessNamespacePrefix(symbol.getParentScope()->asSymbol(), prefix);
if (!symbol.name.empty()) {
prefix += symbol.name;
prefix += "::";
}
}

/// Convert a function and its arguments to a function declaration in the IR.
/// This does not convert the function body.
FunctionLowering *
Expand Down
31 changes: 1 addition & 30 deletions lib/Conversion/MooreToCore/MooreToCore.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -513,35 +513,6 @@ struct ConstantOpConv : public OpConversionPattern<ConstantOp> {
}
};

struct NamedConstantOpConv : public OpConversionPattern<NamedConstantOp> {
using OpConversionPattern::OpConversionPattern;

LogicalResult
matchAndRewrite(NamedConstantOp op, OpAdaptor adaptor,
ConversionPatternRewriter &rewriter) const override {

Type resultType = typeConverter->convertType(op.getResult().getType());
SmallString<32> symStr;
switch (op.getKind()) {
case NamedConst::Parameter:
symStr = "parameter";
break;
case NamedConst::LocalParameter:
symStr = "localparameter";
break;
case NamedConst::SpecParameter:
symStr = "specparameter";
break;
}
auto symAttr =
rewriter.getStringAttr(symStr + Twine("_") + adaptor.getName());
rewriter.replaceOpWithNewOp<hw::WireOp>(op, resultType, adaptor.getValue(),
op.getNameAttr(),
hw::InnerSymAttr::get(symAttr));
return success();
}
};

struct ConcatOpConversion : public OpConversionPattern<ConcatOp> {
using OpConversionPattern::OpConversionPattern;
LogicalResult
Expand Down Expand Up @@ -1341,7 +1312,7 @@ static void populateOpConversion(RewritePatternSet &patterns,
// Patterns of miscellaneous operations.
ConstantOpConv, ConcatOpConversion, ReplicateOpConversion,
ExtractOpConversion, DynExtractOpConversion, DynExtractRefOpConversion,
ConversionOpConversion, ReadOpConversion, NamedConstantOpConv,
ConversionOpConversion, ReadOpConversion,
StructExtractOpConversion, StructExtractRefOpConversion,
ExtractRefOpConversion, StructCreateOpConversion, ConditionalOpConversion,
YieldOpConversion, OutputOpConversion,
Expand Down
8 changes: 0 additions & 8 deletions lib/Dialect/Moore/MooreOps.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -633,14 +633,6 @@ OpFoldResult ConstantOp::fold(FoldAdaptor adaptor) {
return getValueAttr();
}

//===----------------------------------------------------------------------===//
// NamedConstantOp
//===----------------------------------------------------------------------===//

void NamedConstantOp::getAsmResultNames(OpAsmSetValueNameFn setNameFn) {
setNameFn(getResult(), getName());
}

//===----------------------------------------------------------------------===//
// ConcatOp
//===----------------------------------------------------------------------===//
Expand Down
Loading

0 comments on commit 18c8c5c

Please sign in to comment.