[FIRRTL][Sim] Add `fopen` and `fclose`

[SpriteOvO]

Adding fopen and fclose allows fwrite to real files instead of just standard streams.

According to SV spec 21.3.1, the return value fd of fopen is an integer. The CIRCT SV dialect does not seem to implement the integer type at the moment.

I am preparing PR to add these two system tasks. For integer, should I implement it in its entirety as a new type, or try to reuse the existing Inout type? If the former is better, that means I'll have to implement a new op like sv.assign_integer for it as well, right?

CC @seldridge @nandor @sequencer

[uenoku]

Adding fopen and fclose allows fwrite to real files instead of just standard streams.

At SV level we can already represent fopen/fclose by sv.system op (this might be slightly changed/unified with regard to SV function). Though currently there is no good way to use it from chisel.

hw.module @Fd() {
  %fd = sv.reg : !hw.inout<i32> 
  sv.initial {
    %file_name = sv.constantStr "test.log"
    %mode = sv.constantStr "r"
    %fopen = sv.system "fopen"(%file_name, %mode) : (!hw.string, !hw.string) -> i32
    sv.bpassign %fd, %fopen: i32
  }
}

// Generated by CIRCT unknown git version
module Fd();    // bar.mlir:2:1
  reg [31:0] fd;    // bar.mlir:3:9
  initial   // bar.mlir:4:3
    fd = $fopen("test.log", "r");   // bar.mlir:5:18, :6:13, :7:14, :8:5
endmodule

According to SV spec 21.3.1, the return value fd of fopen is an integer. The CIRCT SV dialect does not seem to implement the integer type at the moment.

It's not pretty but we can still use 4-state logic to store a return value of fopen/fclose as shown above (it might cause lint warning but at least VCS/verilator were happy about it). I agree that we would want to explicitly distinguish 2-state from 4-state types at SV, and actually it has been on my task list to introduce sv.bit<> type (e.g. integer could be represented as sv.bit<32>) in the context of DPI function.

I am preparing PR to add these two system tasks. For integer, should I implement it in its entirety as a new type, or try to reuse the existing Inout type? If the former is better, that means I'll have to implement a new op like sv.assign_integer for it as well, right?

I think we should reuse inout, and we don't have to introduce new assign operations. sv.assign/bpassign/passign should be sufficient. One missing piece is a way to declare an integer variable. Maybe we can abuse sv.reg and sv.logic but I think we need another operation like sv.variable which is a dedicate operation for variable declaration.

[sequencer]

My question is if there are multiple modules open a same fd, and try to write to it, would that be an issue?

If not, I'm proposing this: add fd attribute to printf to select three options: stdout, stderr, file, then declare one fd per layer, and one root fd for printf selected File but not under a layer.

This eases the API design in Chisel, that chisel no need to care about filesystem, but user can still have a fine grained logging experience.

[uenoku]

My question is if there are multiple modules open a same fd, and try to write to it, would that be an issue?

As far as I see SV spec doesn't clarify it's well defined or not. It's very simulator dependent and I have no idea how it works especially when simulation is multithread. I think that's fairly unsafe but maybe simulator is buffering properly.

If not, I'm proposing this: add fd attribute to printf to select three options: stdout, stderr, file, then declare one fd per layer, and one root fd for printf selected File but not under a layer.

I'm not sure I understand correctly but IMO chisel should be responsible for creating one fd per layer if that's goal. Maybe we could provide a way to query the current layer name (maybe as an intrinsic that returns string) but I don't think compiler should magically set fd.

[SpriteOvO]

@uenoku Thanks, that's very helpful! I'm gonna implement a new op for fopen for Chisel, and add a new optional operand for printf.

[uenoku]

I think it might be controversial to add a concept of file descriptor into chisel so you might want to ask chisel community first (if not yet). If that's ok, introducing intrinsics for fopen/fclose looks a good direction to me.

[sequencer]

I think we need to add an intrinsic first, and return a UInt, while this UInt can be feed to printf.