Which repo should I write this bug for? (error when compiling firrtl: error: 'module' must be first token on its line)

[JL102]

Hi,

It seems that if you instantiate a module with the name "module", the Chisel->FIRRTL step has no issues with that, but then I assume the FIRRTL->Verilog step is what's giving me the error error: 'module' must be first token on its line.

class WithMessageQueue(queueLength: Int, opcodes: OpcodeSet = OpcodeSet.custom0) extends Config((site, here, up) => {
	case BuildRoCC => Seq((p: Parameters) => {                                                                                           
    val module = LazyModule( new MessageQueue(OpcodeSet.custom0)(p))
    module                                                                                                      
  })
})

It outputs to this line in the FIRRTL:

inst module of MessageQueue @[MessageQueue.scala 51:28]

Since FIRRTL doesn't like an instance to be named module, I think the Chisel -> FIRRTL step should consider "module" to be a reserved word, and throw a user error saying something like "You can't name an instance module". What repository should I put this issue in? https://github.com/chipsalliance/chisel or somewhere else?

[milovanovic]

Yes, I believe Chisel would be the right place to raise the issue you are describing. Bare in mind that currently FIRRTL compiler is in maintenance mode and soon it will be deprecated in Chipyard too, therefore you might want to confirm the next generation CIRCT behavior before alerting the Chisel guys.

[JL102]

Thanks for the response.
Regarding the second statement: I thought that CIRCT was a project that includes FIRRTL, i.e., that we can still compile Chisel into .fir files and from .fir into Verilog. Am I wrong? I would've assumed that there wouldn't be any major changes to the FIRRTL language (e.g. reserved words like "module").

Is there a way for me to test this in a smaller development environment?

[milovanovic]

As far as I understand it FIRRTL is the term used interchangeably for both the intermediate representation (IR) language and also for its Scala compiler, hence two separate firrtl repos. So the FIRRTL language spec of course remains, whereas the Scala compiler gets deprecated in the process of migration towards CIRCT. Anyhow, I guess both Chisel and FIRRTL spec are good places for your issue.

[JL102]

After looking into it more, it looks like it is actually an issue with CIRCT's implementation of FIRRTL - or perhaps with the spec, for not specifying this edge case. If I put the following code into the Chisel Bootcamp notebook, and run getFirrtl() and getVerilog(), it has no issues: in fact, it renames module to module_.

class Passthrough extends Module {
  val io = IO(new Bundle {
    val in = Input(UInt(4.W))
    val out = Output(UInt(4.W))
  })
  io.out := io.in
}

class Passthrough2 extends Module {
  val io = IO(new Bundle {
    val in = Input(UInt(4.W))
    val out = Output(UInt(4.W))
  })
    
    val module = Module(new Passthrough())
    module.io.in := io.in
    io.out := module.io.out
}

Verilog output:

module Passthrough(
  input  [3:0] io_in,
  output [3:0] io_out
);
  assign io_out = io_in; // @[cmd18.sc 6:10]
endmodule
module Passthrough2(
  input        clock,
  input        reset,
  input  [3:0] io_in,
  output [3:0] io_out
);
  wire [3:0] module__io_in; // @[cmd18.sc 15:24]
  wire [3:0] module__io_out; // @[cmd18.sc 15:24]
  Passthrough module_ ( // @[cmd18.sc 15:24]
    .io_in(module__io_in),
    .io_out(module__io_out)
  );
  assign io_out = module__io_out; // @[cmd18.sc 17:12]
  assign module__io_in = io_in; // @[cmd18.sc 16:18]
endmodule

After a short search, I don't see anything in the spec about reserved words or adding an underscore to the name to an instance. So I'll add a bug report to the FIRRTL spec repo.

[JL102]

chipsalliance/firrtl-spec#115

[JL102]

Forgot to add: Thanks for the info @milovanovic !