From b05fe5303ed17e668c6ec2ec3558cd5a52eff787 Mon Sep 17 00:00:00 2001 From: rsetaluri Date: Tue, 8 Aug 2023 11:53:05 -0700 Subject: [PATCH] [MLIR] Make minor fixes and improvements (#1299) * Adds a tool to convert an MLIR file to Verilog with the same options we have in the top down flow. Useful for debugging (e.g. hand-modify .mlir file, recompile to Verilog). * Always checks Verilog compilation for backend tests by default. Previously, this was turned off since (a) CI didn't always have `circt` available; (b) `circt-opt` wasn't standardized; and (c) running `circt-opt` was slow. Now all of these issues are addressed with v2.3.0. --- magma/backend/mlir/mlir_to_verilog.py | 5 +- .../test_mlir/golds/register_array_of_bit.v | 2 +- .../test_mlir/test_mlir_to_verilog.py | 60 ++++++++++----- tests/test_backend/test_mlir/test_utils.py | 2 +- tests/test_tools/test_mlir_to_verilog_main.py | 19 +++++ tools/mlir_to_verilog_main.py | 74 +++++++++++++++++++ 6 files changed, 139 insertions(+), 23 deletions(-) create mode 100644 tests/test_tools/test_mlir_to_verilog_main.py create mode 100644 tools/mlir_to_verilog_main.py diff --git a/magma/backend/mlir/mlir_to_verilog.py b/magma/backend/mlir/mlir_to_verilog.py index e99c2ef0a..7a7791d96 100644 --- a/magma/backend/mlir/mlir_to_verilog.py +++ b/magma/backend/mlir/mlir_to_verilog.py @@ -1,5 +1,6 @@ import dataclasses import io +from typing import Optional import circt import circt.passmanager @@ -10,6 +11,7 @@ @dataclasses.dataclass class MlirToVerilogOpts: split_verilog: bool = False + split_verilog_directory: Optional[str] = None def mlir_to_verilog( @@ -33,6 +35,7 @@ def mlir_to_verilog( ) pm.run(module.operation) if opts.split_verilog: - circt.export_split_verilog(module, "") + directory = opts.split_verilog_directory or "." + circt.export_split_verilog(module, directory) else: circt.export_verilog(module, ostream) diff --git a/tests/test_backend/test_mlir/golds/register_array_of_bit.v b/tests/test_backend/test_mlir/golds/register_array_of_bit.v index 669cdaef2..cca25560d 100644 --- a/tests/test_backend/test_mlir/golds/register_array_of_bit.v +++ b/tests/test_backend/test_mlir/golds/register_array_of_bit.v @@ -1,4 +1,4 @@ -// Generated by CIRCT circtorg-0.0.0-2220-ge81df61a7 +// Generated by CIRCT firtool-1.48.0-34-g7018fb13b module register_array_of_bit( input [3:0] I, input CLK, diff --git a/tests/test_backend/test_mlir/test_mlir_to_verilog.py b/tests/test_backend/test_mlir/test_mlir_to_verilog.py index 497a497af..6fb6e2016 100644 --- a/tests/test_backend/test_mlir/test_mlir_to_verilog.py +++ b/tests/test_backend/test_mlir/test_mlir_to_verilog.py @@ -1,5 +1,8 @@ import io +import pathlib import pytest +import tempfile +import textwrap from typing import Optional import circt @@ -43,7 +46,7 @@ def test_bad_input(): @pytest.mark.parametrize("style", ("plain", "wrapInAtSquareBracket", "none")) def test_location_info_style(style): - ir = ( + ir = textwrap.dedent( """ module attributes {{circt.loweringOptions = "locationInfoStyle={style}"}} {{ hw.module @M() -> () {{}} @@ -57,16 +60,16 @@ def test_location_info_style(style): line = ostream.readline().rstrip() expected = "module M();" if style == "plain": - expected += " // -:3:11" + expected += " // -:3:3" elif style == "wrapInAtSquareBracket": - expected += " // @[-:3:11]" + expected += " // @[-:3:3]" assert line == expected @pytest.mark.parametrize("explicit_bitcast", (False, True)) def test_explicit_bitcast(explicit_bitcast): explicit_bitcast_attr = ",explicitBitcast" if explicit_bitcast else "" - ir = ( + ir = textwrap.dedent( """ module attributes {{circt.loweringOptions = "locationInfoStyle=none{explicit_bitcast_attr}"}} {{ hw.module @M(%a: i8, %b: i8) -> (y: i8) {{ @@ -98,7 +101,7 @@ def test_disallow_expression_inlining_in_ports(disallow_expression_inlining_in_p else "" ) - ir = ( + ir = textwrap.dedent( """ module attributes {{circt.loweringOptions = "locationInfoStyle=none{disallow_expression_inlining_in_ports_attr}"}} {{ hw.module.extern @Foo(%I: i1) -> (O: i1) @@ -137,7 +140,7 @@ def test_omit_version_comment(omit_version_comment): else "" ) - ir = ( + ir = textwrap.dedent( """ module attributes {{circt.loweringOptions = "locationInfoStyle=none{omit_version_comment_attr}"}} {{ hw.module @M() -> () {{}} @@ -156,22 +159,39 @@ def test_omit_version_comment(omit_version_comment): assert first.startswith("// Generated by") -def test_split_verilog(): - ir = ( +@pytest.mark.parametrize("specify_output_file", (False, True)) +def test_split_verilog(specify_output_file): + ir = textwrap.dedent( """ module attributes {{circt.loweringOptions = "locationInfoStyle=none"}} {{ - hw.module @M() -> () attributes {{output_file = {output_file}}} {{}} + hw.module @M() -> () {attribute_string} {{}} }} """ ) - output_file = "tests/test_backend/test_mlir/build/test_mlir_to_verilog_split_verilog.sv" - ir = ir.format(output_file=f"#hw.output_file<\"{output_file}\">") - _, ostream = _run_test(ir, split_verilog=True) - ostream.seek(0) - assert not ostream.readline() # output expected to be empty - - # Now read ostream from the expcted output file. - ostream = open(output_file) - ostream.readline() # skip header - assert ostream.readline().rstrip() == "module M();" - assert ostream.readline().rstrip() == "endmodule" + with tempfile.TemporaryDirectory() as tempdir: + output_file = f"{tempdir}/outfile.sv" + if specify_output_file: + attribute_string = ( + f"attributes " + f"{{output_file = #hw.output_file<\"{output_file}\">}}" + ) + else: + attribute_string = "" + ir = ir.format(attribute_string=attribute_string) + opts = {"split_verilog": True} + if not specify_output_file: + opts["split_verilog_directory"] = tempdir + _, ostream = _run_test(ir, **opts) + ostream.seek(0) + # We expect the output to be empty due to split verilog. + assert not ostream.readline() + + # Now read ostream from the expcted output file. If the output file is + # not specificed explicitly, then it goes into /.sv (in this case, /M.sv). + if not specify_output_file: + output_file = f"{tempdir}/M.sv" + with open(output_file, "r") as ostream: + ostream.readline() # skip header + assert ostream.readline().rstrip() == "module M();" + assert ostream.readline().rstrip() == "endmodule" diff --git a/tests/test_backend/test_mlir/test_utils.py b/tests/test_backend/test_mlir/test_utils.py index b3e55f060..9a14baa3d 100644 --- a/tests/test_backend/test_mlir/test_utils.py +++ b/tests/test_backend/test_mlir/test_utils.py @@ -25,7 +25,7 @@ config._register( test_mlir_check_verilog=EnvConfig( - "TEST_MLIR_CHECK_VERILOG", False, bool)) + "TEST_MLIR_CHECK_VERILOG", True, bool)) config._register( test_mlir_write_output_files=EnvConfig( "TEST_MLIR_WRITE_OUTPUT_FILES", False, bool)) diff --git a/tests/test_tools/test_mlir_to_verilog_main.py b/tests/test_tools/test_mlir_to_verilog_main.py new file mode 100644 index 000000000..6c7b2a226 --- /dev/null +++ b/tests/test_tools/test_mlir_to_verilog_main.py @@ -0,0 +1,19 @@ +import pathlib +import tempfile +import textwrap + +from tools.mlir_to_verilog_main import main + + +def test_basic(): + + with tempfile.TemporaryDirectory() as tempdir: + tempdir = pathlib.Path(tempdir) + infile = tempdir / "infile.mlir" + with open(infile, "w") as f: + f.write("module {}\n") + outfile = tempdir / "outfile.v" + main([str(infile), "--outfile", str(outfile)]) + assert outfile.is_file() + + diff --git a/tools/mlir_to_verilog_main.py b/tools/mlir_to_verilog_main.py new file mode 100644 index 000000000..3cffb21c9 --- /dev/null +++ b/tools/mlir_to_verilog_main.py @@ -0,0 +1,74 @@ +import argparse +import dataclasses +import logging +import os +import sys +from typing import Dict + +from magma.backend.mlir.mlir_to_verilog import mlir_to_verilog, MlirToVerilogOpts +from magma.common import slice_opts + + +logging.basicConfig(level=os.environ.get("LOGLEVEL", "WARNING").upper()) + + +def _field_to_argument_params(field: dataclasses.Field) -> Dict: + if field.default_factory is not dataclasses.MISSING: + raise TypeError(field) + params = {} + params["required"] = field.default is dataclasses.MISSING + if field.type is bool and not params["required"] and not field.default: + params["action"] = "store_true" + return params + if not params["required"]: + params["default"] = field.default + params["action"] = "store" + params["type"] = field.type + return params + + +def _add_dataclass_arguments(parser: argparse.ArgumentParser, cls: type): + assert dataclasses.is_dataclass(cls) + for field in dataclasses.fields(cls): + params = _field_to_argument_params(field) + parser.add_argument(f"--{field.name}", **params) + + +def main(prog_args = None) -> int: + parser = argparse.ArgumentParser( + "Compile a (MLIR) .mlir file to verilog (.v/.sv)" + ) + + parser.add_argument( + "infile", + metavar="", + action="store", + type=str, + help="Input MLIR file", + ) + parser.add_argument( + "--outfile", + metavar="", + action="store", + type=argparse.FileType("w"), + required=False, + default=sys.stdout, + ) + _add_dataclass_arguments(parser, MlirToVerilogOpts) + + args = parser.parse_args(prog_args) + opts = slice_opts(vars(args), MlirToVerilogOpts) + + logging.debug(f"Running with opts: {opts}") + if opts.split_verilog and args.outfile is not sys.stdout: + logging.warning( + f"outfile ({args.outfile.name}) ignored with split_verilog enabled" + ) + + with open(args.infile, "r") as f_in: + mlir_to_verilog(f_in, args.outfile, opts) + + +if __name__ == "__main__": + exit(main()) +