Unconventional mux logic

[saw235]

Instead of using the case statement for select encoding and having an a mux cell, may I know what is the reason for the unconventional way of doing the mux logic in this repository?

It is quite non-intuitive and does not guarantee the conditions to be mutually exclusive.

For example

//ifu_aln_ctl.sv line 565
assign f1pc_in[31:1] = ({31{fetch_to_f1}} & ifu_fetch_pc[31:1]) |
                       ({31{shift_f2_f1}} & f2pc[31:1]) |
                       ({31{~fetch_to_f1&~shift_f2_f1}} & sf1pc[31:1]);

[olofk]

I'm not the designer of this core, but an and-or mux like this should be cheaper if you know that certain conditions can't occur. In this case it seems to take advantage of the fact that fetch_to_f1 and shift_f2_f1 can't be asserted at the same time

[saw235]

I see, that make sense. But I feel like there should be some arguments against for maintainability purposes. And these kind of optimizations should be recognized and made by the synthesis tool/design compiler instead whenever possible.

[olofk]

I'll leave it to the core designers to give a definite answer, but my personal experience is that the tools aren't always as clever as you might expect and that sometimes you need to sacrifice readability for performance. Especially if you want a consistent implementation across different tools

[Contental]

Assuming 'fetch_to_f1' and 'shift_f2_f1' are never asserted at the same time, this is a very quick mux. That's why it's done this way. It actually is a conventional way of implementing a fast mux.

case (and casez/casex) statements have intrisic priority, the first case is evaluated first (see section 12.5 of the LRM), so this adds extra logic that may or may not be optimised out.

[Contental]

This is logged as an issue with the core. It is not an issue with the core. Such discussions should be held on another forum. Don't know where though. Any suggestions?

This issue should be closed. Who does that?

[olofk]

There is no dedicated mailing list but https://lists.chipsalliance.org/g/technical-discuss has been used to discuss other technical aspects and questions about SweRV so I recommend that for now

[saw235]

@Contental SV have usable constructs (unique/priority) to deal with this kind of ambiguity. https://www.verilogpro.com/systemverilog-unique-priority/

Interesting, this is actually the first time I have come across a mux being done this way after 3 years in the industry.