Add linebreaks to module/instance codegen

[leonardt]

No description provided.

[leonardt]

This will break magma/mantle regression tests because of the verilog code generation changes, so we should be sure to update those tests when we merge/release this

[rsetaluri]

While we're here, I think we should think a little bit more about customizability of code-generation. Someone may prefer 2 lines between statements, someone 1 line; someone may prefer all ports on one line, some on multiple; etc. The fact that someone's preference on what their output verilog looks like would cause us to update all magma/mantle golds is, to me, an indicator that we should employ some configurability.

For example llvm has this: https://clang.llvm.org/docs/ClangFormatStyleOptions.html. We don't need to think of the full configuration space right now, but some refactoring to abstract out these options would be good.

If this is a blocker, we can merge (we're blocked on magma/mantle tests anyway). Otherwise, I can work on proposing a not-too-much-work way of doing this.

[leonardt]

I don't think it's a blocker. Adding configurability seems reasonable. The main issue I see is how these configuration options are chosen. Will they be provided in the coreir frontend? (E.g. coreir -i <file>.json -o <file>.v --style indent=4,linebreaks=1) Or will they be part of verilogAST through a config file (e.g. .verilogAST-format, we'd have to do logic to look in various places like home folder, current working directory, project root directory). We could also use an environment variable.

[rsetaluri]

What I'd imagine is something like a CodeGenerator class in verilogAST which could abstract some of the common choices (like tabs, spacing, etc.) and each node could refer to when doing code generation (ideally delegate to, through functions like add_line() or something). A CodeGenerator instance would be passed from CoreIR to verilogAST when doing code-gen, so the parameters would come from CoreIR. Then, I think the easiest thing to do would be to allow specifying a format file with a command line param like --format-file my_format.yaml (can be yaml or simpler text, just like k-v pairs). If it's not specified, some default params are used.

[leonardt]

@rsetaluri this is a little stale now, I propose we just add this as the default behavior and submit configuration options as a a separate PR? I think there's plenty of higher priority issues at the moment.

[rsetaluri]

I'm fine with that but it requires changes to everyone's verilog expectations (magma/mantle/fault tests) no? Should we stage those so they're not broken for long?

[leonardt]

Yea I will update those test suites