From 2b40a17f8abba45f7d107c4d926b9901db586915 Mon Sep 17 00:00:00 2001 From: baldurk Date: Thu, 27 Jun 2024 12:04:08 +0100 Subject: [PATCH] Add packing rule for bitfield packing/straddling a la #pragma pack(1) --- docs/how/how_buffer_format.rst | 4 ++ qrenderdoc/Code/BufferFormatter.cpp | 103 +++++++++++++++++++++++++--- qrenderdoc/Code/QRDUtils.h | 7 ++ 3 files changed, 106 insertions(+), 8 deletions(-) diff --git a/docs/how/how_buffer_format.rst b/docs/how/how_buffer_format.rst index 7bec88911..a1659ea5a 100644 --- a/docs/how/how_buffer_format.rst +++ b/docs/how/how_buffer_format.rst @@ -154,6 +154,10 @@ The available packing properties are: * ``tight_arrays`` - If enabled, arrays elements are only aligned to the element size. If disabled, each array element is aligned to a 16-byte boundary. This is disabled for ``std140`` and ``cbuffer`` by default. * ``trailing_overlap`` - If enabled, elements can be placed in trailing padding from a previous element such as an array or struct. If disabled, each element's padding is reserved and the next element must come after the padding. This disabled for ``std140``, ``std430``, and ``structured`` by default. +Some additional properties are available to go beyond the normal packing rules: + +* ``tight_bitfield_packing`` - If enabled, bitfields consume bits tightly packed no matter what their base type or offset. This would allow a ``uint`` bitfield to be placed at 30 bits into a buffer and span more than 2 bits, crossing multiple uints. If disabled, padding is added to ensure each bitfield member remains within an instance of its base type. This is disabled by default, and is equivalent to ``#pragma pack(1)`` behaviour. + Annotations ----------- diff --git a/qrenderdoc/Code/BufferFormatter.cpp b/qrenderdoc/Code/BufferFormatter.cpp index 7fedf379f..bf9fd8632 100644 --- a/qrenderdoc/Code/BufferFormatter.cpp +++ b/qrenderdoc/Code/BufferFormatter.cpp @@ -813,6 +813,11 @@ ParsedFormat BufferFormatter::ParseFormatString(const QString &formatString, uin else if(packrule == lit("no_trailing_overlap")) pack.trailing_overlap = false; + else if(packrule == lit("tight_bitfield_packing")) + pack.tight_bitfield_packing = true; + else if(packrule == lit("no_tight_bitfield_packing")) + pack.tight_bitfield_packing = false; + else packrule = QString(); @@ -1925,9 +1930,9 @@ ParsedFormat BufferFormatter::ParseFormatString(const QString &formatString, uin if(el.bitFieldSize > 0) { - // we can use the arrayByteStride since this is a scalar so no vector/arrays, this is just the + // we can use the elAlignment since this is a scalar so no vector/arrays, this is just the // base size. It also works for enums as this is the byte size of the declared underlying type - const uint32_t elemScalarBitSize = el.type.arrayByteStride * 8; + const uint32_t elemScalarBitSize = elAlignment * 8; // bitfields can't be larger than the base type if(el.bitFieldSize > elemScalarBitSize) @@ -1972,11 +1977,22 @@ ParsedFormat BufferFormatter::ParseFormatString(const QString &formatString, uin // unsigned int c : 4; if(start + el.bitFieldSize > elemScalarBitSize) { - // align the offset up to where this bitfield needs to start - cur->offset += ((bitfieldCurPos + (elemScalarBitSize - 1)) / elemScalarBitSize) * - (elemScalarBitSize / 8); - // reset the current bitfield pos - bitfieldCurPos = 0; + if(pack.tight_bitfield_packing) + { + while(bitfieldCurPos >= 8) + { + bitfieldCurPos -= 8; + cur->offset++; + } + } + else + { + // align the offset up to where this bitfield needs to start + cur->offset += ((bitfieldCurPos + (elemScalarBitSize - 1)) / elemScalarBitSize) * + (elemScalarBitSize / 8); + // reset the current bitfield pos + bitfieldCurPos = 0; + } } // if there's no previous bitpacking, nothing much to do @@ -2062,7 +2078,8 @@ ParsedFormat BufferFormatter::ParseFormatString(const QString &formatString, uin fixed = root.structDef; uint32_t end = root.offset; - if(!fixed.type.members.isEmpty()) + if(!fixed.type.members.isEmpty() && + (!pack.tight_bitfield_packing || fixed.type.members.back().bitFieldSize == 0)) end = qMax( end, fixed.type.members.back().byteOffset + GetVarSizeAndTrail(fixed.type.members.back())); @@ -5003,6 +5020,38 @@ unsigned char highbit : 1; CHECK(parsed.fixed.type.members[4].name == "highbit"); CHECK(parsed.fixed.type.members[4].bitFieldOffset == 7); CHECK(parsed.fixed.type.members[4].bitFieldSize == 1); + + def = R"( +uint infirst : 16; +uint infirstalso : 14; +// this will be forced to the second uint, leaving 2 bits of trailing padding in the first +uint overflow : 20; +// this then also can't be packed into the second uint and will be packed into a third, leaving 12 bits of padding +uint inthird : 14; + +// result: total of 64 bits but packed into 3 uints due to padding +)"; + parsed = BufferFormatter::ParseFormatString(def, 0, true); + + CHECK(parsed.errors.isEmpty()); + REQUIRE(parsed.fixed.type.members.size() == 4); + CHECK(parsed.fixed.type.arrayByteStride == 12); + CHECK(parsed.fixed.type.members[0].name == "infirst"); + CHECK(parsed.fixed.type.members[0].byteOffset == 0); + CHECK(parsed.fixed.type.members[0].bitFieldOffset == 0); + CHECK(parsed.fixed.type.members[0].bitFieldSize == 16); + CHECK(parsed.fixed.type.members[1].name == "infirstalso"); + CHECK(parsed.fixed.type.members[1].byteOffset == 0); + CHECK(parsed.fixed.type.members[1].bitFieldOffset == 16); + CHECK(parsed.fixed.type.members[1].bitFieldSize == 14); + CHECK(parsed.fixed.type.members[2].name == "overflow"); + CHECK(parsed.fixed.type.members[2].byteOffset == 4); + CHECK(parsed.fixed.type.members[2].bitFieldOffset == 0); + CHECK(parsed.fixed.type.members[2].bitFieldSize == 20); + CHECK(parsed.fixed.type.members[3].name == "inthird"); + CHECK(parsed.fixed.type.members[3].byteOffset == 8); + CHECK(parsed.fixed.type.members[3].bitFieldOffset == 0); + CHECK(parsed.fixed.type.members[3].bitFieldSize == 14); }; SECTION("pointers") @@ -5789,6 +5838,44 @@ struct s CHECK(parsed.fixed.type.members[7].byteOffset == 165); // h }; + SECTION("Additional packing rules") + { + rdcstr def = R"( +#pack(tight_bitfield_packing) + +uint infirst : 16; +uint infirstalso : 14; +// this will span 2 bits in the first uint and 18 bits in the second +uint overflow : 20; +uint insecond : 14; +)"; + for(rdcstr ruleset : + {"", "#pack(c)", "#pack(scalar)", "#pack(std430)", "#pack(std140)", "#pack(cbuffer)"}) + { + parsed = BufferFormatter::ParseFormatString(ruleset + "\n" + def, 0, true); + + CHECK(parsed.errors.isEmpty()); + REQUIRE(parsed.fixed.type.members.size() == 4); + CHECK(parsed.fixed.type.arrayByteStride == 8); + CHECK(parsed.fixed.type.members[0].name == "infirst"); + CHECK(parsed.fixed.type.members[0].byteOffset == 0); + CHECK(parsed.fixed.type.members[0].bitFieldOffset == 0); + CHECK(parsed.fixed.type.members[0].bitFieldSize == 16); + CHECK(parsed.fixed.type.members[1].name == "infirstalso"); + CHECK(parsed.fixed.type.members[1].byteOffset == 0); + CHECK(parsed.fixed.type.members[1].bitFieldOffset == 16); + CHECK(parsed.fixed.type.members[1].bitFieldSize == 14); + CHECK(parsed.fixed.type.members[2].name == "overflow"); + CHECK(parsed.fixed.type.members[2].byteOffset == 3); + CHECK(parsed.fixed.type.members[2].bitFieldOffset == 6); + CHECK(parsed.fixed.type.members[2].bitFieldSize == 20); + CHECK(parsed.fixed.type.members[3].name == "insecond"); + CHECK(parsed.fixed.type.members[3].byteOffset == 6); + CHECK(parsed.fixed.type.members[3].bitFieldOffset == 2); + CHECK(parsed.fixed.type.members[3].bitFieldSize == 14); + } + }; + SECTION("Testing trailing member alignments") { rdcstr def = R"( diff --git a/qrenderdoc/Code/QRDUtils.h b/qrenderdoc/Code/QRDUtils.h index 9a3acd5c3..e22698129 100644 --- a/qrenderdoc/Code/QRDUtils.h +++ b/qrenderdoc/Code/QRDUtils.h @@ -107,6 +107,9 @@ struct Rules Rules() = default; Rules(APIConfig config) { + // no packing allows this by default, it is only enabled manually + tight_bitfield_packing = false; + // default to the most conservative packing ruleset switch(config) @@ -180,6 +183,10 @@ struct Rules // trailing padding and members after a struct are not packed in that padding). For arrays it does // not apply since C arrays are packed. bool trailing_overlap = false; + + // whether bitfields will allow themselves to straddle their base type, or be aligned to stay + // within it. Equivalent to #pragma pack(1) in C++ + bool tight_bitfield_packing = false; }; }; // namespace Packing