[ExportVerilog] Legalize field names of struct types (#2359)

This commit adds `FieldNameResolver` class to legalize field names in
struct types. Field names may contain verilog reserved keywrods so it's
necessary to rename them before the emission. 

With this commit, field names are renamed while their name being printed.
We rename field names in the module scope, not in global scope.
Therefore, it might be possible to emit different field names for the
same type if they are in different modules.

lib/Conversion/ExportVerilog/ExportVerilog.cpp

@@ -950,6 +950,11 @@ public:
   StringRef getNameRemotely(Value value, const ModulePortInfo &modulePorts,
                             HWModuleOp remoteModule);
 
+  /// Legalize the given field name if it is an invalid verilog name.
+  StringRef getVerilogStructFieldName(StringAttr field) {
+    return fieldNameResolver.getRenamedFieldName(field).getValue();
+  }
+
   //===--------------------------------------------------------------------===//
   // Methods for formatting types.
 
@@ -1000,6 +1005,9 @@ public:
   /// expressions in a procedural region, because we otherwise just emit wires
   /// on demand.
   SmallPtrSet<Operation *, 16> expressionsEmittedIntoDecl;
+
+  /// This class keeps track of field name renamings in the module scope.
+  FieldNameResolver fieldNameResolver;
 };
 
 } // end anonymous namespace
@@ -1097,7 +1105,7 @@ static bool printPackedTypeImpl(Type type, raw_ostream &os, Location loc,
           printPackedTypeImpl(stripUnpackedTypes(element.type), os, loc,
                               structDims, /*implicitIntType=*/false,
                               /*singleBitDefaultType=*/true, emitter);
-          os << ' ' << element.name.getValue();
+          os << ' ' << emitter.getVerilogStructFieldName(element.name);
           emitter.printUnpackedTypePostfix(element.type, os);
           os << "; ";
         }
