Open myrfy001 opened 1 month ago
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.
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?
bsc's
-remove-unused-modules
flag doesn't work withmkProbe
module provided in std library. When enable-remove-unused-modules
, the probe signal will be removed away.