[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

@@ -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<UnpackedType>:$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<UnpackedType>:$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<DestructurableAccessorOpInterface>
-]> {
-  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<RefType, "", [MemRead]>:$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<DestructurableAccessorOpInterface>
 ]> {
-  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 = [{

include/circt/Dialect/Moore/MooreTypes.td

@@ -406,4 +406,35 @@ def BitType : MooreType<CPred<[{
   }];
 }
 
+/// A packed or unpacked struct type.
+def AnyStructType : MooreType<
+  Or<[StructType.predicate, UnpackedStructType.predicate]>,
+  "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<Type type> : ConfinedType<RefType,
+  [SubstLeaves<"$_self", "llvm::cast<moore::RefType>($_self).getNestedType()",
+               type.predicate>],
+  "ref of " # type.summary, "moore::RefType"
+> {
+  Type nestedType = type;
+}
+
+/// Struct references.
+def StructRefType : SpecificRefType<StructType>;
+def UnpackedStructRefType : SpecificRefType<UnpackedStructType>;
+def AnyStructRefType : SpecificRefType<AnyStructType>;
+
+/// Union references.
+def UnionRefType : SpecificRefType<UnionType>;
+def UnpackedUnionRefType : SpecificRefType<UnpackedUnionType>;
+def AnyUnionRefType : SpecificRefType<AnyUnionType>;
+
 #endif // CIRCT_DIALECT_MOORE_MOORETYPES

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<moore::StructExtractRefOp>()) {
-      auto input = refOp.getInput();
-      if (isa<moore::SVModuleOp>(input.getDefiningOp()->getParentOp())) {
-        builder.create<moore::StructInjectOp>(loc, input.getType(), input,
-                                              refOp.getFieldNameAttr(), rhs);
-        refOp->erase();
-        return rhs;
-      }
-    }
     if (expr.isNonBlocking())
       builder.create<moore::NonBlockingAssignOp>(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 {};
   }