From b0fa293c96225f7faddb020a24b71cc2c6a75ed4 Mon Sep 17 00:00:00 2001 From: rsetaluri Date: Thu, 13 Apr 2023 11:24:45 -0700 Subject: [PATCH] [MLIR] Add support for Tuples with integer keys (#1260) * [MLIR] Prefix tuple field keys with _ See #1252. * [MLIR] Factor out magma tuple key to string logic --- magma/backend/mlir/hardware_module.py | 14 ++++++++++---- magma/backend/mlir/magma_common.py | 11 ++++++++++- .../test_mlir/golds/simple_aggregates_tuple.mlir | 10 +++++----- .../test_mlir/golds/simple_aggregates_tuple.v | 9 +++++++++ .../test_compile_to_mlir_local_examples.py | 5 ----- 5 files changed, 34 insertions(+), 15 deletions(-) create mode 100644 tests/test_backend/test_mlir/golds/simple_aggregates_tuple.v diff --git a/magma/backend/mlir/hardware_module.py b/magma/backend/mlir/hardware_module.py index 6dd3343ef..9f39739ea 100644 --- a/magma/backend/mlir/hardware_module.py +++ b/magma/backend/mlir/hardware_module.py @@ -25,6 +25,7 @@ value_or_type_to_string as magma_value_or_type_to_string, visit_value_or_value_wrapper_by_direction as visit_magma_value_or_value_wrapper_by_direction, + tuple_key_to_str as magma_tuple_key_to_str, ) from magma.backend.mlir.mem_utils import ( make_mem_reg, @@ -139,9 +140,11 @@ def magma_type_to_mlir_type(type: Kind) -> MlirType: return magma_type_to_mlir_type(Bits[type.N]) return hw.ArrayType((type.N,), magma_type_to_mlir_type(type.T)) if issubclass(type, m_Tuple): - fields = {str(k): magma_type_to_mlir_type(t) - for k, t in type.field_dict.items()} - return hw.StructType(tuple(fields.items())) + fields = ( + (magma_tuple_key_to_str(k, type), magma_type_to_mlir_type(t)) + for k, t in type.field_dict.items() + ) + return hw.StructType(tuple(fields)) @wrap_with_not_implemented_error @@ -969,7 +972,10 @@ def visit_instance_wrapper(self, module: ModuleWrapper) -> bool: ) return True if inst_wrapper.name.startswith("magma_tuple_get_op"): - index = inst_wrapper.attrs["index"] + index = magma_tuple_key_to_str( + inst_wrapper.attrs["index"], + inst_wrapper.attrs["T"] + ) hw.StructExtractOp( field=index, operands=module.operands, diff --git a/magma/backend/mlir/magma_common.py b/magma/backend/mlir/magma_common.py index 25e5371c8..b29c7f36a 100644 --- a/magma/backend/mlir/magma_common.py +++ b/magma/backend/mlir/magma_common.py @@ -8,7 +8,7 @@ from magma.common import replace_all, Stack, SimpleCounter from magma.ref import Ref, ArrayRef, TupleRef from magma.t import Kind, Type -from magma.tuple import TupleMeta, Tuple as m_Tuple +from magma.tuple import TupleMeta, Tuple as m_Tuple, Product ModuleLike = Union[DefineCircuitKind, Circuit] @@ -42,6 +42,15 @@ def value_or_type_to_string(value_or_type: Union[Type, Kind]): return replace_all(s, _VALUE_OR_TYPE_TO_STRING_REPLACEMENTS) +def tuple_key_to_str(key: Union[str, int], T: TupleMeta) -> str: + # NOTE(rsetaluri): If the type is Tuple (i.e. has integer keys), then the + # struct type we emit should prefix the field key with an underscore + # (e.g. "_0") to make it a valid keyword. + if issubclass(T, Product): + return str(key) + return f"_{key}" + + @dataclasses.dataclass(frozen=True) class ValueWrapper: id: str = dataclasses.field(default_factory=make_unique_name, init=False) diff --git a/tests/test_backend/test_mlir/golds/simple_aggregates_tuple.mlir b/tests/test_backend/test_mlir/golds/simple_aggregates_tuple.mlir index 1545b1e75..5fee293e5 100644 --- a/tests/test_backend/test_mlir/golds/simple_aggregates_tuple.mlir +++ b/tests/test_backend/test_mlir/golds/simple_aggregates_tuple.mlir @@ -1,11 +1,11 @@ module attributes {circt.loweringOptions = "locationInfoStyle=none"} { - hw.module @simple_aggregates_tuple(%a: !hw.struct<0: i8, 1: i8>) -> (y: !hw.struct<0: i8, 1: i8>) { - %0 = hw.struct_extract %a["0"] : !hw.struct<0: i8, 1: i8> + hw.module @simple_aggregates_tuple(%a: !hw.struct<_0: i8, _1: i8>) -> (y: !hw.struct<_0: i8, _1: i8>) { + %0 = hw.struct_extract %a["_0"] : !hw.struct<_0: i8, _1: i8> %2 = hw.constant -1 : i8 %1 = comb.xor %2, %0 : i8 - %3 = hw.struct_extract %a["1"] : !hw.struct<0: i8, 1: i8> + %3 = hw.struct_extract %a["_1"] : !hw.struct<_0: i8, _1: i8> %4 = comb.xor %2, %3 : i8 - %5 = hw.struct_create (%1, %4) : !hw.struct<0: i8, 1: i8> - hw.output %5 : !hw.struct<0: i8, 1: i8> + %5 = hw.struct_create (%1, %4) : !hw.struct<_0: i8, _1: i8> + hw.output %5 : !hw.struct<_0: i8, _1: i8> } } diff --git a/tests/test_backend/test_mlir/golds/simple_aggregates_tuple.v b/tests/test_backend/test_mlir/golds/simple_aggregates_tuple.v new file mode 100644 index 000000000..c2a1e3cb7 --- /dev/null +++ b/tests/test_backend/test_mlir/golds/simple_aggregates_tuple.v @@ -0,0 +1,9 @@ +// Generated by CIRCT circtorg-0.0.0-1773-g7abbc4313 +module simple_aggregates_tuple( + input struct packed {logic [7:0] _0; logic [7:0] _1; } a, + output struct packed {logic [7:0] _0; logic [7:0] _1; } y +); + + assign y = '{_0: (~a._0), _1: (~a._1)}; +endmodule + diff --git a/tests/test_backend/test_mlir/test_compile_to_mlir_local_examples.py b/tests/test_backend/test_mlir/test_compile_to_mlir_local_examples.py index 7d0a3aacd..cc12f7653 100644 --- a/tests/test_backend/test_mlir/test_compile_to_mlir_local_examples.py +++ b/tests/test_backend/test_mlir/test_compile_to_mlir_local_examples.py @@ -43,11 +43,6 @@ def test_compile_to_mlir(ckt): "use_native_bind_processor": True, "basename": ckt.name, } - # NOTE(rsetaluri): This is a hack to skip the Tuple[] to verilog test since - # MLIR does not allow leading integers in names (e.g. `!hw.struct<0: - # ...>`). We should ultimately fix this by changing the IR code generation. - if ckt.name == "simple_aggregates_tuple": - kwargs["check_verilog"] = False run_test_compile_to_mlir(ckt, **kwargs)