https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/68907
To get the number of children for a VectorType (i.e., a type declared with a `vector_size`/`ext_vector_type` attribute) LLDB previously did following calculation: 1. Get byte-size of the vector container from Clang (`getTypeInfo`). 2. Get byte-size of the element type we want to interpret the array as. (e.g., sometimes we want to interpret an `unsigned char vec[16]` as a `float32[]`). 3. `numChildren = containerSize / reinterpretedElementSize` However, for step 1, clang will return us the *aligned* container byte-size. So for a type such as `float __attribute__((ext_vector_type(3)))` (which is an array of 3 4-byte floats), clang will round up the bit-width of the array to `16`. (see [here](https://github.com/llvm/llvm-project/blob/ab6a66dbec61654d0962f6abf6d6c5b776937584/clang/lib/AST/ASTContext.cpp#L1987-L1992)) This means that for vectors where the size isn't a power-of-2, LLDB will miscalculate the number of elements. **Solution** This patch changes step 1 such that we calculate the container size as `numElementsInSource * byteSizeOfElement`. This mimicks the typical `sizeof(array) / sizeof(array[0])` calculation in C. >From 769cbd0a30f0903ff4b7263b7789fb28c49de1d6 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Thu, 12 Oct 2023 16:25:39 +0100 Subject: [PATCH 1/3] [lldb][DataFormatter][NFC] VectorType: remove redundant parameter to helper function --- lldb/source/DataFormatters/VectorType.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lldb/source/DataFormatters/VectorType.cpp b/lldb/source/DataFormatters/VectorType.cpp index 4afcfa2e8e490e0..7109708c1cc9605 100644 --- a/lldb/source/DataFormatters/VectorType.cpp +++ b/lldb/source/DataFormatters/VectorType.cpp @@ -169,14 +169,12 @@ static lldb::Format GetItemFormatForFormat(lldb::Format format, } } -static size_t CalculateNumChildren( - CompilerType container_type, CompilerType element_type, - lldb_private::ExecutionContextScope *exe_scope = - nullptr // does not matter here because all we trade in are basic types - ) { +static size_t CalculateNumChildren(CompilerType container_type, + CompilerType element_type) { std::optional<uint64_t> container_size = - container_type.GetByteSize(exe_scope); - std::optional<uint64_t> element_size = element_type.GetByteSize(exe_scope); + container_type.GetByteSize(/* exe_scope */ nullptr); + std::optional<uint64_t> element_size = + element_type.GetByteSize(/* exe_scope */ nullptr); if (container_size && element_size && *element_size) { if (*container_size % *element_size) >From c3eeef2d4737e407ebd7bc9620f6460e1f45c654 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Thu, 12 Oct 2023 16:39:52 +0100 Subject: [PATCH 2/3] [lldb][DataFormatter][NFC] VectorType: clean up and document helper function In preparation for changing this function I cleaned it up so the conditions under which the function returns what are clearer. --- lldb/source/DataFormatters/VectorType.cpp | 35 +++++++++++++++++++---- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/lldb/source/DataFormatters/VectorType.cpp b/lldb/source/DataFormatters/VectorType.cpp index 7109708c1cc9605..e574a1e10866d27 100644 --- a/lldb/source/DataFormatters/VectorType.cpp +++ b/lldb/source/DataFormatters/VectorType.cpp @@ -169,19 +169,42 @@ static lldb::Format GetItemFormatForFormat(lldb::Format format, } } +/// \brief Returns the number of elements of 'container_type' +/// as if its elements had type 'element_type'. +/// +/// For example, a container of type +/// `uint8_t __attribute__((vector_size(16)))` has 16 elements. +/// But calling `CalculateNumChildren` with an 'element_type' +/// of `float` (4-bytes) will return `4` because we are interpreting +/// the byte-array as a `float32[]`. +/// +/// \param[in] container_type The type of the container We +/// are calculating the children of. +/// +/// \param[in] element_type The type of elements we interpret +/// container_type to contain for the purposes of calculating +/// the number of children. +/// +/// If size of the container is not a multiple of 'element_type' +/// returns 0. +/// +/// On error, returns 0. static size_t CalculateNumChildren(CompilerType container_type, CompilerType element_type) { std::optional<uint64_t> container_size = container_type.GetByteSize(/* exe_scope */ nullptr); + if (!container_size) + return 0; + std::optional<uint64_t> element_size = element_type.GetByteSize(/* exe_scope */ nullptr); + if (!element_size || !*element_size) + return 0; - if (container_size && element_size && *element_size) { - if (*container_size % *element_size) - return 0; - return *container_size / *element_size; - } - return 0; + if (*container_size % *element_size) + return 0; + + return *container_size / *element_size; } namespace lldb_private { >From 9f80f21d9f26af2dc57025e704c41944773b6c25 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Thu, 12 Oct 2023 17:04:55 +0100 Subject: [PATCH 3/3] [lldb][DataFormatter] Fix GetNumChildren for VectorType formatter To get the number of children for a VectorType (i.e., a type declared with a `vector_size`/`ext_vector_type` attribute) LLDB previously did following calculation: 1. Get byte-size of the vector container from Clang (`getTypeInfo`). 2. Get byte-size of the element type we want to interpret the array as. (e.g., sometimes we want to interpret an `unsigned char vec[16]` as a `float32[]`). 3. `numChildren = containerSize / reinterpretedElementSize` However, for step 1, clang will return us the *aligned* container byte-size. So for a type such as `float __attribute__((ext_vector_type(3)))` (which is an array of 3 4-byte floats), clang will round up the bit-width of the array to `16`. (see [here](https://github.com/llvm/llvm-project/blob/ab6a66dbec61654d0962f6abf6d6c5b776937584/clang/lib/AST/ASTContext.cpp#L1987-L1992)) This means that for vectors where the size isn't a power-of-2, LLDB will miscalculate the number of elements. **Solution** This patch changes step 1 such that we calculate the container size as `numElementsInSource * byteSizeOfElement`. This mimicks the typical `sizeof(array) / sizeof(array[0])` calculation in C. --- lldb/source/DataFormatters/VectorType.cpp | 33 ++++++++++++------- .../vector-types/TestVectorTypesFormatting.py | 7 ++++ .../data-formatter/vector-types/main.cpp | 4 ++- 3 files changed, 31 insertions(+), 13 deletions(-) diff --git a/lldb/source/DataFormatters/VectorType.cpp b/lldb/source/DataFormatters/VectorType.cpp index e574a1e10866d27..93eea9507242a9a 100644 --- a/lldb/source/DataFormatters/VectorType.cpp +++ b/lldb/source/DataFormatters/VectorType.cpp @@ -169,8 +169,9 @@ static lldb::Format GetItemFormatForFormat(lldb::Format format, } } -/// \brief Returns the number of elements of 'container_type' -/// as if its elements had type 'element_type'. +/// \brief Returns the number of elements stored in a container +/// (with element type 'container_elem_type') as if it had elements +/// of type 'element_type'. /// /// For example, a container of type /// `uint8_t __attribute__((vector_size(16)))` has 16 elements. @@ -178,8 +179,11 @@ static lldb::Format GetItemFormatForFormat(lldb::Format format, /// of `float` (4-bytes) will return `4` because we are interpreting /// the byte-array as a `float32[]`. /// -/// \param[in] container_type The type of the container We -/// are calculating the children of. +/// \param[in] container_elem_type The type of the elements stored +/// in the container we are calculating the children of. +/// +/// \param[in] num_elements Number of 'container_elem_type's our +/// container stores. /// /// \param[in] element_type The type of elements we interpret /// container_type to contain for the purposes of calculating @@ -189,22 +193,25 @@ static lldb::Format GetItemFormatForFormat(lldb::Format format, /// returns 0. /// /// On error, returns 0. -static size_t CalculateNumChildren(CompilerType container_type, +static size_t CalculateNumChildren(CompilerType container_elem_type, + uint64_t num_elements, CompilerType element_type) { - std::optional<uint64_t> container_size = - container_type.GetByteSize(/* exe_scope */ nullptr); - if (!container_size) + std::optional<uint64_t> container_elem_size = + container_elem_type.GetByteSize(/* exe_scope */ nullptr); + if (!container_elem_size) return 0; + auto container_size = *container_elem_size * num_elements; + std::optional<uint64_t> element_size = element_type.GetByteSize(/* exe_scope */ nullptr); if (!element_size || !*element_size) return 0; - if (*container_size % *element_size) + if (container_size % *element_size) return 0; - return *container_size / *element_size; + return container_size / *element_size; } namespace lldb_private { @@ -242,11 +249,13 @@ class VectorTypeSyntheticFrontEnd : public SyntheticChildrenFrontEnd { m_parent_format = m_backend.GetFormat(); CompilerType parent_type(m_backend.GetCompilerType()); CompilerType element_type; - parent_type.IsVectorType(&element_type); + uint64_t num_elements; + parent_type.IsVectorType(&element_type, &num_elements); m_child_type = ::GetCompilerTypeForFormat( m_parent_format, element_type, parent_type.GetTypeSystem().GetSharedPointer()); - m_num_children = ::CalculateNumChildren(parent_type, m_child_type); + m_num_children = + ::CalculateNumChildren(element_type, num_elements, m_child_type); m_item_format = GetItemFormatForFormat(m_parent_format, m_child_type); return false; } diff --git a/lldb/test/API/functionalities/data-formatter/vector-types/TestVectorTypesFormatting.py b/lldb/test/API/functionalities/data-formatter/vector-types/TestVectorTypesFormatting.py index 4103d62878c70f3..1839c28aeb29fc6 100644 --- a/lldb/test/API/functionalities/data-formatter/vector-types/TestVectorTypesFormatting.py +++ b/lldb/test/API/functionalities/data-formatter/vector-types/TestVectorTypesFormatting.py @@ -86,3 +86,10 @@ def cleanup(): v.SetFormat(lldb.eFormatVectorOfFloat32) oldValueAgain = v.GetChildAtIndex(0).GetValue() self.assertEqual(oldValue, oldValueAgain, "same format but different values") + + # Test formatter for vector types whose size is not a power-of-2 + f3 = self.frame().FindVariable("f3") + self.assertEqual(f3.GetNumChildren(), 3) + self.assertEqual(f3.GetChildAtIndex(0).GetData().float[0], 1.25) + self.assertEqual(f3.GetChildAtIndex(1).GetData().float[0], 2.50) + self.assertEqual(f3.GetChildAtIndex(2).GetData().float[0], 2.50) diff --git a/lldb/test/API/functionalities/data-formatter/vector-types/main.cpp b/lldb/test/API/functionalities/data-formatter/vector-types/main.cpp index ef0a227560bc253..7f2309e776bc27e 100644 --- a/lldb/test/API/functionalities/data-formatter/vector-types/main.cpp +++ b/lldb/test/API/functionalities/data-formatter/vector-types/main.cpp @@ -1,8 +1,10 @@ typedef float float4 __attribute__((ext_vector_type(4))); -typedef unsigned char vec __attribute__((ext_vector_type(16))); +typedef unsigned char vec __attribute__((ext_vector_type(16))); +typedef float float3 __attribute__((ext_vector_type(3))); int main() { float4 f4 = {1.25, 1.25, 2.50, 2.50}; vec v = (vec)f4; + float3 f3 = f4.gba; return 0; // break here } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
