From 5f3dda9473be2cfec9248006c50a61d4812de754 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Kr=C3=B3lik?= <66667989+Damian-Nordic@users.noreply.github.com> Date: Wed, 4 Jan 2023 22:44:20 +0100 Subject: [PATCH] [tlv] Optimize code size of processing context tags (#24224) * [tlv] Encapsulate Tag encoding Make the internal representation of Tag private so that it is easier and safer to change it. Signed-off-by: Damian Krolik * [tlv] Optimize code size of processing context tags TLV tag is represented as uint64_t that encodes the following components: - 16-bit vendor ID - 16-bit profile number - 32-bit tag number Context tags, which account for vast majority of tag usage in the SDK, are encoded as having both vendor ID and profile number equal to 0xFFFF. Anonymous tags are encoded in the same way, but using 0xFFFFFFFF tag number. This is correct because vendor IDs higher than 0xFFF0 shall not be assigned to real manufacturers, but constructing 0xFF... constants in hundreds of places adds non-negligible overhead to the flash usage. Encode profile ID in the negated form internally to optimize the code size when using special tags. * Fix build * Code review Signed-off-by: Damian Krolik --- src/darwin/Framework/CHIP/MTRBaseDevice.mm | 7 +- src/lib/core/CHIPTLVReader.cpp | 6 +- src/lib/core/CHIPTLVTags.h | 152 +++++++++++++-------- src/lib/core/CHIPTLVWriter.cpp | 2 +- 4 files changed, 105 insertions(+), 62 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm index 7725c9ab6e1f1b..e8211845e91cae 100644 --- a/src/darwin/Framework/CHIP/MTRBaseDevice.mm +++ b/src/darwin/Framework/CHIP/MTRBaseDevice.mm @@ -1437,7 +1437,7 @@ + (id)CHIPEncodeAndDecodeNSObject:(id)object uint8_t buffer[1024]; writer.Init(buffer, sizeof(buffer)); - CHIP_ERROR error = originalData.Encode(writer, chip::TLV::Tag(1)); + CHIP_ERROR error = originalData.Encode(writer, chip::TLV::CommonTag(1)); if (error != CHIP_NO_ERROR) { MTR_LOG_ERROR("Error: Data encoding failed: %s", error.AsString()); return nil; @@ -1456,8 +1456,9 @@ + (id)CHIPEncodeAndDecodeNSObject:(id)object return nil; } __auto_type tag = reader.GetTag(); - if (tag != chip::TLV::Tag(1)) { - MTR_LOG_ERROR("Error: TLV reader did not read the tag correctly: %llu", tag.mVal); + if (tag != chip::TLV::CommonTag(1)) { + MTR_LOG_ERROR("Error: TLV reader did not read the tag correctly: %x.%u", chip::TLV::ProfileIdFromTag(tag), + chip::TLV::TagNumFromTag(tag)); return nil; } MTRDataValueDictionaryDecodableType decodedData; diff --git a/src/lib/core/CHIPTLVReader.cpp b/src/lib/core/CHIPTLVReader.cpp index 9a95a42779e9dc..5d175eb35b7108 100644 --- a/src/lib/core/CHIPTLVReader.cpp +++ b/src/lib/core/CHIPTLVReader.cpp @@ -749,7 +749,7 @@ CHIP_ERROR TLVReader::VerifyElement() } else { - if (mElemTag == UnknownImplicitTag) + if (mElemTag == UnknownImplicitTag()) return CHIP_ERROR_UNKNOWN_IMPLICIT_TLV_TAG; switch (mContainerType) { @@ -806,11 +806,11 @@ Tag TLVReader::ReadTag(TLVTagControl tagControl, const uint8_t *& p) const return CommonTag(LittleEndian::Read32(p)); case TLVTagControl::ImplicitProfile_2Bytes: if (ImplicitProfileId == kProfileIdNotSpecified) - return UnknownImplicitTag; + return UnknownImplicitTag(); return ProfileTag(ImplicitProfileId, LittleEndian::Read16(p)); case TLVTagControl::ImplicitProfile_4Bytes: if (ImplicitProfileId == kProfileIdNotSpecified) - return UnknownImplicitTag; + return UnknownImplicitTag(); return ProfileTag(ImplicitProfileId, LittleEndian::Read32(p)); case TLVTagControl::FullyQualified_6Bytes: vendorId = LittleEndian::Read16(p); diff --git a/src/lib/core/CHIPTLVTags.h b/src/lib/core/CHIPTLVTags.h index 12de1d9e3ef2e1..da8f91006fae48 100644 --- a/src/lib/core/CHIPTLVTags.h +++ b/src/lib/core/CHIPTLVTags.h @@ -29,12 +29,59 @@ namespace chip { namespace TLV { -struct Tag +class Tag { - explicit constexpr Tag(uint64_t val) : mVal(val) {} - Tag() {} +public: + enum SpecialTagNumber : uint32_t + { + kContextTagMaxNum = UINT8_MAX, + kAnonymousTagNum, + kUnknownImplicitTagNum + }; + + Tag() = default; + constexpr bool operator==(const Tag & other) const { return mVal == other.mVal; } constexpr bool operator!=(const Tag & other) const { return mVal != other.mVal; } + +private: + explicit constexpr Tag(uint64_t val) : mVal(val) {} + + friend constexpr Tag ProfileTag(uint32_t profileId, uint32_t tagNum); + friend constexpr Tag ProfileTag(uint16_t vendorId, uint16_t profileNum, uint32_t tagNum); + friend constexpr Tag ContextTag(uint8_t tagNum); + friend constexpr Tag CommonTag(uint32_t tagNum); + friend constexpr Tag AnonymousTag(); + friend constexpr Tag UnknownImplicitTag(); + + // The following friend functions could be Tag class methods, but it turns out in some cases + // they may not be inlined and then passing the tag by argument/value results in smaller code + // than passing it by 'this' pointer. This can be worked around by applying 'always_inline' + // function attribute, but friend functions are likely a more portable solution. + + friend constexpr uint32_t ProfileIdFromTag(Tag tag); + friend constexpr uint16_t VendorIdFromTag(Tag tag); + friend constexpr uint16_t ProfileNumFromTag(Tag tag); + friend constexpr uint32_t TagNumFromTag(Tag tag); + + friend constexpr bool IsProfileTag(Tag tag); + friend constexpr bool IsContextTag(Tag tag); + friend constexpr bool IsSpecialTag(Tag tag); + + static constexpr uint32_t kProfileIdShift = 32; + static constexpr uint32_t kVendorIdShift = 48; + static constexpr uint32_t kProfileNumShift = 32; + static constexpr uint32_t kSpecialTagProfileId = 0xFFFFFFFF; + + // The storage of the tag value uses the following encoding: + // + // 63 47 31 + // +-------------------------------+-------------------------------+----------------------------------------------+ + // | Vendor id (bitwise-negated) | Profile num (bitwise-negated) | Tag number | + // +-------------------------------+-------------------------------+----------------------------------------------+ + // + // Vendor id and profile number are bitwise-negated in order to optimize the code size when + // using context tags, the most commonly used tags in the SDK. uint64_t mVal; }; @@ -50,20 +97,6 @@ enum TLVCommonProfiles kCommonProfileId = 0 }; -// TODO: Move to private namespace -enum TLVTagFields -{ - kProfileIdMask = 0xFFFFFFFF00000000ULL, - kProfileNumMask = 0x0000FFFF00000000ULL, - kVendorIdMask = 0xFFFF000000000000ULL, - kProfileIdShift = 32, - kVendorIdShift = 48, - kProfileNumShift = 32, - kTagNumMask = 0x00000000FFFFFFFFULL, - kSpecialTagMarker = 0xFFFFFFFF00000000ULL, - kContextTagMaxNum = UINT8_MAX -}; - // TODO: Move to private namespace enum class TLVTagControl : uint8_t { @@ -99,9 +132,9 @@ enum * @param[in] tagNum The profile-specific tag number assigned to the tag. * @return A 64-bit integer representing the tag. */ -inline constexpr Tag ProfileTag(uint32_t profileId, uint32_t tagNum) +constexpr Tag ProfileTag(uint32_t profileId, uint32_t tagNum) { - return Tag(((static_cast(profileId)) << kProfileIdShift) | tagNum); + return Tag((static_cast(~profileId) << Tag::kProfileIdShift) | tagNum); } /** @@ -112,21 +145,22 @@ inline constexpr Tag ProfileTag(uint32_t profileId, uint32_t tagNum) * @param[in] tagNum The profile-specific tag number assigned to the tag. * @return A 64-bit integer representing the tag. */ -inline constexpr Tag ProfileTag(uint16_t vendorId, uint16_t profileNum, uint32_t tagNum) +constexpr Tag ProfileTag(uint16_t vendorId, uint16_t profileNum, uint32_t tagNum) { - return Tag(((static_cast(vendorId)) << kVendorIdShift) | ((static_cast(profileNum)) << kProfileNumShift) | - tagNum); + constexpr uint32_t kVendorIdShift = Tag::kVendorIdShift - Tag::kProfileIdShift; + + return ProfileTag((static_cast(vendorId) << kVendorIdShift) | profileNum, tagNum); } /** - * Generates the API representation for of context-specific TLV tag + * Generates the API representation of a context-specific TLV tag * * @param[in] tagNum The context-specific tag number assigned to the tag. * @return A 64-bit integer representing the tag. */ -inline constexpr Tag ContextTag(uint8_t tagNum) +constexpr Tag ContextTag(uint8_t tagNum) { - return Tag(kSpecialTagMarker | tagNum); + return ProfileTag(Tag::kSpecialTagProfileId, tagNum); } /** @@ -135,7 +169,7 @@ inline constexpr Tag ContextTag(uint8_t tagNum) * @param[in] tagNum The common profile tag number assigned to the tag. * @return A 64-bit integer representing the tag. */ -inline constexpr Tag CommonTag(uint32_t tagNum) +constexpr Tag CommonTag(uint32_t tagNum) { return ProfileTag(kCommonProfileId, tagNum); } @@ -143,12 +177,18 @@ inline constexpr Tag CommonTag(uint32_t tagNum) /** * A value signifying a TLV element that has no tag (i.e. an anonymous element). */ -inline constexpr Tag AnonymousTag() +constexpr Tag AnonymousTag() { - return Tag(kSpecialTagMarker | 0x00000000FFFFFFFFULL); + return ProfileTag(Tag::kSpecialTagProfileId, Tag::kAnonymousTagNum); +} + +/** + * An invalid tag that represents a TLV element decoding error due to unknown implicit profile id. + */ +constexpr Tag UnknownImplicitTag() +{ + return ProfileTag(Tag::kSpecialTagProfileId, Tag::kUnknownImplicitTagNum); } -// TODO: Move to private namespace -static constexpr Tag UnknownImplicitTag(kSpecialTagMarker | 0x00000000FFFFFFFEULL); /** * Returns the profile id from a TLV tag @@ -158,9 +198,24 @@ static constexpr Tag UnknownImplicitTag(kSpecialTagMarker | 0x00000000FFFFFFFEUL * @param[in] tag The API representation of a profile-specific TLV tag. * @return The profile id. */ -inline constexpr uint32_t ProfileIdFromTag(Tag tag) +constexpr uint32_t ProfileIdFromTag(Tag tag) +{ + return ~static_cast(tag.mVal >> Tag::kProfileIdShift); +} + +/** + * Returns the vendor id from a TLV tag + * + * @note The behavior of this function is undefined if the supplied tag is not a profile-specific tag. + * + * @param[in] tag The API representation of a profile-specific TLV tag. + * @return The associated vendor id. + */ +constexpr uint16_t VendorIdFromTag(Tag tag) { - return static_cast((tag.mVal & kProfileIdMask) >> kProfileIdShift); + constexpr uint32_t kVendorIdShift = Tag::kVendorIdShift - Tag::kProfileIdShift; + + return static_cast(ProfileIdFromTag(tag) >> kVendorIdShift); } /** @@ -171,9 +226,9 @@ inline constexpr uint32_t ProfileIdFromTag(Tag tag) * @param[in] tag The API representation of a profile-specific TLV tag. * @return The associated profile number. */ -inline constexpr uint16_t ProfileNumFromTag(Tag tag) +constexpr uint16_t ProfileNumFromTag(Tag tag) { - return static_cast((tag.mVal & kProfileNumMask) >> kProfileNumShift); + return static_cast(ProfileIdFromTag(tag)); } /** @@ -187,44 +242,31 @@ inline constexpr uint16_t ProfileNumFromTag(Tag tag) * @param[in] tag The API representation of a profile-specific or context-specific TLV tag. * @return The associated tag number. */ -inline constexpr uint32_t TagNumFromTag(Tag tag) -{ - return static_cast(tag.mVal & kTagNumMask); -} - -/** - * Returns the vendor id from a TLV tag - * - * @note The behavior of this function is undefined if the supplied tag is not a profile-specific tag. - * - * @param[in] tag The API representation of a profile-specific TLV tag. - * @return The associated vendor id. - */ -inline constexpr uint16_t VendorIdFromTag(Tag tag) +constexpr uint32_t TagNumFromTag(Tag tag) { - return static_cast((tag.mVal & kVendorIdMask) >> kVendorIdShift); + return static_cast(tag.mVal); } /** * Returns true of the supplied tag is a profile-specific tag. */ -inline constexpr bool IsProfileTag(Tag tag) +constexpr bool IsProfileTag(Tag tag) { - return (tag.mVal & kProfileIdMask) != kSpecialTagMarker; + return ProfileIdFromTag(tag) != Tag::kSpecialTagProfileId; } /** * Returns true if the supplied tag is a context-specific tag. */ -inline constexpr bool IsContextTag(Tag tag) +constexpr bool IsContextTag(Tag tag) { - return (tag.mVal & kProfileIdMask) == kSpecialTagMarker && TagNumFromTag(tag) <= kContextTagMaxNum; + return ProfileIdFromTag(tag) == Tag::kSpecialTagProfileId && TagNumFromTag(tag) <= Tag::kContextTagMaxNum; } // TODO: move to private namespace -inline constexpr bool IsSpecialTag(Tag tag) +constexpr bool IsSpecialTag(Tag tag) { - return (tag.mVal & kProfileIdMask) == kSpecialTagMarker; + return ProfileIdFromTag(tag) == Tag::kSpecialTagProfileId; } } // namespace TLV diff --git a/src/lib/core/CHIPTLVWriter.cpp b/src/lib/core/CHIPTLVWriter.cpp index 3f9743e23464f4..8a1eb7021edb66 100644 --- a/src/lib/core/CHIPTLVWriter.cpp +++ b/src/lib/core/CHIPTLVWriter.cpp @@ -631,7 +631,7 @@ CHIP_ERROR TLVWriter::WriteElementHead(TLVElementType elemType, Tag tag, uint64_ if (IsSpecialTag(tag)) { - if (tagNum <= kContextTagMaxNum) + if (tagNum <= Tag::kContextTagMaxNum) { if (mContainerType != kTLVType_Structure && mContainerType != kTLVType_List) return CHIP_ERROR_INVALID_TLV_TAG;