[FIRRTL] Make input probes illegal #6921
Conversation
|
Not sure about this direction (thoroughly ripping this out from all the things), but leaving here for now. |
dtzSiFive
force-pushed
the
feature/no-input-probes
branch
from
ab62cc8
to
7ef0d02
Compare
dtzSiFive
force-pushed
the
feature/no-input-probes
branch
from
7ef0d02
to
b46e105
Compare
| // expected-note @below {{width is constrained to be at least 2 here:}} | ||
| firrtl.ref.define %inst2_ref, %w2_ref : !firrtl.rwprobe<uint> | ||
| } | ||
| } |
There was a problem hiding this comment.
Can this test be rewritten to an output port to still cover the issue that this was added to fix. (Or is this covered by another test?)
There was a problem hiding this comment.
We need multiple drivers for the same static (in the IR, like a specific wire or port). Since probe's must be driven exactly once this is a bit tricky to test.
An input probe driven in different instantiation contexts with different widths is clearest way I know to do this (this is test I had commented out earlier for same "seems like a good thing to have under test still" thinking).
( #6921 (comment) )
I had made a comment previously about instance-choice (if has an output probe of unknown width, in theory the two choices might try to infer to different widths based on their contents), but I'm not sure story there re:unknown widths and interaction with inferences.
So, not sure how to keep this as a test but that would be nice, agreed, thanks!
dtzSiFive
force-pushed
the
feature/no-input-probes
branch
from
b46e105
to
4f71430
Compare
Per reviewer feedback, thanks!
dtzSiFive
force-pushed
the
feature/no-input-probes
branch
from
4f71430
to
b5d2f03
Compare
Disallow input probes.
cc chipsalliance/firrtl-spec#183 .
Adds verifier for this, and mostly updates tests.
Delete ProbeDCE.
Many passes can (and should) be simplified if they do not need to consider this.
Incomplete list: