dpdk: Eliminate unused metadata fields #3096
Conversation
kamleshbhalui
force-pushed
the
remove_unused_metadata_fields
branch
from
12bb9fe
to
43cf763
Compare
backends/dpdk/dpdkAsmOpt.cpp
Outdated
| const IR::Node* RemoveUnusedMetadataFields::preorder(IR::DpdkAsmProgram *p) { | ||
| auto usedFieldsCollector = new CollectUsedMetadataField(); | ||
| usedFieldsCollector->setCalledBy(this); | ||
| p->apply(*usedFieldsCollector); |
There was a problem hiding this comment.
you can probably call prune() too after this, this visitor does not need to traverse p.
This pattern of allocating the visitor in the preorder makes the program just a bit harder to read, why not just put CollectUsedMetadataFields in the list of backend passes and pass a reference to it to RemoveUnusedMetadataFields (or a reference to the used_fields structure)?
Frankly, it's not a big difference, it up to you, but the other pattern is the one used almost everywhere. We do allocate visitors locally too, but that's usually when we want to apply them to inner nodes.
There was a problem hiding this comment.
Incorporated the suggestions.
| @@ -135,6 +135,27 @@ class RemoveLabelAfterLabel : public Transform { | |||
| } | |||
| }; | |||
|
|
|||
|
|
|||
| // This pass Collects all metadata struct member used in program | |||
| class CollectUsedMetadataField : public Inspector { | |||
There was a problem hiding this comment.
Alternatively, if this pass is used only within dpdkAsmOpt.cpp, it does not need to have a declaration in the header, it can be local to the file (within an empty namespace to avoid name clashes).
There was a problem hiding this comment.
Now it's used in backend.cpp
| class CollectUsedMetadataField : public Inspector { | ||
| ordered_set<cstring> used_fields; | ||
| public: | ||
| bool preorder(const IR::Member *m) override { |
There was a problem hiding this comment.
This is not necessarily a field of the metadata structure, but I guess it will include all the fields. So you may keep some extra fields here if they happen to show up in other structures.
There was a problem hiding this comment.
Now only insert if it's metadata's member.
kamleshbhalui
force-pushed
the
remove_unused_metadata_fields
branch
from
43cf763
to
8cee431
Compare
|
|
||
| const IR::Node* RemoveUnusedMetadataFields::preorder(IR::DpdkAsmProgram *p) { | ||
| IR::IndexedVector<IR::DpdkStructType> usedStruct; | ||
| bool isMetadataStruct = false; |
There was a problem hiding this comment.
I would call this "foundMetadataStruct".
Perhaps you can change it in a future PR.
The PSA and PNA architecture require a lot of architecture-specific meta-data fields, but most of them are not actually used by a typical P4 program.
If an architecture-specific meta-data field is defined in the P4 because it is required by the architecture (e.g. PSA or PNA), but this field is not used at all by the P4 program, then this field should not be carried into the output .spec file generated by the P4C-DPDK compiler.