Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sv] How to format module instantiations that fit in a single line? #53

Open
imphil opened this issue May 19, 2021 · 5 comments
Open

[sv] How to format module instantiations that fit in a single line? #53

imphil opened this issue May 19, 2021 · 5 comments

Comments

@imphil
Copy link
Contributor

imphil commented May 19, 2021

In the SystemVerilog style guide, should we require module instantiations which fit on a single line to do so? (We're trying to come down to a single way of formatting certain constructs, so this question is either-or.)

Option 1:

  and2 my_gate (.out_o(sig_output), .in1_i(first_input), .in2_i(second_input));

vs

Option 2:

  and2 my_gate (
    .out_o(sig_output),
    .in1_i(first_input),
    .in2_i(second_input)
  );

We already prefer (or allow) the use of Option 1 in other occasions, so it would be consistent with that (e.g. https://github.com/lowRISC/style-guides/blob/master/VerilogCodingStyle.md#begin--end).

I guess the main use case would be the instantiation of SV modules modeling gates, clock buffers, or other tiny modules.

@hzeller Is this something Verible has an opinion on right now?

CC @mdhayter @eunchan @msfschaffner @tomroberts-lowrisc @dsandrs @GregAC @rswarbrick @sriyerg @mwbranstad

(Split out from #48)

@sriyerg
Copy link

sriyerg commented May 19, 2021

+1 for option 1 if it fits; though, I am used to not adding any space between my_gate and ( (similar to invoking functions).

@mwbranstad
Copy link

since this is in the opinion department, I suppose can't hurt to give one. I tend to like option 2 since it seems more readable to me. Option 1 is more terse, but that reason does not necessarily make it better. So if we are looking for a value-added direction, that is my preference between the two options.

@rswarbrick
Copy link
Contributor

As Philipp wrote, we have to choose one or the other if we want a canonical formatting. I'd probably go for option 2 to avoid spurious diffs when port lists change.

@msfschaffner
Copy link
Contributor

I also lean towards option 2, since I think it is more readable.

@GregAC
Copy link
Contributor

GregAC commented May 24, 2021

I like the ability to use option 1 where it makes sense, e.g. if you're doing multiple instantiations of some simple primitive it can make it a lot more readable, e.g. so you had some mux_onehot module

mux_onehot baz_mux(.in(baz_mux_in), .out(baz_mux_out), .sel(baz_mux_sel));
mux_onehot bar_mux(.in(bar_mux_in), .out(bar_mux_out), .sel(bar_mux_sel));
mux_onehot foo_mux(.in(foo_mux_in), .out(foo_mux_out), .sel(foo_mux_sel));

But I think we want to strictly enforce it one way or the other? My example above is fairly niche and @rswarbrick makes a good point about diff stability, plus I think there's scenarios where enforcing the single line will make it less readable (imagine a bunch of instantiations that aren't all the same module so don't like up nicely like my example). So I'd probably go for always multiline if we can't allow a designer choice here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants