Parameter/attribute type encoding in protobuf backend

[daveshah1]

Consider:

(* blackbox *)
module submod(output q);
parameter [7:0] a = 0;
parameter [7:0] b = 0;
parameter [71:0] c = 0;
parameter [72*8-1:0] d = 0;
endmodule

module top(output q);
	submod #(
		.a(8'b11110000),
		.b(8'b1111xxxx),
		.c(72'h1),
		.d("000000000000000000000000000000000000000000000000000000000000000000000001")
	) submod_i(
		.q(q)
	);
endmodule

running yosys -p "proc; hierarchy; write_protobuf -text" ptypes.v gives (abridged):

        parameter {
          key: "a"
          value {
            int: 240
          }
        }
        parameter {
          key: "b"
          value {
            int: 240
          }
        }
        parameter {
          key: "c"
          value {
            str: "000000000000000000000000000000000000000000000000000000000000000000000001"
          }
        }
        parameter {
          key: "d"
          value {
            str: "000000000000000000000000000000000000000000000000000000000000000000000001"
          }
        }

Clearly a and b; and c and d; should not be encoded identically.

As I am interested in protobuf or equivalent being a JSON replacement for nextpnr; at least for more advanced flows with hierarchy; it would be good to sort this out properly and avoid the pain we had fixing this in the JSON flow.

A proposal would be to add an explicit type message that stores width, signedness and stringness; and also ensure that any value with 'x' bits is not converted to an integer.

Any other thoughts before I implement something like this?

[whitequark]

This is only somewhat related, but have you looked at flatbuffers?

[daveshah1]

It would certainly be an option. But I'm not sure if we should be maintaining both flatbuffer and protobuf front/backends for Yosys, so it would be good to decide one way or another.

[whitequark]

AFAIK it is technically superior to protobuf in essentially every way, so it is a matter of there being enough desire to switch.

[daveshah1]

I would certainly be keen to see this decided before I work on the nextpnr side. I have no objections to switching - and making the change on the Yosys side - but I've never used either option "in anger". Any comments @q3k, given you implemented the protobuf backend in the first place?

[mithro]

I would recommend Cap'n'Proto over flatbuffers. After doing an assessment of ProtoBuf, FlatBuffers and Cap'n'Proto we decided to use Cap'n'Proto in VtR.

FYI - Support both Protobuf and Cap'n'Proto could make sense, they have reasonably different use cases. Protobuf is more efficient in the data transmission side (for things like RPCs), while Cap'n'Proto (and flatbuffers) can be memory mapped directly without any decoding overhead.

[q3k]

AFAIK it is technically superior to protobuf in essentially every way, so it is a matter of there being enough desire to switch.

Not really, but I think that's outside the scope for this discussion.

However, I do agree for some usecases (like having zero copy) Cap'n'Proto/Flatbuffers are a better choice than Protobufs. I don't have a strong personal preference regarding changing to or adding a different proto-style format. I also don't think there are any large consumers of the current Protobuf format, so it's probably okay to break backwards compatibility in a sane way (eg. by changing tag numbers so existing consumers don't mis-decode data, and instead find the design empty).

The reason the current protobuf backend type system is dodgy is that I couldn't come up with a universal way to pack arbitrary width integers, and kept the same mindset as the JSON backend did - deciding on the emitted type based on the bit width of the incoming number. An alternative would be to always ship these as a message Value { int32 nbits; bytes packed; } with a well defined endianness and padding scheme, but that would make writing consumer code somewhat annoying. And still wouldn't solve the problem of extra states like 'X', 'Z' or others.

I'm not sure any alternative has a better type for this. This isn't a typical problem that users of these exchange formats face. But I'd like to hear your ideas.

[daveshah1]

I don't actually think zero copy is that much of an issue here; as any consumer is likely to be building its own structures (e.g. hash tables of objects, etc) that will be much more expensive than the parse. But it's a nice to have.

The number one requirement is good cross platform support, if this is to become the recommended interchange format for nextpnr in the long term it needs to cause no issues for Windows or macOS.

As for parameters; I think one-byte-per-bit is reasonable as throwing away 'x's is not correct - particularly for memory initialisation these can have significance as parameters. Likewise it is important to disambiguate strings that look like bit vectors, and large bit vectors.

[q3k]

Comparison of language support between cap'n'proto and protobuf:

https://github.com/protocolbuffers/protobuf/blob/master/docs/third_party.md

https://capnproto.org/otherlang.html

Flatbuffers has probably the smallest language support base of the three.