circt
circt copied to clipboard
llvm
[FIRRTL] Remove support for printf-encoded verification, tests.
Add small test for load-bearing "UNR" behavior especially interaction with --add-companion-assume.
LGTM! What happens if someone feeds a FIR file with some older FIR version? Do they get the deprecation error added by that other PR?
No, they get what their FIRRTL spec says they get, a printf. FIRRTL version is irrelevant to whether printf-encoding is supported as it was always a hack that should not have been made load-bearing.
However, breaking compatibility with what Chisel produced in the past (ancient or recent) is not something we have a story for, and this is core enough that dropping this means breaking compatibility with quite a long history of FIRRTL.
Bummer.
I'm not sure maintaining the ability to parse these things /without/ the ability to generate the corresponding IR is worth bothering with, so I think this means this lives on indefinitely.
Hmmm good point, it's not actually in the spec. It just happened to work, like an intrinsic without any clear definition of what it does. It might be worthwhile to just rip off the bandaid, and drop support for this from firtool. Up-to-date versions of Chisel will keep working, and everything else breaks.
Or we could allocate some grace period while this still works with a warning, and then pull the plug in say 3 months? Would be a shame to not delete old stale code.
Printf-encoded verification ops were always a SiFive-internal hack. If this got hop-on external users, they knew what they were getting into (also a user sufficiently motivated to reverse engineer these printf-encoded assertions is likely still motivated to use the documented replacements).
Drop 'em. I see no risk here.
Printf-encoded verification ops were always a SiFive-internal hack
Note that chisel assert has been also encoded as printf so it could be user-facing for external users.
Created #7030 to run with the idea of removing support but retaining classification/recognition to diagnose unsupported use of these printf's, FWIW.
PR updated to remove the rest not already removed: tests and the detection + error introduced with https://github.com/llvm/circt/pull/7030 .
Not rejecting Chisel-produced variants has at least some related/reasonable behavior (printf, stop, assert -- they'll still generally do the right thing), but the other flavors produced by rocketchip or similar (e.g., "this is really a cover" in printf means assert is a cover) won't work and designs built on this will be broken/wrong.