Fix bmv2/json generation crashes for p4_16 code converted from p4_14 #124
Conversation
| result->emplace("name", nameFromAnnotation(st->annotations, st->name.name)); | ||
| result->emplace("id", nextId("header_types")); | ||
| auto fields = mkArrayField(result, "fields"); | ||
| void JsonConverter::pushFields(cstring prefix, const IR::Type_StructLike *st, |
There was a problem hiding this comment.
I don't think this works - the rest of the code won't handle correctly references to the inner struct. Consider:
struct S { bit x; }
struct T { S s1; S s2; }
T t;
t.s1 = t.s2;
I think that we need additional passes to properly rewrite the code that may access the inner structures.
I am also not sure this will properly handle arrays within structs.
There was a problem hiding this comment.
Yes, this just covers not crashing in the json generation -- it won't generate correct json for things like struct assignments that bmv2 can't handle (or more precisely, it will generate json with struct assignments that bmv2 will then reject). Actually handling struct (and tuple) assignment requires breaking them down into multiple assignments, which is a problem for tuples as we don't have a P4 syntax for accessing a field out of a tuple.
| field->append(8 - padding); | ||
| field->append(false); | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
I don't think you want to add padding except for the outer struct.
There was a problem hiding this comment.
When it's a struct of headers, each header needs to be individually padded out to bytes. When it's just metadata I don't think any padding is required, but it was being done anyway before.
| return name; | ||
| } | ||
|
|
||
| cstring JsonConverter::createJsonType(const IR::Type_Tuple *tt) { |
There was a problem hiding this comment.
Does this interact correctly with functions which expect a field_list? Field lists are compiled into tuples, which here are converted into structs.
There was a problem hiding this comment.
This should only be called for things that are being converted into bmv2 actions (where the bmv2 is expected to evaluate the expressions), which need to be structs as it doesn't understand tuples. Builtin things (extern methods) should not end calling this.
- refactor the code, moving repeated arguments into the JsonConverter object - support for Tuple types - support for nested structs - skips things related to checksums that were crashing (TODO)
object