Assignment missing from generated SystemVerilog

[chykon]

Describe the bug

Assignment missing from generated SystemVerilog.

To Reproduce

Use code:

import 'dart:io';
import 'package:rohd/rohd.dart';

class ExampleModule extends Module {
  ExampleModule() {
    final out = addOutput('out');
    final val = Logic(name: 'val');
    val <= Const(1);

    Combinational([
      out < val,
    ]);
  }

  Logic get out => output('out');
}

Future<void> main() async {
  final exampleModule = ExampleModule();
  await exampleModule.build();
  File('code.sv').writeAsStringSync(exampleModule.generateSynth());
  WaveDumper(exampleModule);
  print(exampleModule.out.value);
}

The console output is correct:

1'h1
Exited

But the file with SystemVerilog code:

/**
 * Generated by ROHD - www.github.com/intel/rohd
 * Generation time: 2022-09-19 17:16:56.034245
 */

module ExampleModule(
output logic out
);
logic val;

//  combinational
always_comb begin
  out = val;
end

endmodule : ExampleModule

Expected behavior

The generated SystemVerilog code has an assignment for the "val" variable.

Actual behavior

The variable "val" contains an unknown bit.

Additional details

Dart SDK version: 2.18.0 (stable) (Fri Aug 26 10:22:54 2022 +0000) on "linux_x64"

name: package

environment:
  sdk: '>=2.14.0 <3.0.0'

dependencies:
  rohd: ^0.3.2

[mkorbel1]

Thank you for reporting this! I was able to reproduce the issue and I'm looking into a root-cause and fix.

[mkorbel1]

I found a bug in the logic which collapses assignments of constants in generated SystemVerilog in certain scenarios, which when fixed makes your code example pass.

I added a test based on your example code and a fix to this pull request:
#160

You can try using this branch to resolve your issue, feel free to let us know if it works for you. Unless there's still some related issue after the fix, we will push in the fix for this soon and it will be included in the next release.

Thank you again for reporting this @chykon!

[chykon]

Yes, everything is correct now. Although constant inlining reduces the readability of SystemVerilog code. More readable would be:

module ExampleModule(
output logic out
);
logic val;

assign val = 1'h1;

//  combinational
always_comb begin
  out = val;
end

endmodule : ExampleModule

[chykon]

And one more small remark already on the formatting of the generated code. Instead of logic val; it is now just an extra empty line:

module ExampleModule(
output logic out
);
// extra line

//  combinational
always_comb begin
  out = 1'h1;
end

endmodule : ExampleModule

Perhaps it should be removed?

[mkorbel1]

Regarding the in-lining of constants and readability, it can be a matter of preference and style and it is difficult to judge algorithmically which scenarios may look more appealing and readable. This issue (#147) proposes methods for disabling the collapsing of signal names, perhaps on a per-synthesizer or per-signal basis so that the default in-lining behavior can be overridden.

The extra line is a simple fix, I'll include that one, thanks for pointing it out.

In general, the upcoming CIRCT (https://github.com/llvm/circt) Synthesizer will probably produce substantially more beautiful SystemVerilog once it's ready (both CIRCT and the Synthesizer). The CIRCT project still has some work to do in terms of supporting basic SystemVerilog functionality (e.g. some types of case statements llvm/circt#2908) and also can produce some weird looking SystemVerilog at times, but they are approaching the problem with pretty generation and scalability in mind. The ROHD SystemVerilog Synthesizer does a pretty good job of keeping everything structurally similar with just some intermediate signal name and assignment collapsing, but there's some room for improvement in making the code prettier.