From 619cd443d41c3013746e4957e9cee89656991a21 Mon Sep 17 00:00:00 2001 From: Fabian Schuiki Date: Sat, 27 Jul 2024 11:51:08 -0700 Subject: [PATCH] [Moore] Clean up struct ops and add missing tests Rework the Moore dialect operations that manipulate struct values. These are intended to operate on `StructType` and `UnpackedStructType` values directly, but were defined in ODS as operating on references to structs. This was likely a hold-over from early development where we were still figuring out the distinction between ref types and value types in SV. This commit adjusts the struct ops such that they actually operate on struct values instead of references to structs. It also moves more operand constraints into ODS and simplifies the op verifiers by factoring out some common code into helper functions. Enhance the `struct_inject` canonicalizer to also consider `struct_create` operations as part of the inject chain. This allows an initial `struct_create` that is modified by a subsequent inject to be canonicalized into a new `struct_create` with updated values. Add missing basic and error tests for the struct-related ops, and simplify the variable destructuring test by removing unrelated operations. Also fixes an issue in variable op destructuring where a variable with initial value would have its initial value discarded during destructuring. Initial values now prevent destructuring. Alternatively, we may choose to insert appropriate `struct_extract` ops to destructure the initial value in the future. --- include/circt/Dialect/Moore/MooreOps.td | 128 ++----- include/circt/Dialect/Moore/MooreTypes.td | 31 ++ lib/Conversion/ImportVerilog/Expressions.cpp | 15 +- lib/Dialect/Moore/MooreOps.cpp | 372 +++++++------------ test/Conversion/ImportVerilog/basic.sv | 10 +- test/Dialect/Moore/basic.mlir | 17 +- test/Dialect/Moore/canonicalizers.mlir | 109 +++--- test/Dialect/Moore/errors.mlir | 50 +++ test/Dialect/Moore/sroa.mlir | 106 +++--- 9 files changed, 384 insertions(+), 454 deletions(-) diff --git a/include/circt/Dialect/Moore/MooreOps.td b/include/circt/Dialect/Moore/MooreOps.td index f8ca29324071..09c8223624a7 100644 --- a/include/circt/Dialect/Moore/MooreOps.td +++ b/include/circt/Dialect/Moore/MooreOps.td @@ -924,6 +924,10 @@ def ReplicateOp : MooreOp<"replicate", [ }]; } +//===----------------------------------------------------------------------===// +// Bit/Element Extraction +//===----------------------------------------------------------------------===// + def ExtractOp : MooreOp<"extract", [Pure]> { let summary = "Extract a range or single bits from a value"; let description = [{ @@ -996,71 +1000,24 @@ def DynExtractRefOp : MooreOp<"dyn_extract_ref", [Pure]> { }]; } -def StructCreateOp : MooreOp<"struct_create", [Pure]> { - let summary = "Struct Create operation"; - let description = [{ - A structure represents a collection of data types - that can be referenced as a whole, or the individual data types - that make up the structure can be referenced by name. - By default, structures are unpacked, meaning that there is - an implementation-dependent packing of the data types. - Unpacked structures can contain any data type. - See IEEE 1800-2017 § 7.2 "Structures" +//===----------------------------------------------------------------------===// +// Struct Manipulation +//===----------------------------------------------------------------------===// - Example: - ``` - struct { bit [7:0] opcode; bit [23:0] addr; }IR; - IR.opcode = 1; // set field in IR. - // anonymous structure - // defines variable IR - typedef struct { - bit [7:0] opcode; - bit [23:0] addr; - } instruction; // named structure type - instruction IR; // define variable - ``` - See IEEE 1800-2017 § 7.2. "Structures". - }]; - let arguments = (ins Variadic:$input); - let results = (outs RefType:$result); +def StructCreateOp : MooreOp<"struct_create", [Pure]> { + let summary = "Create a struct value from individual fields"; + let arguments = (ins Variadic:$fields); + let results = (outs AnyStructType:$result); let assemblyFormat = [{ - $input attr-dict `:` type($input) `->` type($result) + $fields attr-dict `:` type($fields) `->` type($result) }]; let hasVerifier = 1; let hasFolder = 1; } -def StructExtractOp : MooreOp<"struct_extract", [ - DeclareOpInterfaceMethods -]> { - let summary = "Struct Extract operation"; - let description = [{ - Structures can be converted to bits preserving the bit pattern. - In other words, they can be converted back to the same value - without any loss of information. When unpacked data are converted - to the packed representation, the order of the data in the packed - representation is such that the first field in the structure - occupies the MSBs. The effect is the same as a concatenation of - the data items (struct fields or array elements) in order. - The type of the elements in an unpacked structure or array - shall be valid for a packed representation in order to be - cast to any other type, whether packed or unpacked. - See IEEE 1800-2017 § 6.24.1 "Cast operator" - - Example: - ``` - typedef struct { - int addr = 1 + constant; - int crc; - byte data [4] = '{4{1}}; - } packet1; - - packet1 p1; // initialization defined by the typedef. - // p1.crc will use the default value for an int - ``` - See IEEE 1800-2017 § 7.2.1 "Assigning to structures". - }]; - let arguments = (ins StrAttr:$fieldName, Arg:$input); +def StructExtractOp : MooreOp<"struct_extract", [Pure]> { + let summary = "Obtain the value of a struct field"; + let arguments = (ins StrAttr:$fieldName, AnyStructType:$input); let results = (outs UnpackedType:$result); let assemblyFormat = [{ $input `,` $fieldName attr-dict `:` type($input) `->` type($result) @@ -1070,51 +1027,46 @@ def StructExtractOp : MooreOp<"struct_extract", [ } def StructExtractRefOp : MooreOp<"struct_extract_ref", [ + Pure, DeclareOpInterfaceMethods ]> { - let summary = "Struct Extract operation"; - let arguments = (ins StrAttr:$fieldName, RefType:$input); + let summary = "Create a reference to a struct field"; + let arguments = (ins StrAttr:$fieldName, AnyStructRefType:$input); let results = (outs RefType:$result); let assemblyFormat = [{ - $input `,` $fieldName attr-dict `:` - type($input) `->` type($result) + $input `,` $fieldName attr-dict `:` type($input) `->` type($result) }]; let hasVerifier = 1; } -def StructInjectOp : MooreOp<"struct_inject", [Pure]> { - let summary = "Struct Field operation"; +def StructInjectOp : MooreOp<"struct_inject", [ + Pure, + AllTypesMatch<["input", "result"]> +]> { + let summary = "Update the value of a struct field"; let description = [{ - A structure can be assigned as a whole and passed to - or from a subroutine as a whole. Members of a structure - data type can be assigned individual default member - values by using an initial assignment with the declaration - of each member. The assigned expression shall be - a constant expression. - See IEEE 1800-2017 § 7.2.2 "Assigning to structures" - - Example: - ``` - typedef struct { - int addr = 1 + constant; - int crc; - byte data [4] = '{4{1}}; - } packet1; - - packet1 p1; // initialization defined by the typedef. - // p1.crc will use the default value for an int - ``` - See IEEE 1800-2017 § 7.2. "Assigning to structures". + Takes an existing struct value, sets one of its fields to a new value, and + returns the resulting struct value. + }]; + let arguments = (ins + AnyStructType:$input, + StrAttr:$fieldName, + UnpackedType:$newValue + ); + let results = (outs AnyStructType:$result); + let assemblyFormat = [{ + $input `,` $fieldName `,` $newValue attr-dict + `:` type($input) `,` type($newValue) }]; - let arguments = (ins RefType:$input, StrAttr:$fieldName, - UnpackedType:$newValue); - let results = (outs RefType:$result); - let hasCustomAssemblyFormat = 1; let hasVerifier = 1; let hasFolder = 1; let hasCanonicalizeMethod = true; } +//===----------------------------------------------------------------------===// +// Union Manipulation +//===----------------------------------------------------------------------===// + def UnionCreateOp : MooreOp<"union_create", [Pure]> { let summary = "Union Create operation"; let description = [{ diff --git a/include/circt/Dialect/Moore/MooreTypes.td b/include/circt/Dialect/Moore/MooreTypes.td index 700308b35a0b..5830daab8aae 100644 --- a/include/circt/Dialect/Moore/MooreTypes.td +++ b/include/circt/Dialect/Moore/MooreTypes.td @@ -406,4 +406,35 @@ def BitType : MooreType, + "packed or unpacked struct type", + "moore::UnpackedType">; + +/// A packed or unpacked union type. +def AnyUnionType : MooreType< + Or<[UnionType.predicate, UnpackedUnionType.predicate]>, + "packed or unpacked union type", + "moore::UnpackedType">; + +/// A ref type with the specified constraints on the nested type. +class SpecificRefType : ConfinedType($_self).getNestedType()", + type.predicate>], + "ref of " # type.summary, "moore::RefType" +> { + Type nestedType = type; +} + +/// Struct references. +def StructRefType : SpecificRefType; +def UnpackedStructRefType : SpecificRefType; +def AnyStructRefType : SpecificRefType; + +/// Union references. +def UnionRefType : SpecificRefType; +def UnpackedUnionRefType : SpecificRefType; +def AnyUnionRefType : SpecificRefType; + #endif // CIRCT_DIALECT_MOORE_MOORETYPES diff --git a/lib/Conversion/ImportVerilog/Expressions.cpp b/lib/Conversion/ImportVerilog/Expressions.cpp index 1b480f1800c3..c1b077d9a9d4 100644 --- a/lib/Conversion/ImportVerilog/Expressions.cpp +++ b/lib/Conversion/ImportVerilog/Expressions.cpp @@ -83,7 +83,7 @@ struct RvalueExprVisitor { // value for this expression's symbol to the `context.valueSymbols` table. auto d = mlir::emitError(loc, "unknown name `") << expr.symbol.name << "`"; d.attachNote(context.convertLocation(expr.symbol.location)) - << "no value generated for " << slang::ast::toString(expr.symbol.kind); + << "no rvalue generated for " << slang::ast::toString(expr.symbol.kind); return {}; } @@ -113,15 +113,6 @@ struct RvalueExprVisitor { return {}; } - if (auto refOp = lhs.getDefiningOp()) { - auto input = refOp.getInput(); - if (isa(input.getDefiningOp()->getParentOp())) { - builder.create(loc, input.getType(), input, - refOp.getFieldNameAttr(), rhs); - refOp->erase(); - return rhs; - } - } if (expr.isNonBlocking()) builder.create(loc, lhs, rhs); else @@ -520,7 +511,7 @@ struct RvalueExprVisitor { Value visit(const slang::ast::MemberAccessExpression &expr) { auto type = context.convertType(*expr.type); auto valueType = expr.value().type; - auto value = context.convertLvalueExpression(expr.value()); + auto value = context.convertRvalueExpression(expr.value()); if (!type || !value) return {}; if (valueType->isStruct()) { @@ -755,7 +746,7 @@ struct LvalueExprVisitor { return value; auto d = mlir::emitError(loc, "unknown name `") << expr.symbol.name << "`"; d.attachNote(context.convertLocation(expr.symbol.location)) - << "no value generated for " << slang::ast::toString(expr.symbol.kind); + << "no lvalue generated for " << slang::ast::toString(expr.symbol.kind); return {}; } diff --git a/lib/Dialect/Moore/MooreOps.cpp b/lib/Dialect/Moore/MooreOps.cpp index 567bf0883f4f..5c365648c62c 100644 --- a/lib/Dialect/Moore/MooreOps.cpp +++ b/lib/Dialect/Moore/MooreOps.cpp @@ -261,6 +261,8 @@ void VariableOp::getAsmResultNames(OpAsmSetValueNameFn setNameFn) { llvm::SmallVector VariableOp::getPromotableSlots() { if (isa(getOperation()->getParentOp())) return {}; + if (getInitial()) + return {}; return {MemorySlot{getResult(), getType()}}; } @@ -310,6 +312,8 @@ LogicalResult VariableOp::canonicalize(VariableOp op, SmallVector VariableOp::getDestructurableSlots() { if (isa(getOperation()->getParentOp())) return {}; + if (getInitial()) + return {}; auto refType = getType(); auto destructurable = llvm::dyn_cast(refType); @@ -328,6 +332,7 @@ DenseMap VariableOp::destructure( const SmallPtrSetImpl &usedIndices, OpBuilder &builder, SmallVectorImpl &newAllocators) { assert(slot.ptr == getResult()); + assert(!getInitial()); builder.setInsertionPointAfter(*this); auto destructurableType = cast(getType()); @@ -335,8 +340,12 @@ DenseMap VariableOp::destructure( for (Attribute index : usedIndices) { auto elemType = cast(destructurableType.getTypeAtIndex(index)); assert(elemType && "used index must exist"); - auto varOp = builder.create(getLoc(), elemType, - cast(index), Value()); + StringAttr varName; + if (auto name = getName(); name && !name->empty()) + varName = StringAttr::get( + getContext(), (*name) + "." + cast(index).getValue()); + auto varOp = + builder.create(getLoc(), elemType, varName, Value()); newAllocators.push_back(varOp); slotMap.try_emplace(index, {varOp.getResult(), elemType}); } @@ -507,47 +516,59 @@ LogicalResult ConcatRefOp::inferReturnTypes( // StructCreateOp //===----------------------------------------------------------------------===// -LogicalResult StructCreateOp::verify() { - /// checks if the types of the inputs are exactly equal to the types of the - /// result struct fields - return TypeSwitch(getType().getNestedType()) - .Case([this](auto &type) { - auto members = type.getMembers(); - auto inputs = getInput(); - if (inputs.size() != members.size()) - return failure(); - for (size_t i = 0; i < members.size(); i++) { - auto memberType = cast(members[i].type); - auto inputType = inputs[i].getType(); - if (inputType != memberType) { - emitOpError("input types must match struct field types and orders"); - return failure(); - } - } - return success(); - }) - .Default([this](auto &) { - emitOpError("Result type must be StructType or UnpackedStructType"); - return failure(); - }); +static std::optional getStructFieldIndex(Type type, StringAttr name) { + if (auto structType = dyn_cast(type)) + return structType.getFieldIndex(name); + if (auto structType = dyn_cast(type)) + return structType.getFieldIndex(name); + assert(0 && "expected StructType or UnpackedStructType"); + return {}; } -OpFoldResult StructCreateOp::fold(FoldAdaptor adaptor) { - auto inputs = adaptor.getInput(); +static ArrayRef getStructMembers(Type type) { + if (auto structType = dyn_cast(type)) + return structType.getMembers(); + if (auto structType = dyn_cast(type)) + return structType.getMembers(); + assert(0 && "expected StructType or UnpackedStructType"); + return {}; +} - if (llvm::any_of(inputs, [](Attribute attr) { return !attr; })) - return {}; +static UnpackedType getStructFieldType(Type type, StringAttr name) { + if (auto index = getStructFieldIndex(type, name)) + return getStructMembers(type)[*index].type; + return {}; +} - auto members = TypeSwitch>( - cast(getType()).getNestedType()) - .Case( - [](auto &type) { return type.getMembers(); }) - .Default([](auto) { return std::nullopt; }); - SmallVector namedInputs; - for (auto [input, member] : llvm::zip(inputs, members)) - namedInputs.push_back(NamedAttribute(member.name, input)); +LogicalResult StructCreateOp::verify() { + auto members = getStructMembers(getType()); + + // Check that the number of operands matches the number of struct fields. + if (getFields().size() != members.size()) + return emitOpError() << "has " << getFields().size() + << " operands, but result type requires " + << members.size(); + + // Check that the operand types match the struct field types. + for (auto [index, pair] : llvm::enumerate(llvm::zip(getFields(), members))) { + auto [value, member] = pair; + if (value.getType() != member.type) + return emitOpError() << "operand #" << index << " has type " + << value.getType() << ", but struct field " + << member.name << " requires " << member.type; + } + return success(); +} - return DictionaryAttr::get(getContext(), namedInputs); +OpFoldResult StructCreateOp::fold(FoldAdaptor adaptor) { + SmallVector fields; + for (auto [member, field] : + llvm::zip(getStructMembers(getType()), adaptor.getFields())) { + if (!field) + return {}; + fields.push_back(NamedAttribute(member.name, field)); + } + return DictionaryAttr::get(getContext(), fields); } //===----------------------------------------------------------------------===// @@ -555,73 +576,36 @@ OpFoldResult StructCreateOp::fold(FoldAdaptor adaptor) { //===----------------------------------------------------------------------===// LogicalResult StructExtractOp::verify() { - /// checks if the type of the result match field type in this struct - return TypeSwitch(getInput().getType().getNestedType()) - .Case([this](auto &type) { - auto members = type.getMembers(); - auto filedName = getFieldName(); - auto resultType = getType(); - for (const auto &member : members) { - if (member.name == filedName && member.type == resultType) { - return success(); - } - } - emitOpError("result type must match struct field type"); - return failure(); - }) - .Default([this](auto &) { - emitOpError("input type must be StructType or UnpackedStructType"); - return failure(); - }); -} - -bool StructExtractOp::canRewire(const DestructurableMemorySlot &slot, - SmallPtrSetImpl &usedIndices, - SmallVectorImpl &mustBeSafelyUsed, - const DataLayout &dataLayout) { - if (slot.ptr == getInput()) { - usedIndices.insert(getFieldNameAttr()); - return true; - } - return false; -} - -DeletionKind StructExtractOp::rewire(const DestructurableMemorySlot &slot, - DenseMap &subslots, - OpBuilder &builder, - const DataLayout &dataLayout) { - auto index = getFieldNameAttr(); - const auto &memorySlot = subslots.at(index); - auto readOp = builder.create( - getLoc(), cast(memorySlot.elemType).getNestedType(), - memorySlot.ptr); - replaceAllUsesWith(readOp.getResult()); - getInputMutable().drop(); - erase(); - return DeletionKind::Keep; + auto type = getStructFieldType(getInput().getType(), getFieldNameAttr()); + if (!type) + return emitOpError() << "extracts field " << getFieldNameAttr() + << " which does not exist in " << getInput().getType(); + if (type != getType()) + return emitOpError() << "result type " << getType() + << " must match struct field type " << type; + return success(); } OpFoldResult StructExtractOp::fold(FoldAdaptor adaptor) { - if (auto constOperand = adaptor.getInput()) { - auto operandAttr = llvm::cast(constOperand); - for (const auto &ele : operandAttr) - if (ele.getName() == getFieldNameAttr()) - return ele.getValue(); + // Extract on a constant struct input. + if (auto fields = dyn_cast_or_null(adaptor.getInput())) + if (auto value = fields.get(getFieldNameAttr())) + return value; + + // extract(inject(s, "field", v), "field") -> v + if (auto inject = getInput().getDefiningOp()) { + if (inject.getFieldNameAttr() == getFieldNameAttr()) + return inject.getNewValue(); + return {}; } - if (auto structInject = getInput().getDefiningOp()) - return structInject.getFieldNameAttr() == getFieldNameAttr() - ? structInject.getNewValue() - : Value(); - if (auto structCreate = getInput().getDefiningOp()) { - auto ind = TypeSwitch>( - getInput().getType().getNestedType()) - .Case([this](auto &type) { - return type.getFieldIndex(getFieldNameAttr()); - }) - .Default([](auto &) { return std::nullopt; }); - return ind.has_value() ? structCreate->getOperand(ind.value()) : Value(); + // extract(create({"field": v, ...}), "field") -> v + if (auto create = getInput().getDefiningOp()) { + if (auto index = getStructFieldIndex(create.getType(), getFieldNameAttr())) + return create.getFields()[*index]; + return {}; } + return {}; } @@ -630,25 +614,15 @@ OpFoldResult StructExtractOp::fold(FoldAdaptor adaptor) { //===----------------------------------------------------------------------===// LogicalResult StructExtractRefOp::verify() { - /// checks if the type of the result match field type in this struct - return TypeSwitch(getInput().getType().getNestedType()) - .Case([this](auto &type) { - auto members = type.getMembers(); - auto filedName = getFieldName(); - auto resultType = getType().getNestedType(); - for (const auto &member : members) { - if (member.name == filedName && member.type == resultType) { - return success(); - } - } - emitOpError("result type must match struct field type"); - return failure(); - }) - .Default([this](auto &) { - emitOpError("input type must be refrence of StructType or " - "UnpackedStructType"); - return failure(); - }); + auto type = getStructFieldType( + cast(getInput().getType()).getNestedType(), getFieldNameAttr()); + if (!type) + return emitOpError() << "extracts field " << getFieldNameAttr() + << " which does not exist in " << getInput().getType(); + if (type != getType().getNestedType()) + return emitOpError() << "result ref of type " << getType().getNestedType() + << " must match struct field type " << type; + return success(); } bool StructExtractRefOp::canRewire( @@ -682,75 +656,14 @@ StructExtractRefOp::rewire(const DestructurableMemorySlot &slot, //===----------------------------------------------------------------------===// LogicalResult StructInjectOp::verify() { - /// checks if the type of the new value match field type in this struct - return TypeSwitch(getInput().getType().getNestedType()) - .Case([this](auto &type) { - auto members = type.getMembers(); - auto filedName = getFieldName(); - auto newValueType = getNewValue().getType(); - for (const auto &member : members) { - if (member.name == filedName && member.type == newValueType) { - return success(); - } - } - emitOpError("new value type must match struct field type"); - return failure(); - }) - .Default([this](auto &) { - emitOpError("input type must be StructType or UnpackedStructType"); - return failure(); - }); -} - -void StructInjectOp::print(OpAsmPrinter &p) { - p << " "; - p.printOperand(getInput()); - p << ", " << getFieldNameAttr() << ", "; - p.printOperand(getNewValue()); - p << " : " << getInput().getType(); -} - -ParseResult StructInjectOp::parse(OpAsmParser &parser, OperationState &result) { - llvm::SMLoc inputOperandsLoc = parser.getCurrentLocation(); - OpAsmParser::UnresolvedOperand operand, val; - StringAttr fieldName; - Type declType; - - if (parser.parseOperand(operand) || parser.parseComma() || - parser.parseAttribute(fieldName) || parser.parseComma() || - parser.parseOperand(val) || parser.parseColonType(declType)) - return failure(); - - return TypeSwitch(cast(declType).getNestedType()) - .Case([&parser, &result, &declType, - &fieldName, &operand, &val, - &inputOperandsLoc](auto &type) { - auto members = type.getMembers(); - Type fieldType; - for (const auto &member : members) - if (member.name == fieldName) - fieldType = member.type; - if (!fieldType) { - parser.emitError(parser.getNameLoc(), - "field name '" + fieldName.getValue() + - "' not found in struct type"); - return failure(); - } - - auto fieldNameAttr = - StringAttr::get(parser.getContext(), Twine(fieldName)); - result.addAttribute("fieldName", fieldNameAttr); - result.addTypes(declType); - if (parser.resolveOperands({operand, val}, {declType, fieldType}, - inputOperandsLoc, result.operands)) - return failure(); - - return success(); - }) - .Default([&parser, &inputOperandsLoc](auto &) { - return parser.emitError(inputOperandsLoc, - "invalid kind of type specified"); - }); + auto type = getStructFieldType(getInput().getType(), getFieldNameAttr()); + if (!type) + return emitOpError() << "injects field " << getFieldNameAttr() + << " which does not exist in " << getInput().getType(); + if (type != getNewValue().getType()) + return emitOpError() << "injected value " << getNewValue().getType() + << " must match struct field type " << type; + return success(); } OpFoldResult StructInjectOp::fold(FoldAdaptor adaptor) { @@ -758,66 +671,51 @@ OpFoldResult StructInjectOp::fold(FoldAdaptor adaptor) { auto newValue = adaptor.getNewValue(); if (!input || !newValue) return {}; - SmallVector array; - llvm::copy(cast(input), std::back_inserter(array)); - for (auto &ele : array) { - if (ele.getName() == getFieldName()) - ele.setValue(newValue); - } - return DictionaryAttr::get(getContext(), array); + NamedAttrList fields(cast(input)); + fields.set(getFieldNameAttr(), newValue); + return fields.getDictionary(getContext()); } LogicalResult StructInjectOp::canonicalize(StructInjectOp op, PatternRewriter &rewriter) { - // Canonicalize multiple injects into a create op and eliminate overwrites. - SmallPtrSet injects; - DenseMap fields; - - // Chase a chain of injects. Bail out if cycles are present. - StructInjectOp inject = op; - Value input; - do { - if (!injects.insert(inject).second) + auto members = getStructMembers(op.getType()); + + // Chase a chain of `struct_inject` ops, with an optional final + // `struct_create`, and take note of the values assigned to each field. + SmallPtrSet injectOps; + DenseMap fieldValues; + Value input = op; + while (auto injectOp = input.getDefiningOp()) { + if (!injectOps.insert(injectOp).second) return failure(); - - fields.try_emplace(inject.getFieldNameAttr(), inject.getNewValue()); - input = inject.getInput(); - inject = input.getDefiningOp(); - } while (inject); - assert(input && "missing input to inject chain"); - - auto members = TypeSwitch>( - cast(op.getType()).getNestedType()) - .Case( - [](auto &type) { return type.getMembers(); }) - .Default([](auto) { return std::nullopt; }); - - // If the inject chain sets all fields, canonicalize to create. - if (fields.size() == members.size()) { - SmallVector createFields; - for (const auto &member : members) { - auto it = fields.find(member.name); - assert(it != fields.end() && "missing field"); - createFields.push_back(it->second); - } - op.getInputMutable(); - rewriter.replaceOpWithNewOp(op, op.getType(), createFields); + fieldValues.insert({injectOp.getFieldNameAttr(), injectOp.getNewValue()}); + input = injectOp.getInput(); + } + if (auto createOp = input.getDefiningOp()) + for (auto [value, member] : llvm::zip(createOp.getFields(), members)) + fieldValues.insert({member.name, value}); + + // If the inject chain sets all fields, canonicalize to a `struct_create`. + if (fieldValues.size() == members.size()) { + SmallVector values; + values.reserve(fieldValues.size()); + for (auto member : members) + values.push_back(fieldValues.lookup(member.name)); + rewriter.replaceOpWithNewOp(op, op.getType(), values); return success(); } - // Nothing to canonicalize, only the original inject in the chain. - if (injects.size() == fields.size()) + // If each inject op in the chain assigned to a unique field, there is nothing + // to canonicalize. + if (injectOps.size() == fieldValues.size()) return failure(); - // Eliminate overwrites. The hash map contains the last write to each field. - for (const auto &member : members) { - auto it = fields.find(member.name); - if (it == fields.end()) - continue; - input = rewriter.create(op.getLoc(), op.getType(), input, - member.name, it->second); - } - + // Otherwise we can eliminate overwrites by creating new injects. The hash map + // of field values contains the last assigned value for each field. + for (auto member : members) + if (auto value = fieldValues.lookup(member.name)) + input = rewriter.create(op.getLoc(), op.getType(), input, + member.name, value); rewriter.replaceOp(op, input); return success(); } diff --git a/test/Conversion/ImportVerilog/basic.sv b/test/Conversion/ImportVerilog/basic.sv index e2567da8451f..85e572300f0f 100644 --- a/test/Conversion/ImportVerilog/basic.sv +++ b/test/Conversion/ImportVerilog/basic.sv @@ -997,12 +997,14 @@ module Expressions; // CHECK: moore.blocking_assign %a, [[TMP2]] a += (a *= a--); - // CHECK: [[TMP1:%.+]] = moore.read %a - // CHECK: [[TMP2:%.+]] = moore.struct_inject %struct0, "a", [[TMP1]] : !moore.ref> + // CHECK: [[TMP1:%.+]] = moore.struct_extract_ref %struct0, "a" : > -> + // CHECK: [[TMP2:%.+]] = moore.read %a + // CHECK: moore.blocking_assign [[TMP1]], [[TMP2]] struct0.a = a; - // CHECK: [[TMP3:%.+]] = moore.struct_extract %struct0, "b" : > -> i32 - // CHECK: moore.blocking_assign %b, [[TMP3]] : i32 + // CHECK: [[TMP1:%.+]] = moore.read %struct0 + // CHECK: [[TMP2:%.+]] = moore.struct_extract [[TMP1]], "b" : struct<{a: i32, b: i32}> -> i32 + // CHECK: moore.blocking_assign %b, [[TMP2]] b = struct0.b; //===------------------------------------------------------------------===// diff --git a/test/Dialect/Moore/basic.mlir b/test/Dialect/Moore/basic.mlir index 2aacd336b911..e6f372df8167 100644 --- a/test/Dialect/Moore/basic.mlir +++ b/test/Dialect/Moore/basic.mlir @@ -132,7 +132,12 @@ moore.module @Expressions( // CHECK-SAME: in [[REF_ARRAY1:%[^:]+]] : !moore.ref> in %refArray1: !moore.ref>, // CHECK-SAME: in [[REF_ARRAY2:%[^:]+]] : !moore.ref>> - in %refArray2: !moore.ref>> + in %refArray2: !moore.ref>>, + + // CHECK-SAME: in [[STRUCT1:%.+]] : !moore.struct<{a: i32, b: i32}> + in %struct1: !moore.struct<{a: i32, b: i32}>, + // CHECK-SAME: in [[REF_STRUCT1:%.+]] : !moore.ref> + in %refStruct1: !moore.ref> ) { // CHECK: moore.constant 0 : i32 moore.constant 0 : i32 @@ -269,6 +274,16 @@ moore.module @Expressions( } { moore.yield %b : i32 } + + // CHECK: moore.struct_create [[A]], [[B]] : !moore.i32, !moore.i32 -> struct<{a: i32, b: i32}> + moore.struct_create %a, %b : !moore.i32, !moore.i32 -> struct<{a: i32, b: i32}> + // CHECK: moore.struct_extract [[STRUCT1]], "a" : struct<{a: i32, b: i32}> -> i32 + moore.struct_extract %struct1, "a" : struct<{a: i32, b: i32}> -> i32 + // CHECK: moore.struct_extract_ref [[REF_STRUCT1]], "a" : > -> + moore.struct_extract_ref %refStruct1, "a" : > -> + // CHECK: moore.struct_inject [[STRUCT1]], "a", [[B]] : struct<{a: i32, b: i32}>, i32 + moore.struct_inject %struct1, "a", %b : struct<{a: i32, b: i32}>, i32 + moore.output } diff --git a/test/Dialect/Moore/canonicalizers.mlir b/test/Dialect/Moore/canonicalizers.mlir index 284c30150b64..872fa7766d00 100644 --- a/test/Dialect/Moore/canonicalizers.mlir +++ b/test/Dialect/Moore/canonicalizers.mlir @@ -38,65 +38,58 @@ moore.module @MultiAssign() { moore.output } -// CHECK-LABEL: moore.module @structAssign -moore.module @structAssign(out a : !moore.ref>) { - %x = moore.variable : - %y = moore.variable : - %z = moore.variable : - // CHECK: %0 = moore.constant 4 : i32 - // CHECK: %1 = moore.read %x - // CHECK: %2 = moore.constant 1 : i32 - // CHECK: %3 = moore.add %1, %2 : i32 - // CHECK: %4 = moore.struct_create %3, %0 : !moore.i32, !moore.i32 -> > - %ii = moore.variable : > - %0 = moore.constant 4 : i32 - %1 = moore.conversion %0 : !moore.i32 -> !moore.i32 - %2 = moore.struct_inject %ii, "b", %1 : !moore.ref> - %3 = moore.read %x : - %4 = moore.constant 1 : i32 - %5 = moore.add %3, %4 : i32 - %6 = moore.struct_inject %2, "a", %5 : !moore.ref> - %7 = moore.struct_extract %6, "a" : > -> i32 - // CHECK: moore.assign %y, %3 : i32 - moore.assign %y, %7 : i32 - %8 = moore.struct_extract %6, "a" : > -> i32 - // CHECK: moore.assign %z, %3 : i32 - moore.assign %z, %8 : i32 - // CHECK: moore.output %4 : !moore.ref> - moore.output %6 : !moore.ref> +// CHECK-LABEL: func.func @StructExtractFold1 +func.func @StructExtractFold1(%arg0: !moore.struct<{a: i17, b: i42}>, %arg1: !moore.i17) -> (!moore.i17) { + // CHECK-NEXT: return %arg1 : !moore.i17 + %0 = moore.struct_inject %arg0, "a", %arg1 : struct<{a: i17, b: i42}>, i17 + %1 = moore.struct_extract %0, "a" : struct<{a: i17, b: i42}> -> i17 + return %1 : !moore.i17 } -// CHECK-LABEL: moore.module @structInjectFold -moore.module @structInjectFold(out a : !moore.ref>) { - %x = moore.variable : - %y = moore.variable : - %z = moore.variable : - %ii = moore.variable : > - // CHECK: %0 = moore.read %x - // CHECK: %1 = moore.constant 1 : i32 - // CHECK: %2 = moore.add %0, %1 : i32 - // CHECK: %3 = moore.struct_inject %ii, "a", %2 : !moore.ref> - %0 = moore.constant 4 : i32 - %1 = moore.conversion %0 : !moore.i32 -> !moore.i32 - %2 = moore.struct_inject %ii, "a", %1 : !moore.ref> - %3 = moore.read %x : - %4 = moore.constant 1 : i32 - %5 = moore.add %3, %4 : i32 - %6 = moore.struct_inject %2, "a", %5 : !moore.ref> - %7 = moore.struct_extract %6, "a" : > -> i32 - // CHECK: moore.assign %y, %2 : i32 - moore.assign %y, %7 : i32 - %8 = moore.struct_extract %6, "a" : > -> i32 - // CHECK: moore.assign %z, %2 : i32 - moore.assign %z, %8 : i32 - // CHECK: moore.output %3 : !moore.ref> - moore.output %6 : !moore.ref> +// CHECK-LABEL: func.func @StructExtractFold2 +func.func @StructExtractFold2(%arg0: !moore.i17, %arg1: !moore.i42) -> (!moore.i17, !moore.i42) { + // CHECK-NEXT: return %arg0, %arg1 : !moore.i17, !moore.i42 + %0 = moore.struct_create %arg0, %arg1 : !moore.i17, !moore.i42 -> struct<{a: i17, b: i42}> + %1 = moore.struct_extract %0, "a" : struct<{a: i17, b: i42}> -> i17 + %2 = moore.struct_extract %0, "b" : struct<{a: i17, b: i42}> -> i42 + return %1, %2 : !moore.i17, !moore.i42 } -// CHECK-LABEL: moore.module @structCreateFold -moore.module @structCreateFold(in %a : !moore.i1, out b : !moore.i1) { - %0 = moore.struct_create %a : !moore.i1 -> > - %1 = moore.struct_extract %0, "a" : > -> i1 - // CHECK: moore.output %a : !moore.i1 - moore.output %1 : !moore.i1 - } +// CHECK-LABEL: func.func @StructInjectFold1 +func.func @StructInjectFold1(%arg0: !moore.struct<{a: i32, b: i32}>) -> (!moore.struct<{a: i32, b: i32}>) { + // CHECK-NEXT: [[C42:%.+]] = moore.constant 42 + // CHECK-NEXT: [[C43:%.+]] = moore.constant 43 + // CHECK-NEXT: [[TMP:%.+]] = moore.struct_create [[C42]], [[C43]] : !moore.i32, !moore.i32 -> struct<{a: i32, b: i32}> + // CHECK-NEXT: return [[TMP]] + %0 = moore.constant 42 : i32 + %1 = moore.constant 43 : i32 + %2 = moore.struct_inject %arg0, "a", %1 : struct<{a: i32, b: i32}>, i32 + %3 = moore.struct_inject %2, "b", %1 : struct<{a: i32, b: i32}>, i32 + %4 = moore.struct_inject %3, "a", %0 : struct<{a: i32, b: i32}>, i32 + return %4 : !moore.struct<{a: i32, b: i32}> +} + +// CHECK-LABEL: func.func @StructInjectFold2 +func.func @StructInjectFold2() -> (!moore.struct<{a: i32, b: i32}>) { + // CHECK-NEXT: [[C42:%.+]] = moore.constant 42 + // CHECK-NEXT: [[C43:%.+]] = moore.constant 43 + // CHECK-NEXT: [[TMP:%.+]] = moore.struct_create [[C42]], [[C43]] : !moore.i32, !moore.i32 -> struct<{a: i32, b: i32}> + // CHECK-NEXT: return [[TMP]] + %0 = moore.constant 42 : i32 + %1 = moore.constant 43 : i32 + %2 = moore.struct_create %0, %0 : !moore.i32, !moore.i32 -> struct<{a: i32, b: i32}> + %3 = moore.struct_inject %2, "b", %1 : struct<{a: i32, b: i32}>, i32 + return %3 : !moore.struct<{a: i32, b: i32}> +} + +// CHECK-LABEL: func.func @StructInjectFold3 +func.func @StructInjectFold3(%arg0: !moore.struct<{a: i32, b: i32}>) -> (!moore.struct<{a: i32, b: i32}>) { + // CHECK-NEXT: [[C43:%.+]] = moore.constant 43 + // CHECK-NEXT: [[TMP:%.+]] = moore.struct_inject %arg0, "a", [[C43]] : struct<{a: i32, b: i32}>, i32 + // CHECK-NEXT: return [[TMP]] + %0 = moore.constant 42 : i32 + %1 = moore.constant 43 : i32 + %2 = moore.struct_inject %arg0, "a", %0 : struct<{a: i32, b: i32}>, i32 + %3 = moore.struct_inject %2, "a", %1 : struct<{a: i32, b: i32}>, i32 + return %3 : !moore.struct<{a: i32, b: i32}> +} diff --git a/test/Dialect/Moore/errors.mlir b/test/Dialect/Moore/errors.mlir index 494f603fe7cd..de561d2dcb07 100644 --- a/test/Dialect/Moore/errors.mlir +++ b/test/Dialect/Moore/errors.mlir @@ -97,3 +97,53 @@ moore.conditional %0 : i1 -> i32 { // expected-error @below {{yield type must match conditional. Expected '!moore.i32', but got '!moore.i8'}} moore.yield %2 : i8 } + +// ----- + +%0 = moore.constant 42 : i32 +// expected-error @below {{op has 1 operands, but result type requires 2}} +moore.struct_create %0 : !moore.i32 -> struct<{a: i32, b: i32}> + +// ----- + +%0 = moore.constant 42 : i32 +// expected-error @below {{op operand #0 has type '!moore.i32', but struct field "a" requires '!moore.i1337'}} +moore.struct_create %0 : !moore.i32 -> struct<{a: i1337}> + +// ----- + +%0 = unrealized_conversion_cast to !moore.struct<{a: i32}> +// expected-error @below {{op extracts field "b" which does not exist}} +moore.struct_extract %0, "b" : struct<{a: i32}> -> i9001 + +// ----- + +%0 = unrealized_conversion_cast to !moore.struct<{a: i32}> +// expected-error @below {{op result type '!moore.i9001' must match struct field type '!moore.i32'}} +moore.struct_extract %0, "a" : struct<{a: i32}> -> i9001 + +// ----- + +%0 = unrealized_conversion_cast to !moore.ref> +// expected-error @below {{op extracts field "b" which does not exist}} +moore.struct_extract_ref %0, "b" : > -> + +// ----- + +%0 = unrealized_conversion_cast to !moore.ref> +// expected-error @below {{op result ref of type '!moore.i9001' must match struct field type '!moore.i32'}} +moore.struct_extract_ref %0, "a" : > -> + +// ----- + +%0 = unrealized_conversion_cast to !moore.struct<{a: i32}> +%1 = moore.constant 42 : i32 +// expected-error @below {{op injects field "b" which does not exist}} +moore.struct_inject %0, "b", %1 : struct<{a: i32}>, i32 + +// ----- + +%0 = unrealized_conversion_cast to !moore.struct<{a: i32}> +%1 = moore.constant 42 : i9001 +// expected-error @below {{op injected value '!moore.i9001' must match struct field type '!moore.i32'}} +moore.struct_inject %0, "a", %1 : struct<{a: i32}>, i9001 diff --git a/test/Dialect/Moore/sroa.mlir b/test/Dialect/Moore/sroa.mlir index 8f31489a80a1..61b0fca5c5bc 100644 --- a/test/Dialect/Moore/sroa.mlir +++ b/test/Dialect/Moore/sroa.mlir @@ -1,58 +1,56 @@ // RUN: circt-opt --sroa %s | FileCheck %s -// CHECK-LABEL: moore.module @LocalVar -moore.module @LocalVar() { - // CHECK: %x = moore.variable : - // CHECK: %y = moore.variable : - // CHECK: %z = moore.variable : - %x = moore.variable : - %y = moore.variable : - %z = moore.variable : - moore.procedure always_comb { - // CHECK: %a = moore.variable : - // CHECK: %b = moore.variable : - // CHECK: %0 = moore.constant 1 : i32 - // CHECK: %1 = moore.conversion %0 : !moore.i32 -> !moore.i32 - // CHECK: moore.blocking_assign %a, %1 : i32 - // CHECK: %2 = moore.constant 4 : i32 - // CHECK: %3 = moore.conversion %2 : !moore.i32 -> !moore.i32 - // CHECK: moore.blocking_assign %b, %3 : i32 - // CHECK: %4 = moore.read %x - // CHECK: %5 = moore.constant 1 : i32 - // CHECK: %6 = moore.add %4, %5 : i32 - // CHECK: moore.blocking_assign %a, %6 : i32 - // CHECK: %7 = moore.read %a - // CHECK: moore.blocking_assign %y, %7 : i32 - // CHECK: %8 = moore.read %a - // CHECK: %9 = moore.constant 1 : i32 - // CHECK: %10 = moore.add %8, %9 : i32 - // CHECK: moore.blocking_assign %a, %10 : i32 - // CHECK: %11 = moore.read %a - // CHECK: moore.blocking_assign %z, %11 : i32 - %ii = moore.variable : > - %0 = moore.struct_extract_ref %ii, "a" : > -> - %1 = moore.constant 1 : i32 - %2 = moore.conversion %1 : !moore.i32 -> !moore.i32 - moore.blocking_assign %0, %2 : i32 - %3 = moore.struct_extract_ref %ii, "b" : > -> - %4 = moore.constant 4 : i32 - %5 = moore.conversion %4 : !moore.i32 -> !moore.i32 - moore.blocking_assign %3, %5 : i32 - %6 = moore.struct_extract_ref %ii, "a" : > -> - %7 = moore.read %x : - %8 = moore.constant 1 : i32 - %9 = moore.add %7, %8 : i32 - moore.blocking_assign %6, %9 : i32 - %10 = moore.struct_extract %ii, "a" : > -> i32 - moore.blocking_assign %y, %10 : i32 - %11 = moore.struct_extract_ref %ii, "a" : > -> - %12 = moore.struct_extract %ii, "a" : > -> i32 - %13 = moore.constant 1 : i32 - %14 = moore.add %12, %13 : i32 - moore.blocking_assign %11, %14 : i32 - %15 = moore.struct_extract %ii, "a" : > -> i32 - moore.blocking_assign %z, %15 : i32 - } -} +// CHECK-LABEL: func.func @SplitStructs +func.func @SplitStructs(%arg0: !moore.i42, %arg1: !moore.i1337) { + // Named variables + + // CHECK-DAG: %x.a = moore.variable : + // CHECK-DAG: %x.b = moore.variable : + // CHECK-NOT: moore.struct_extract_ref + %x = moore.variable : > + %0 = moore.struct_extract_ref %x, "a" : > -> + %1 = moore.struct_extract_ref %x, "b" : > -> + // CHECK: moore.blocking_assign %x.a, %arg0 + // CHECK: moore.blocking_assign %x.b, %arg1 + moore.blocking_assign %0, %arg0 : !moore.i42 + moore.blocking_assign %1, %arg1 : !moore.i1337 + + // Anonymous variables + // CHECK-DAG: [[A:%.+]] = moore.variable : + // CHECK-DAG: [[B:%.+]] = moore.variable : + // CHECK-NOT: moore.struct_extract_ref + %2 = moore.variable : > + %3 = moore.struct_extract_ref %2, "a" : > -> + %4 = moore.struct_extract_ref %2, "b" : > -> + // CHECK: moore.blocking_assign [[A]], %arg0 + // CHECK: moore.blocking_assign [[B]], %arg1 + moore.blocking_assign %3, %arg0 : !moore.i42 + moore.blocking_assign %4, %arg1 : !moore.i1337 + return +} + +// CHECK-LABEL: func.func @SplitNestedStructs +func.func @SplitNestedStructs(%arg0: !moore.i42, %arg1: !moore.i1337, %arg2: !moore.i9001) { + // CHECK-DAG: %k.a.x = moore.variable : + // CHECK-DAG: %k.a.y = moore.variable : + // CHECK-DAG: %k.b.u = moore.variable : + // CHECK-DAG: %k.b.v = moore.variable : + %k = moore.variable : , b: struct<{u: i1337, v: i9001}>}>> + %0 = moore.struct_extract_ref %k, "a" : , b: struct<{u: i1337, v: i9001}>}>> -> > + %1 = moore.struct_extract_ref %k, "b" : , b: struct<{u: i1337, v: i9001}>}>> -> > + %2 = moore.struct_extract_ref %0, "x" : > -> + %3 = moore.struct_extract_ref %0, "y" : > -> + %4 = moore.struct_extract_ref %1, "u" : > -> + %5 = moore.struct_extract_ref %1, "v" : > -> + // CHECK: moore.blocking_assign %k.a.x, %arg0 + // CHECK: moore.blocking_assign %k.a.y, %arg1 + // CHECK: moore.blocking_assign %k.b.u, %arg1 + // CHECK: moore.blocking_assign %k.b.v, %arg2 + moore.blocking_assign %2, %arg0 : !moore.i42 + moore.blocking_assign %3, %arg1 : !moore.i1337 + moore.blocking_assign %4, %arg1 : !moore.i1337 + moore.blocking_assign %5, %arg2 : !moore.i9001 + return +}