From a0afaa44f4e6fecc9189ad3e9912205da2a84e11 Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Wed, 22 Sep 2021 09:34:56 -0700 Subject: [PATCH] [HW] Add simple expressions to the parameter support. This includes export verilog and legalizenames support. This is just a minimal first step. It has several issues that we need to improve over time: 1) Support variadic operators. 2) Support better .mlir syntax 3) Precedence aware printing in generated verilog. --- include/circt/Dialect/HW/HWAttributes.h | 7 +++++ include/circt/Dialect/HW/HWAttributes.td | 16 ++++++++++ lib/Dialect/HW/HWAttributes.cpp | 31 +++++++++++++++++++ lib/Dialect/HW/HWOps.cpp | 10 ++++++ lib/Dialect/SV/Transforms/HWLegalizeNames.cpp | 13 ++++++++ .../ExportVerilog/ExportVerilog.cpp | 13 ++++++++ test/Dialect/HW/modules.mlir | 7 ++++- test/Dialect/SV/hw-legalize-names.mlir | 3 ++ test/ExportVerilog/hw-dialect.mlir | 9 ++++-- 9 files changed, 106 insertions(+), 3 deletions(-) diff --git a/include/circt/Dialect/HW/HWAttributes.h b/include/circt/Dialect/HW/HWAttributes.h index 1153195290..b4e3ccaff9 100644 --- a/include/circt/Dialect/HW/HWAttributes.h +++ b/include/circt/Dialect/HW/HWAttributes.h @@ -12,6 +12,13 @@ #include "mlir/IR/Attributes.h" #include "mlir/IR/BuiltinAttributes.h" +namespace circt { +namespace hw { +class PBOAttr; +enum class PBO : uint32_t; +} // namespace hw +} // namespace circt + #define GET_ATTRDEF_CLASSES #include "circt/Dialect/HW/HWAttributes.h.inc" diff --git a/include/circt/Dialect/HW/HWAttributes.td b/include/circt/Dialect/HW/HWAttributes.td index e71642a04c..ffc4f95c3e 100644 --- a/include/circt/Dialect/HW/HWAttributes.td +++ b/include/circt/Dialect/HW/HWAttributes.td @@ -85,6 +85,22 @@ def ParamVerbatimAttr : AttrDef { let mnemonic = "param.verbatim"; } + +let cppNamespace = "circt::hw" in { +def PBO_Add : I32EnumAttrCase<"Add", 0, "add">; +def PBO_Mul : I32EnumAttrCase<"Mul", 1, "mul">; +def PBOAttr : I32EnumAttr<"PBO", "Parameter Binary Operation Code", + [PBO_Add, PBO_Mul]>; +} + +def ParamBinaryAttr : AttrDef { + let summary = "Binary operation combining two parameter expressions"; + let parameters = (ins "PBO":$opcode, + "::mlir::Attribute":$lhs, "::mlir::Attribute":$rhs, + AttributeSelfTypeParameter<"">:$type); + let mnemonic = "param.binary"; +} + let cppNamespace = "circt::hw" in { def WUW_Undefined : I32EnumAttrCase<"Undefined", 0>; def WUW_PortOrder : I32EnumAttrCase<"PortOrder", 1>; diff --git a/lib/Dialect/HW/HWAttributes.cpp b/lib/Dialect/HW/HWAttributes.cpp index d076666c10..de6a95f143 100644 --- a/lib/Dialect/HW/HWAttributes.cpp +++ b/lib/Dialect/HW/HWAttributes.cpp @@ -96,3 +96,34 @@ Attribute ParamVerbatimAttr::parse(MLIRContext *context, DialectAsmParser &p, void ParamVerbatimAttr::print(DialectAsmPrinter &p) const { p << "param.verbatim<" << getValue() << ">"; } + +//===----------------------------------------------------------------------===// +// ParamBinaryAttr +//===----------------------------------------------------------------------===// + +Attribute ParamBinaryAttr::parse(MLIRContext *context, DialectAsmParser &p, + Type type) { + Attribute lhs, rhs; + StringRef opcodeStr; + auto loc = p.getCurrentLocation(); + if (p.parseLess() || p.parseKeyword(&opcodeStr) || + p.parseAttribute(lhs, type) || p.parseComma() || + p.parseAttribute(rhs, type) || p.parseGreater()) + return Attribute(); + + Optional opcode = symbolizePBO(opcodeStr); + if (!opcode.hasValue()) { + p.emitError(loc, "unknown binary operator name"); + return {}; + } + + return ParamBinaryAttr::get(context, *opcode, lhs, rhs, type); +} + +void ParamBinaryAttr::print(DialectAsmPrinter &p) const { + p << "param.binary<" << stringifyPBO(getOpcode()) << " "; + p.printAttributeWithoutType(getLhs()); + p << ", "; + p.printAttributeWithoutType(getRhs()); + p << ">"; +} diff --git a/lib/Dialect/HW/HWOps.cpp b/lib/Dialect/HW/HWOps.cpp index ab33181527..ea77e8e101 100644 --- a/lib/Dialect/HW/HWOps.cpp +++ b/lib/Dialect/HW/HWOps.cpp @@ -52,6 +52,16 @@ LogicalResult hw::checkParameterInContext(Attribute value, Operation *module, value.isa() || value.isa()) return success(); + // Check both arms of an expression. + if (auto binop = value.dyn_cast()) { + if (failed(checkParameterInContext(binop.getLhs(), module, usingOp, + disallowParamRefs)) || + failed(checkParameterInContext(binop.getRhs(), module, usingOp, + disallowParamRefs))) + return failure(); + return success(); + } + // Parameter references need more analysis to make sure they are valid within // this module. if (auto parameterRef = value.dyn_cast()) { diff --git a/lib/Dialect/SV/Transforms/HWLegalizeNames.cpp b/lib/Dialect/SV/Transforms/HWLegalizeNames.cpp index 30658cee54..b69ec89f42 100644 --- a/lib/Dialect/SV/Transforms/HWLegalizeNames.cpp +++ b/lib/Dialect/SV/Transforms/HWLegalizeNames.cpp @@ -220,6 +220,19 @@ remapRenamedParameters(Attribute value, HWModuleOp module, value.isa() || value.isa()) return value; + // Remap leaves of expressions if needed. + if (auto binOp = value.dyn_cast()) { + auto newLHS = + remapRenamedParameters(binOp.getLhs(), module, renamedParameterInfo); + auto newRHS = + remapRenamedParameters(binOp.getRhs(), module, renamedParameterInfo); + // Don't rebuild an attribute if nothing changed. + if (newLHS == binOp.getLhs() && newRHS == binOp.getRhs()) + return value; + return ParamBinaryAttr::get(value.getContext(), binOp.getOpcode(), newLHS, + newRHS, value.getType()); + } + // TODO: Handle nested expressions when we support them. // Otherwise this must be a parameter reference. diff --git a/lib/Translation/ExportVerilog/ExportVerilog.cpp b/lib/Translation/ExportVerilog/ExportVerilog.cpp index 94667fc804..8edffdfadb 100644 --- a/lib/Translation/ExportVerilog/ExportVerilog.cpp +++ b/lib/Translation/ExportVerilog/ExportVerilog.cpp @@ -83,6 +83,19 @@ static void printParamValue(Attribute value, Operation *op, StringRef paramName, os << verbatimParam.getValue().getValue(); } else if (auto parameterRef = value.dyn_cast()) { os << parameterRef.getName().getValue(); + } else if (auto paramBinOp = value.dyn_cast()) { + printParamValue(paramBinOp.getLhs(), op, paramName, os); + + // FIXME: Handle precedence, support variadic versions of these. + switch (paramBinOp.getOpcode()) { + case PBO::Add: + os << " + "; + break; + case PBO::Mul: + os << " * "; + break; + } + printParamValue(paramBinOp.getRhs(), op, paramName, os); } else { os << "<>"; emitError(op->getLoc(), "unknown parameter value '") diff --git a/test/Dialect/HW/modules.mlir b/test/Dialect/HW/modules.mlir index acd673e075..fa38f7fad2 100644 --- a/test/Dialect/HW/modules.mlir +++ b/test/Dialect/HW/modules.mlir @@ -82,7 +82,12 @@ hw.module.extern @NoArg() // CHECK-LABEL: hw.module @UseParameters() { hw.module @UseParameters() { - // CHECK: hw.instance "verbatimparam" @NoArg>() -> () + // CHECK: hw.instance "verbatimparam" @NoArg>() -> () hw.instance "verbatimparam" @NoArg>() -> () + + // CHECK: hw.instance "verbatimparam" @NoArg, 17>>() -> () + hw.instance "verbatimparam" @NoArg, 17>>() -> () hw.output } diff --git a/test/Dialect/SV/hw-legalize-names.mlir b/test/Dialect/SV/hw-legalize-names.mlir index 85789f263d..555ba83424 100644 --- a/test/Dialect/SV/hw-legalize-names.mlir +++ b/test/Dialect/SV/hw-legalize-names.mlir @@ -122,6 +122,9 @@ hw.module @parameters(%p1: i8) { // CHECK: hw.instance "inst" @module_with_bool> hw.instance "inst" @module_with_bool>() -> () + + // CHECK: hw.instance "inst2" @module_with_bool, true>>() + hw.instance "inst2" @module_with_bool, true>>() -> () } // CHECK-LABEL: hw.module @use_parameters diff --git a/test/ExportVerilog/hw-dialect.mlir b/test/ExportVerilog/hw-dialect.mlir index a6b545e687..be54636221 100644 --- a/test/ExportVerilog/hw-dialect.mlir +++ b/test/ExportVerilog/hw-dialect.mlir @@ -861,7 +861,7 @@ hw.module @UseParameterized(%a: i8) -> (ww: i8, xx: i8, yy: i8, zz: i8) { } // CHECK-LABEL: module UseParameterValue -hw.module @UseParameterValue(%arg0: i8) -> (out: i8) { +hw.module @UseParameterValue(%arg0: i8) -> (out1: i8, out2: i8) { // CHECK-NEXT: #(parameter [41:0] xx) ( // CHECK: parameters2 #( @@ -869,6 +869,11 @@ hw.module @UseParameterValue(%arg0: i8) -> (out: i8) { // CHECK-NEXT: ) inst1 ( %a = hw.instance "inst1" @parameters2, p2: i1 = 0>(arg0: %arg0: i8) -> (out: i8) - hw.output %a : i8 + // CHECK: parameters2 #( + // CHECK-NEXT: .p1(xx + 42'd17) + // CHECK-NEXT: ) inst2 ( + %b = hw.instance "inst2" @parameters2, 17>, p2: i1 = 0>(arg0: %arg0: i8) -> (out: i8) + + hw.output %a, %b : i8, i8 }