[SimToSV][HWToSV] Print-like SV operations are not guarded from synthesis

[nanjo712]

The lowering path in SimToSV that emits sv.fwrite / sv.write does not currently make sure that these operations are protected from synthesis.

This is problematic because these operations are simulation-only side effects. They should probably be emitted under the same synthesis guard mechanism used for other non-synthesizable SV constructs.

Run

circt-opt --lower-sim-to-sv --lower-hw-to-sv test/Conversion/SimToSV/lower-print-formatted-proc-to-sv.mlir 

Then the part of output would be:

...
hw.module @print_to_literal_file(in %clk : i1) attributes {emit.fragments = [@CIRCT_LIB_LOGGING_FRAGMENT]} {
  sv.always posedge %clk {
    %0 = sv.constantStr "static.log"
    %1 = sv.func.call.procedural @"__circt_lib_logging::FileDescriptor::get"(%0) : (!hw.string) -> i32
    sv.fwrite %1, "value"
  }
  hw.output
}
...

The generated sv.fwrite is emitted directly in the procedural region, without a synthesis guard around it.

[nanjo712]

I noticed this while working on the Sim/SV lowering path, but I don't currently have bandwidth to fix it immediately. Recording it separately so it does not get lost.

[seldridge]

Note that, similar to John's issue with asserts, it's not clear to me that this is something that should happen automatically.

I'm much more in favor of just emitting exactly what is in the IR. If the user guards it with an ifdef, then emit that, but don't do it automatically. We have no real information on how any given non-synthesizable construct is being used. Additionally, there is no consensus on what is sunthesizable. This was attempted to be formalized in IEEE 1364.1, but was dropped and doesn't apply to IEEE 1800 (as far as I know).

Certain flows do support synthesizing prints (e.g., FireSim or other FPGA flows) and almost any "non-synthesizable" construct could be given the same treatment. Forcing these to be guarded only makes those flows harder to develop.

That said, I am fine with an optional pass that does this, like what John did for asserts!

[nanjo712]

Thanks, that distinction makes sense to me. I agree that the generic SimToSV/SV emission path should probably emit what is represented in the IR and avoid making an implicit policy decision about whether a construct should be considered non-synthesizable.

My concern is specifically about preserving the existing FIRRTL/Chisel default lowering behavior. Today, FIRRTL prints lowered through the direct SV path are guarded from synthesis. If we route FIRRTL printing through Sim instead, I think that policy should be applied explicitly by the FIRRTL lowering pipeline rather than being baked into generic SimToSV lowering.

Would it make sense to add an opt-in pass for this, and have the FIRRTL default pipeline run it? That would keep SimToSV policy-free while still preserving the current behavior for the default FIRRTL path. Flows that want to synthesize or otherwise consume prints, such as FireSim-like FPGA flows, could then simply avoid that pass.

[seldridge]

My concern is specifically about preserving the existing FIRRTL/Chisel default lowering behavior. Today, FIRRTL prints lowered through the direct SV path are guarded from synthesis. If we route FIRRTL printing through Sim instead, I think that policy should be applied explicitly by the FIRRTL lowering pipeline rather than being baked into generic SimToSV lowering.

I've been thinking that we could actually just remove this. Once Chisel started automatically moving printf into layers [1], the need for the SYNTHESIS guarding is basically gone.

But, yes, I'd much prefer to have the FIRRTL to HW conversion deal with handling the "FIRRTL ABI" and not have that leak into later passe that are supposed to be "core CIRCT". Layers go one step beyond this by removing the need for a FIRRTL ABI here and allowing the front end language user to specify whatever they want (with defaults that the language sets).

How about we just drop the guarding and see what happens? 😉 If you PR something, I can try running an internal build and see if anything breaks. We have some flows that print out logs and then consume those with downstream tools. I think those will be sufficient to check this. If things break, we can extend John's pass (#10479) to handle more things. This will also cover the case of wanting to compile old FIRRTL with new CIRCT. (This is rare, but does come up sometimes and could affect @TaoBi22.)

Thank you for your attention to existing users! I try to jump in when I know something can be dropped, though.

@prithayan: Do you know if we have any internal flows which rely on matching the SYNTHESIS guards around these operations?