@@ -2077,7 +2085,7 @@ SubExprInfo ExprEmitter::visitSV(IndexedPartSelectOp op) {
 
 SubExprInfo ExprEmitter::visitSV(StructFieldInOutOp op) {
   auto prec = emitSubExpr(op.input(), Selection, OOLUnary);
-  os << '.' << op.field();
+  os << '.' << emitter.getVerilogStructFieldName(op.fieldAttr());
   return {Selection, prec.signedness};
 }
 
@@ -2101,9 +2109,9 @@ SubExprInfo ExprEmitter::visitTypeOp(StructCreateOp op) {
   StructType stype = op.getType();
   os << "'{";
   size_t i = 0;
-  llvm::interleaveComma(stype.getElements(), os,
-                        [&](const StructType::FieldInfo &field) {
-                          os << field.name.getValue() << ": ";
+  llvm::interleaveComma(
+      stype.getElements(), os, [&](const StructType::FieldInfo &field) {
+        os << emitter.getVerilogStructFieldName(field.name) << ": ";
         emitSubExpr(op.getOperand(i++), Selection, OOLBinary);
       });
   os << '}';
@@ -2112,16 +2120,16 @@ SubExprInfo ExprEmitter::visitTypeOp(StructCreateOp op) {
 
 SubExprInfo ExprEmitter::visitTypeOp(StructExtractOp op) {
   emitSubExpr(op.input(), Selection, OOLUnary);
-  os << '.' << op.field();
+  os << '.' << emitter.getVerilogStructFieldName(op.fieldAttr());
   return {Selection, IsUnsigned};
 }
 
 SubExprInfo ExprEmitter::visitTypeOp(StructInjectOp op) {
   StructType stype = op.getType().cast<StructType>();
   os << "'{";
-  llvm::interleaveComma(stype.getElements(), os,
-                        [&](const StructType::FieldInfo &field) {
-                          os << field.name.getValue() << ": ";
+  llvm::interleaveComma(
+      stype.getElements(), os, [&](const StructType::FieldInfo &field) {
+        os << emitter.getVerilogStructFieldName(field.name) << ": ";
         if (field.name == op.field()) {
           emitSubExpr(op.newValue(), Selection, OOLBinary);
         } else {

lib/Conversion/ExportVerilog/ExportVerilogInternals.h

@@ -87,6 +87,31 @@ private:
   void operator=(const NameCollisionResolver &) = delete;
 };
 
+//===----------------------------------------------------------------------===//
+// FieldNameResolver
+//===----------------------------------------------------------------------===//
+
+struct FieldNameResolver {
+  FieldNameResolver() = default;
+
+  StringAttr getRenamedFieldName(StringAttr fieldName);
+
+private:
+  void setRenamedFieldName(StringAttr fieldName, StringAttr newFieldName);
+
+  /// Those contain entries for field names and types respectively. Struct types
+  /// have names as field names, which must be renamed if they conflict with
+  /// verilog keywords.
+  DenseMap<StringAttr, StringAttr> renamedFieldNames;
+
+  /// This contains field names *after* the type legalization to avoid conflicts
+  /// of renamed field names.
+  llvm::StringSet<> usedFieldNames;
+
+  /// Numeric suffix used as uniquification agent when resolving conflicts.
+  size_t nextGeneratedNameID = 0;
+};
+
 //===----------------------------------------------------------------------===//
 // SharedEmitterState
 //===----------------------------------------------------------------------===//

lib/Conversion/ExportVerilog/LegalizeNames.cpp

@@ -41,6 +41,39 @@ StringRef NameCollisionResolver::getLegalName(StringRef originalName) {
   return legalizeName(originalName, usedNames, nextGeneratedNameID);
 }
 
+//===----------------------------------------------------------------------===//
+// FieldNameResolver
+//===----------------------------------------------------------------------===//
+
+void FieldNameResolver::setRenamedFieldName(StringAttr fieldName,
+                                            StringAttr newFieldName) {
+  renamedFieldNames[fieldName] = newFieldName;
+  usedFieldNames.insert(newFieldName);
+}
+
+StringAttr FieldNameResolver::getRenamedFieldName(StringAttr fieldName) {
+  auto it = renamedFieldNames.find(fieldName);
+  if (it != renamedFieldNames.end())
+    return it->second;
+
+  // If a field name is not verilog name or used already, we have to rename it.
+  bool hasToBeRenamed = !sv::isNameValid(fieldName.getValue()) ||
+                        usedFieldNames.count(fieldName.getValue());
+
+  if (!hasToBeRenamed) {
+    setRenamedFieldName(fieldName, fieldName);
+    return fieldName;
+  }
+
+  StringRef newFieldName = sv::legalizeName(
+      fieldName.getValue(), usedFieldNames, nextGeneratedNameID);
+
+  auto newFieldNameAttr = StringAttr::get(fieldName.getContext(), newFieldName);
+
+  setRenamedFieldName(fieldName, newFieldNameAttr);
+  return newFieldNameAttr;
+}
+
 //===----------------------------------------------------------------------===//
 // GlobalNameResolver
 //===----------------------------------------------------------------------===//

test/Conversion/ExportVerilog/hw-dialect.mlir

@@ -856,6 +856,42 @@ hw.module @structExtractFromTemporary(%cond: i1, %a: !hw.struct<c: i1>, %b: !hw.
     hw.output %1 : i1
 }
 
+// Rename field names
+// CHECK-LABEL: renameKeyword(
+// CHECK-NEXT:  input  struct packed {logic repeat_0; logic repeat_0_1; } a,
+// CHECK-NEXT:  output struct packed {logic repeat_0; logic repeat_0_1; } r1);
+hw.module @renameKeyword(%a: !hw.struct<repeat: i1, repeat_0: i1>) -> (r1: !hw.struct<repeat: i1, repeat_0: i1>){
+  hw.output %a : !hw.struct<repeat: i1, repeat_0: i1>
+}
+
+// CHECK-LABEL: useRenamedStruct(
+// CHECK-NEXT:  inout  struct packed {logic repeat_0; logic repeat_0_1; } a,
+// CHECK-NEXT:  output                                                    r1, r2,
+// CHECK-NEXT:  output struct packed {logic repeat_0; logic repeat_0_1; } r3);
+hw.module @useRenamedStruct(%a: !hw.inout<struct<repeat: i1, repeat_0: i1>>) -> (r1: i1, r2: i1, r3: !hw.struct<repeat: i1, repeat_0: i1>) {
+  // CHECK-EMPTY:
+  // CHECK-NEXT: wire struct packed {logic repeat_0; logic repeat_0_1; } inst1_r1;
+  %read = sv.read_inout %a : !hw.inout<struct<repeat: i1, repeat_0: i1>>
+
+  %i0 = hw.instance "inst1" @renameKeyword(a: %read: !hw.struct<repeat: i1, repeat_0: i1>) -> (r1: !hw.struct<repeat: i1, repeat_0: i1>)
+  // CHECK:      renameKeyword inst1 (
+  // CHECK-NEXT:   .a  (a),
+  // CHECK-NEXT:   .r1 (inst1_r1)
+  // CHECK-NEXT: )
+
+  // CHECK: wire struct packed {logic repeat_0; logic repeat_0_1; } [[WIREA:.+]] = a;
+  %0 = sv.struct_field_inout %a["repeat"] : !hw.inout<struct<repeat: i1, repeat_0: i1>>
+  %1 = sv.read_inout %0 : !hw.inout<i1>
+  // assign r1 = a.repeat_0;
+  %2 = hw.struct_extract %read["repeat_0"] : !hw.struct<repeat: i1, repeat_0: i1>
+  // assign r2 = [[WIREA]].repeat_0_1;
+  %true = hw.constant true
+  %3 = hw.struct_inject %read["repeat_0"], %true : !hw.struct<repeat: i1, repeat_0: i1>
+  // assign r3 = '{repeat_0: a.repeat_0, repeat_0_1: (1'h1)};
+  hw.output %1, %2, %3 : i1, i1, !hw.struct<repeat: i1, repeat_0: i1>
+}
+
+
 // CHECK-LABEL: module replicate
 hw.module @replicate(%arg0: i7, %arg1: i1) -> (r1: i21, r2: i9, r3: i16, r4: i16) {
   // CHECK: assign r1 = {3{arg0}};

test/Conversion/ExportVerilog/sv-interfaces.mlir

@@ -128,4 +128,10 @@ module {
     %s = hw.struct_create (%90) : !hw.struct<foo: !hw.array<384xi1>>
   }
 
+  // CHECK-LABEL: interface renameType;
+  sv.interface @renameType {
+    sv.interface.signal @data : !hw.struct<repeat: i1>
+    // CHECK-NEXT: struct packed {logic repeat_0; } data;
+  }
+
 }