-remove-unused-modules flag doesn't work with mkProbe module provided in std library

[myrfy001]

bsc's -remove-unused-modules flag doesn't work with mkProbe module provided in std library. When enable -remove-unused-modules, the probe signal will be removed away.

[quark17]

Thank you for reporting this.

I don't know how people are using probes or the flag, so I don't know if some people want the flag to remove the probes -- say, if they're applying the flag when they are ready for production. If someone wants to remove probes under some circumstances, then they could probably implement that using macros or conditionals. So it's probably safe to change the behavior, to preserve probes.

If you're able to attach an example in the future, that would be helpful. I made the following example, which tests both the Probe and ProbeWire libraries:

import Probe::*;
import ProbeWire::*;

interface Ifc;
  method Bool m(Bit#(8) v);
endinterface

(* synthesize *)
module mkTest (Ifc);

  // Test Probe

  Reg#(Bit#(8)) rgA <- mkRegU;
  Reg#(Bit#(8)) rgB <- mkRegU;
  Probe#(Bit#(8)) p <- mkProbe;

  rule rl;
    p <= rgA + rgB;
    rgA <= rgA + 1;
  endrule

  // Test ProbeWire

  Reg#(Bit#(8)) rgC <- mkRegU;
  Reg#(Bit#(8)) rgD <- mkRegU;
  ProbeWire#(Bit#(8)) pw <- mkProbeWire;

  method Bool m(Bit#(8) v);
    let x = pw.id(rgC - v);
    return (x < rgD);
  endmethod

endmodule

If compiled with just -verilog, you get this output in the generated Verilog:

  // probes
  wire [7 : 0] p$PROBE;
  wire p$PROBE_VALID;

  // ports of submodule pw
  wire [7 : 0] pw$IN, pw$OUT;

  ...

  // probes
  assign p$PROBE = rgA + rgB ;
  assign p$PROBE_VALID = 1'd1 ;

  // submodule pw
  assign pw$IN = rgC - m_v ;

And if you provide the -remove-unused-modules flag, then the Probe is removed and the ProbeWire is not.

The ProbeWire was introduced for probing an expression in a value method, where you're not able to write to a Probe because it's not an action context. It doesn't appear to be supported in the same way that Probe is, and I think that's something that should be fixed. For example, the ProbeWire is not listed under the probes comment heading, but is instead just an ordinary submodule that appears under its own heading. The ProbeWire has an output, so it doesn't need special handling by BSC to keep it in the generated Verilog, and can therefore be an ordinary module; that also means that it isn't removed when the -remove-unused-modules flag is given.

There are two stages in BSC where the -remove-unused-modules flag is consulted: VFinalCleanup.hs and AVerilog.hs (corresponding to the stage names finalcleanup and verilog if you use the -v or -d flags). These can be seen by looking for removeUnusedMods in src/comp/. It appears that the Probe is being removed in the finalcleanup stage, even before getting to verilog (which does the final conversion into Verilog).

To keep probes even in the presence of the flag, we would need to fix both of those stages.

Some other observations:

I notice that the AVerilog code looks for the hardcoded string "Probe" as the module name. This could probably be cleaned up. (At the very least, put the hardcoded string in PreStrings. But it might be better to check that this is the Probe module from the Probes library, and that information is lost by AVerilog, I think; so maybe have an earlier stage check this and record the info somehow, as a separate field for probes or an ID property.)

I notice that there is an ID property IdPProbe defined in Id.hs, but it seems unused, so it could be removed (or repurposed). The AConv.hs stage does use it -- it applies the property to every submodule instantiation, I think -- but then nothing ever looks for that property (and since it's applied to every instantiation, it doesn't seem meaningful). It was one of the original five properties when IdPProp was added in 2004, when BSC was being productized, and was probably intended for developer debugging, to see where Ids propagate to? It's use in AConv was around that time and looks like it was just accidentally included in a commit.

[myrfy001]

My apologisze. When I first try mkProbe, I was on a project which enabled -remove-unused-modules, and it caused me half a day to find why it doesn't work, so when I found it out I think it was a bug. But you idea that when doing release build, this flag can be used to remove probe from sourcee code, which also make sense. And I think using a compiler flag to remove probe is more better than using macros(which is verbose).

But I do think the name of remove-unused-modules is misleading when working with mkProbe since I'm really using it. So, maybe another flag like -keep-probes together with -remove-unused-modules is better solution?