Add Connectable instance for connecting ReadOnly to WriteOnly

[darius-bluespec]

No description provided.

[quark17]

I'm not sure about this instance. The interfaces don't have handshaking, so it's unclear what is an allowable way to connect them -- I'd rather not have this instance and thus require the user to manually write the rule connecting them, to force the user to know what they're doing. At the very least, I would want the rule in this instance to have the attribute fire_when_enabled. I'd also like to see no_implicit_conditions as a safety check in most uses, but I could imagine some situations where that wouldn't be satisfied, so if we put it in the instance then it wouldn't be useable in those cases.

[darius-bluespec]

I pushed an updated branch with fire_when_enabled added to the
connection rule.

I appreciate the point about lack of handshaking, however there are
already Connectable instances for RWires and various
combinations of Action, ActionValue, and Value
methods, which I think have the same concern about lack of handshaking
and/or lack of universal behavior semantics.

My thought is that there is a simple and obvious way to connect
ReadOnly to WriteOnly, which is what this change does. It may or may
not be appropriate for all situations; ReadOnly and WriteOnly are
fairly low level constructs, and ultimately it's up to the designer to
decide if this is sufficient or not.

Feel free to drop it if you don't think it belongs; it's easy enough
for me to keep a local copy in my project.

[quark17]

Connecting Action with ActionValue does have handshaking, in the EN signals. However, you're right that the connection of a with a -> Action (which I assume you mean by Value) doesn't, and I agree that it's equivalent to what you've written, and I would probably want to remove it. The RWire to RWire instance is also poor (it even uses validValue rather than pattern matching) and I would want to remove that.

Let me discuss with others and see what they think. I'm still tempted to not accept it.

If you put this in your own project, I would suggest making it a function/module that is only in scope where you need it, rather than making it a Connectable instance, which is always exported from your package. For example, instead of calling mkConnection(ro,wo) in your code, you'd call mkConnectionRW(ro,wo), say, and define that:

module mkConnectionRW#(ReadOnly ro, WriteOnly wo)(Empty);
  (* fire_when_enabled, no_implicit_conditions *)
  rule connect_rw;
    wo <= ro;
  endrule
endmodule

That's assuming that you don't want to just inline the rule wherever you'd put mkConnectionRW.

[darius-bluespec]

If I keep it in my project, I can adjust the type to be more specific
so that it isn't used beyond the intended case.

The reason I don't simply connect the wires directly is that the
package this is used in exports two modules that are intended to be
instantiated separately, possibly in unrelated hierarchy, and
connected together. The connectable instance allows the user to make
that connection somewhat easier.

[quark17]

I talked with Nikhil and I've come around to the idea (that he pointed out and which you also said) that Connectable provides one way of connecting, though some types might have multiple ways of connecting them, and if the user wants another way than what the Connectable instance provides, then the user has to go with something else. So I've come around to being OK with adding instance for ReadOnly and WriteOnly. Both Nikhil and I think the instance should have both attributes, "fire_when_enabled" and "no_implicit_conditions". The PR currently just has the first one; do you have an objection to adding the second attribute? If you add that, I'll merge it. We also figure that the other similar instances should also have those attributes, but we can do that in a later, separate PR.

[darius-bluespec]

no_implicit_conditions is fine with me. I updated the branch with
that change.