[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/steakhal approved this pull request. https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/86411 >From b6ca6f0ef83d03e299d6ee9a8ed9b8044477914e Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:14:44 -0700 Subject: [PATCH 1/9] Update checkers.rst Modification of documentation to demonstrate utilization of AllowedPad within PaddingChecker, along with its use and effects on code analysis. --- clang/docs/analyzer/checkers.rst | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index fe211514914272..64b09bc6ecd1d8 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (PaddingChecker) +""" Check for excessively padded structs. +.. code-block:: objc + + struct TestStruct { + int data1; // 4 bytes + char data2; // 1 byte + char padding[27]; // 27 bytes of padding +}; // Total size: 32 bytes + + void TestFunction() { + struct TestStruct struct1; // Warning is generated due to excessive padding. +} + + // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. + + // AllowedPad: Default Value: 24 bytes + + Usage: `AllowedPad=` + .. _optin-portability-UnixAPI: optin.portability.UnixAPI >From 403115cd960653a3afe0491d2855d35d377d9312 Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:20:46 -0700 Subject: [PATCH 2/9] Update Checkers.td Changed NotDocumented to HasDocumentation for Padding Checker under performance checker. --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 686e5e99f4a62c..c0e4e9a70c2ef3 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -908,7 +908,7 @@ def PaddingChecker : Checker<"Padding">, "24", Released> ]>, - Documentation; + Documentation; } // end: "padding" >From cdef12fa6a6b6c4833bc2017dae9557ee7115c92 Mon Sep 17 00:00:00 2001 From: komalverma04 Date: Sun, 24 Mar 2024 20:39:56 +0530 Subject: [PATCH 3/9] Update checkers.rst Made Indentation consistent and added description on limitation of checker. --- clang/docs/analyzer/checkers.rst | 44 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 64b09bc6ecd1d8..85f3f33d68bc5d 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,27 +804,37 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding (PaddingChecker) -""" +optin.performance.Padding (C, C++, objC) + Check for excessively padded structs. -.. code-block:: objc - - struct TestStruct { - int data1; // 4 bytes - char data2; // 1 byte - char padding[27]; // 27 bytes of padding -}; // Total size: 32 bytes - - void TestFunction() { - struct TestStruct struct1; // Warning is generated due to excessive padding. -} - - // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. +This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance. + +.. code-block:: C + + #include + // #pragma pack(1) // Uncomment it to disable structure padding + struct TestStruct { + char data1; // 1 byte + char data2; // 1 byte + int data3; // 4 bytes + }; // Total size: 6 bytes + + int main () { + struct TestStruct struct1; + print("Structure size: %d",sizeof(struct1)); // Structure size: 8 + return 0; + } + +Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure; it increases the performance of the processor
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/86411 >From b6ca6f0ef83d03e299d6ee9a8ed9b8044477914e Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:14:44 -0700 Subject: [PATCH 1/8] Update checkers.rst Modification of documentation to demonstrate utilization of AllowedPad within PaddingChecker, along with its use and effects on code analysis. --- clang/docs/analyzer/checkers.rst | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index fe211514914272..64b09bc6ecd1d8 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (PaddingChecker) +""" Check for excessively padded structs. +.. code-block:: objc + + struct TestStruct { + int data1; // 4 bytes + char data2; // 1 byte + char padding[27]; // 27 bytes of padding +}; // Total size: 32 bytes + + void TestFunction() { + struct TestStruct struct1; // Warning is generated due to excessive padding. +} + + // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. + + // AllowedPad: Default Value: 24 bytes + + Usage: `AllowedPad=` + .. _optin-portability-UnixAPI: optin.portability.UnixAPI >From 403115cd960653a3afe0491d2855d35d377d9312 Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:20:46 -0700 Subject: [PATCH 2/8] Update Checkers.td Changed NotDocumented to HasDocumentation for Padding Checker under performance checker. --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 686e5e99f4a62c..c0e4e9a70c2ef3 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -908,7 +908,7 @@ def PaddingChecker : Checker<"Padding">, "24", Released> ]>, - Documentation; + Documentation; } // end: "padding" >From cdef12fa6a6b6c4833bc2017dae9557ee7115c92 Mon Sep 17 00:00:00 2001 From: komalverma04 Date: Sun, 24 Mar 2024 20:39:56 +0530 Subject: [PATCH 3/8] Update checkers.rst Made Indentation consistent and added description on limitation of checker. --- clang/docs/analyzer/checkers.rst | 44 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 64b09bc6ecd1d8..85f3f33d68bc5d 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,27 +804,37 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding (PaddingChecker) -""" +optin.performance.Padding (C, C++, objC) + Check for excessively padded structs. -.. code-block:: objc - - struct TestStruct { - int data1; // 4 bytes - char data2; // 1 byte - char padding[27]; // 27 bytes of padding -}; // Total size: 32 bytes - - void TestFunction() { - struct TestStruct struct1; // Warning is generated due to excessive padding. -} - - // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. +This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance. + +.. code-block:: C + + #include + // #pragma pack(1) // Uncomment it to disable structure padding + struct TestStruct { + char data1; // 1 byte + char data2; // 1 byte + int data3; // 4 bytes + }; // Total size: 6 bytes + + int main () { + struct TestStruct struct1; + print("Structure size: %d",sizeof(struct1)); // Structure size: 8 + return 0; + } + +Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure; it increases the performance of the processor
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
komalverma04 wrote: Okay, i completely agree with you. Thank you so much for helping. https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
@@ -804,10 +804,88 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (C, C++, ObjC) + Check for excessively padded structs. +This checker detects structs with excessive padding, which can lead to wasted +memory thus decreased performance by reducing the effectiveness of the +processor cache. Padding bytes are added by compilers to align data accesses +as some processors require data to be aligned to certain boundaries. On others, +unaligned data access are possible, but impose significantly larger latencies. + +To avoid padding bytes, the fields of a struct should be ordered by decreasing +by alignment. Usually, its easier to think of the ``sizeof`` of the fields, and +ordering the fields by ``sizeof`` would usually also lead to the same optimal +layout. + +In rare cases, one can use the ``#pragma pack(1)`` directive to enforce a packed +layout too, but it is discouraged and the reordering of fields should be +preferred. + +.. code-block:: cpp + + // warn: Excessive padding in 'struct NonOptimal' (35 padding bytes, where 3 is optimal) + struct NonOptimal { + char c1; + // 7 bytes of padding + std::int64_t big1; // 8 bytes + char c2; + // 7 bytes of padding + std::int64_t big2; // 8 bytes + char c3; + // 7 bytes of padding + std::int64_t big3; // 8 bytes + char c4; + // 7 bytes of padding + std::int64_t big4; // 8 bytes + char c5; + // 7 bytes of padding + }; + static_assert(sizeof(NonOptimal) == 4*8+5+5*7); + + // no-warning: The fields are nicely aligned to have the minimal amount of padding bytes. + struct Optimal { + std::int64_t big1; // 8 bytes + std::int64_t big2; // 8 bytes + std::int64_t big3; // 8 bytes + std::int64_t big4; // 8 bytes + char c1; + char c2; + char c3; + char c4; + char c5; + // 3 bytes of padding + }; + static_assert(sizeof(Optimal) == 4*8+5+3); + + // no-warning: Enforcing bitpacked representation. + // Access times will have signifficant overhead. Prefer reordering the fields instead. NagyDonat wrote: ```suggestion // no-warning: Bit packing representation is also accepted by this checker, but // it can significantly increase access times, so prefer reordering the fields. ``` https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
@@ -804,10 +804,88 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (C, C++, ObjC) + Check for excessively padded structs. +This checker detects structs with excessive padding, which can lead to wasted +memory thus decreased performance by reducing the effectiveness of the +processor cache. Padding bytes are added by compilers to align data accesses +as some processors require data to be aligned to certain boundaries. On others, +unaligned data access are possible, but impose significantly larger latencies. + +To avoid padding bytes, the fields of a struct should be ordered by decreasing +by alignment. Usually, its easier to think of the ``sizeof`` of the fields, and +ordering the fields by ``sizeof`` would usually also lead to the same optimal +layout. + +In rare cases, one can use the ``#pragma pack(1)`` directive to enforce a packed +layout too, but it is discouraged and the reordering of fields should be +preferred. NagyDonat wrote: ```suggestion In rare cases, one can use the ``#pragma pack(1)`` directive to enforce a packed layout too, but it can significantly increase the access times, so reordering the fields is usually a better solution. ``` https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/NagyDonat approved this pull request. Overall LGTM, I have minor inline suggestions that clarify the description of bitpacking (but the current state is also acceptable). https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
steakhal wrote: I figured, it might be easier to directly propose and apply some of my thoughts. Let me know if you like it. My goal was: - be specific what is the impact of not having the optimal layout: memory, cachemisses - have a single block of examples, demonstrating the bad, the good and an alternative good (bitpacking) case, while have no header dependencies - rework the example to work with the default configuration of the checker (this makes it more similar to the average user's diagnostics) - move all the advanced explanations to the end, as most users won't consider setting a custom threshold - make the definition of "when it warns" more accurate. I've checked [the code](https://github.com/llvm/llvm-project/blob/main/clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp#L103-L109) and it seems that the difference counts compared to the optimal layout. - discourage the use of `pragma pack` - add guide for fixing the issue, along with a practical hint. Given that it looks now as I would like it, I'll invite @NagyDonat for a round of review. Let me know if you like it too, or have suggestions for making it better. Lastly, now the generated html would look like this: ![image](https://github.com/llvm/llvm-project/assets/6280485/9ea66343-5713-4b7d-b365-e81ae1ebb39e) https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/86411 >From b6ca6f0ef83d03e299d6ee9a8ed9b8044477914e Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:14:44 -0700 Subject: [PATCH 1/7] Update checkers.rst Modification of documentation to demonstrate utilization of AllowedPad within PaddingChecker, along with its use and effects on code analysis. --- clang/docs/analyzer/checkers.rst | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index fe211514914272..64b09bc6ecd1d8 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (PaddingChecker) +""" Check for excessively padded structs. +.. code-block:: objc + + struct TestStruct { + int data1; // 4 bytes + char data2; // 1 byte + char padding[27]; // 27 bytes of padding +}; // Total size: 32 bytes + + void TestFunction() { + struct TestStruct struct1; // Warning is generated due to excessive padding. +} + + // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. + + // AllowedPad: Default Value: 24 bytes + + Usage: `AllowedPad=` + .. _optin-portability-UnixAPI: optin.portability.UnixAPI >From 403115cd960653a3afe0491d2855d35d377d9312 Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:20:46 -0700 Subject: [PATCH 2/7] Update Checkers.td Changed NotDocumented to HasDocumentation for Padding Checker under performance checker. --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 686e5e99f4a62c..c0e4e9a70c2ef3 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -908,7 +908,7 @@ def PaddingChecker : Checker<"Padding">, "24", Released> ]>, - Documentation; + Documentation; } // end: "padding" >From cdef12fa6a6b6c4833bc2017dae9557ee7115c92 Mon Sep 17 00:00:00 2001 From: komalverma04 Date: Sun, 24 Mar 2024 20:39:56 +0530 Subject: [PATCH 3/7] Update checkers.rst Made Indentation consistent and added description on limitation of checker. --- clang/docs/analyzer/checkers.rst | 44 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 64b09bc6ecd1d8..85f3f33d68bc5d 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,27 +804,37 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding (PaddingChecker) -""" +optin.performance.Padding (C, C++, objC) + Check for excessively padded structs. -.. code-block:: objc - - struct TestStruct { - int data1; // 4 bytes - char data2; // 1 byte - char padding[27]; // 27 bytes of padding -}; // Total size: 32 bytes - - void TestFunction() { - struct TestStruct struct1; // Warning is generated due to excessive padding. -} - - // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. +This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance. + +.. code-block:: C + + #include + // #pragma pack(1) // Uncomment it to disable structure padding + struct TestStruct { + char data1; // 1 byte + char data2; // 1 byte + int data3; // 4 bytes + }; // Total size: 6 bytes + + int main () { + struct TestStruct struct1; + print("Structure size: %d",sizeof(struct1)); // Structure size: 8 + return 0; + } + +Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure; it increases the performance of the processor
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/komalverma04 edited https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
@@ -804,10 +804,38 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (C, C++, ObjC) + Check for excessively padded structs. +This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance. + +.. code-block:: c + + #include + // #pragma pack(1) // Uncomment it to disable structure padding + struct TestStruct { + char data1; // 1 byte + char data2; // 1 byte + int data3; // 4 bytes + }; // Total size: 6 bytes + + int main () { + struct TestStruct struct1; + printf("Structure size: %d",sizeof(struct1)); // Structure size: 8 + return 0; + } + +Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure, it increases the performance of the processor but at the penalty of memory. +Padding can be disabled by using the pragma directive. +Padding can also be decreased by putting data members of the structure in descending order of their size. + +Reports are only generated if the excessive padding exceeds ``AllowedPad`` in bytes. AllowedPad is the threshold value of padding. + +AllowedPad Option: +- Default Value: 24 bytes +- Usage: ``AllowedPad=`` komalverma04 wrote: @steakhal @AtariDreams Please review it once. Thank you. https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/komalverma04 updated https://github.com/llvm/llvm-project/pull/86411 >From b6ca6f0ef83d03e299d6ee9a8ed9b8044477914e Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:14:44 -0700 Subject: [PATCH 1/6] Update checkers.rst Modification of documentation to demonstrate utilization of AllowedPad within PaddingChecker, along with its use and effects on code analysis. --- clang/docs/analyzer/checkers.rst | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index fe211514914272..64b09bc6ecd1d8 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (PaddingChecker) +""" Check for excessively padded structs. +.. code-block:: objc + + struct TestStruct { + int data1; // 4 bytes + char data2; // 1 byte + char padding[27]; // 27 bytes of padding +}; // Total size: 32 bytes + + void TestFunction() { + struct TestStruct struct1; // Warning is generated due to excessive padding. +} + + // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. + + // AllowedPad: Default Value: 24 bytes + + Usage: `AllowedPad=` + .. _optin-portability-UnixAPI: optin.portability.UnixAPI >From 403115cd960653a3afe0491d2855d35d377d9312 Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:20:46 -0700 Subject: [PATCH 2/6] Update Checkers.td Changed NotDocumented to HasDocumentation for Padding Checker under performance checker. --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 686e5e99f4a62c..c0e4e9a70c2ef3 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -908,7 +908,7 @@ def PaddingChecker : Checker<"Padding">, "24", Released> ]>, - Documentation; + Documentation; } // end: "padding" >From cdef12fa6a6b6c4833bc2017dae9557ee7115c92 Mon Sep 17 00:00:00 2001 From: komalverma04 Date: Sun, 24 Mar 2024 20:39:56 +0530 Subject: [PATCH 3/6] Update checkers.rst Made Indentation consistent and added description on limitation of checker. --- clang/docs/analyzer/checkers.rst | 44 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 64b09bc6ecd1d8..85f3f33d68bc5d 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,27 +804,37 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding (PaddingChecker) -""" +optin.performance.Padding (C, C++, objC) + Check for excessively padded structs. -.. code-block:: objc - - struct TestStruct { - int data1; // 4 bytes - char data2; // 1 byte - char padding[27]; // 27 bytes of padding -}; // Total size: 32 bytes - - void TestFunction() { - struct TestStruct struct1; // Warning is generated due to excessive padding. -} - - // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. +This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance. + +.. code-block:: C + + #include + // #pragma pack(1) // Uncomment it to disable structure padding + struct TestStruct { + char data1; // 1 byte + char data2; // 1 byte + int data3; // 4 bytes + }; // Total size: 6 bytes + + int main () { + struct TestStruct struct1; + print("Structure size: %d",sizeof(struct1)); // Structure size: 8 + return 0; + } + +Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure; it increases the performance of the
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/komalverma04 edited https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
@@ -804,10 +804,38 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (C, C++, ObjC) + Check for excessively padded structs. +This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance. + +.. code-block:: c + + #include + // #pragma pack(1) // Uncomment it to disable structure padding + struct TestStruct { + char data1; // 1 byte + char data2; // 1 byte + int data3; // 4 bytes + }; // Total size: 6 bytes + + int main () { + struct TestStruct struct1; + printf("Structure size: %d",sizeof(struct1)); // Structure size: 8 + return 0; + } + +Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure, it increases the performance of the processor but at the penalty of memory. +Padding can be disabled by using the pragma directive. +Padding can also be decreased by putting data members of the structure in descending order of their size. + +Reports are only generated if the excessive padding exceeds ``AllowedPad`` in bytes. AllowedPad is the threshold value of padding. komalverma04 wrote: @steakhal, if fields are in optimal order that is widest datatype first,still If the padding between two fields exceeds this value, the analyzer will issue a warning indicating that there's excessive padding within the structure. Okay, so i will write it in the documentation in described form. Please correct me if i am wrong https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
@@ -804,10 +804,38 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (C, C++, ObjC) + Check for excessively padded structs. +This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance. + +.. code-block:: c + + #include + // #pragma pack(1) // Uncomment it to disable structure padding + struct TestStruct { + char data1; // 1 byte + char data2; // 1 byte + int data3; // 4 bytes + }; // Total size: 6 bytes + + int main () { + struct TestStruct struct1; + printf("Structure size: %d",sizeof(struct1)); // Structure size: 8 + return 0; + } + +Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure, it increases the performance of the processor but at the penalty of memory. +Padding can be disabled by using the pragma directive. +Padding can also be decreased by putting data members of the structure in descending order of their size. + +Reports are only generated if the excessive padding exceeds ``AllowedPad`` in bytes. AllowedPad is the threshold value of padding. komalverma04 wrote: I will write everything in described form. https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
@@ -804,10 +804,38 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (C, C++, ObjC) + Check for excessively padded structs. +This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance. + +.. code-block:: c + + #include + // #pragma pack(1) // Uncomment it to disable structure padding + struct TestStruct { + char data1; // 1 byte + char data2; // 1 byte + int data3; // 4 bytes + }; // Total size: 6 bytes + + int main () { + struct TestStruct struct1; + printf("Structure size: %d",sizeof(struct1)); // Structure size: 8 + return 0; + } + +Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure, it increases the performance of the processor but at the penalty of memory. +Padding can be disabled by using the pragma directive. +Padding can also be decreased by putting data members of the structure in descending order of their size. + +Reports are only generated if the excessive padding exceeds ``AllowedPad`` in bytes. AllowedPad is the threshold value of padding. + +AllowedPad Option: +- Default Value: 24 bytes +- Usage: ``AllowedPad=`` komalverma04 wrote: Okay, i will do the changes as you said. https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
@@ -804,10 +804,38 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (C, C++, ObjC) + Check for excessively padded structs. +This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance. + +.. code-block:: c + + #include + // #pragma pack(1) // Uncomment it to disable structure padding + struct TestStruct { + char data1; // 1 byte + char data2; // 1 byte + int data3; // 4 bytes steakhal wrote: We usually indent code by 2 spaces in this `checkers.rst` file. Same for the `main` function body. Also avoid trailing spaces in the empty lines. https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/steakhal requested changes to this pull request. The grammar seems good. I have further concerns inline. https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
@@ -804,10 +804,38 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (C, C++, ObjC) + Check for excessively padded structs. +This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance. + +.. code-block:: c + + #include + // #pragma pack(1) // Uncomment it to disable structure padding + struct TestStruct { + char data1; // 1 byte + char data2; // 1 byte + int data3; // 4 bytes + }; // Total size: 6 bytes + + int main () { + struct TestStruct struct1; + printf("Structure size: %d",sizeof(struct1)); // Structure size: 8 + return 0; + } + +Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure, it increases the performance of the processor but at the penalty of memory. +Padding can be disabled by using the pragma directive. +Padding can also be decreased by putting data members of the structure in descending order of their size. + +Reports are only generated if the excessive padding exceeds ``AllowedPad`` in bytes. AllowedPad is the threshold value of padding. + +AllowedPad Option: +- Default Value: 24 bytes +- Usage: ``AllowedPad=`` steakhal wrote: I think you could just drop this, and just mention the default value in the previous paragraph. Maybe add a note like: > To enable set this option from the command-line, pass the `-analyzer-config > optin.performance.Padding:AllowedPad=24` option. https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
@@ -804,10 +804,38 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (C, C++, ObjC) + Check for excessively padded structs. +This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance. + +.. code-block:: c + + #include + // #pragma pack(1) // Uncomment it to disable structure padding + struct TestStruct { steakhal wrote: My problem with this `TestStruct` is that there is no way you could get rid of the two bytes padding using standard C++. Maybe a better example would be if one could reorder the fields to actually benefit from the better layout. As a rule of thump people should sort fields decreasing by alignment (or sizeof) to achieve the best layout. (maybe this technique could be also mentioned in the docs). Lastly, this code example does not raise an issue of this checker, so it would be better to showcase a bug that we actually report. Aside from that, we could also have a `OptimalStruct` that applies the technique I mentioned and demonstrates that the issue goes away. https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
@@ -804,10 +804,38 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (C, C++, ObjC) + Check for excessively padded structs. +This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance. + +.. code-block:: c + + #include + // #pragma pack(1) // Uncomment it to disable structure padding + struct TestStruct { + char data1; // 1 byte + char data2; // 1 byte + int data3; // 4 bytes + }; // Total size: 6 bytes + + int main () { + struct TestStruct struct1; + printf("Structure size: %d",sizeof(struct1)); // Structure size: 8 + return 0; + } + +Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure, it increases the performance of the processor but at the penalty of memory. +Padding can be disabled by using the pragma directive. +Padding can also be decreased by putting data members of the structure in descending order of their size. + +Reports are only generated if the excessive padding exceeds ``AllowedPad`` in bytes. AllowedPad is the threshold value of padding. steakhal wrote: The `AllowedPad is the threshold value of padding.` part is not clear to me. Does it apply if a single padding between two fields is larger than this, or the total padding within the struct? Is the trailing padding, or padding between the fields included even if there is no better way ordering the fields? In other words, would I have a warning if my fields are in the optimal order, but the padding between two fields are larger than `AllowedPad`? https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
komalverma04 wrote: @AtariDreams Thank you for pointing out. I apologies for that mistake. Please review it, if there are more changes. https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/komalverma04 updated https://github.com/llvm/llvm-project/pull/86411 >From b6ca6f0ef83d03e299d6ee9a8ed9b8044477914e Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:14:44 -0700 Subject: [PATCH 1/5] Update checkers.rst Modification of documentation to demonstrate utilization of AllowedPad within PaddingChecker, along with its use and effects on code analysis. --- clang/docs/analyzer/checkers.rst | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index fe211514914272..64b09bc6ecd1d8 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (PaddingChecker) +""" Check for excessively padded structs. +.. code-block:: objc + + struct TestStruct { + int data1; // 4 bytes + char data2; // 1 byte + char padding[27]; // 27 bytes of padding +}; // Total size: 32 bytes + + void TestFunction() { + struct TestStruct struct1; // Warning is generated due to excessive padding. +} + + // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. + + // AllowedPad: Default Value: 24 bytes + + Usage: `AllowedPad=` + .. _optin-portability-UnixAPI: optin.portability.UnixAPI >From 403115cd960653a3afe0491d2855d35d377d9312 Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:20:46 -0700 Subject: [PATCH 2/5] Update Checkers.td Changed NotDocumented to HasDocumentation for Padding Checker under performance checker. --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 686e5e99f4a62c..c0e4e9a70c2ef3 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -908,7 +908,7 @@ def PaddingChecker : Checker<"Padding">, "24", Released> ]>, - Documentation; + Documentation; } // end: "padding" >From cdef12fa6a6b6c4833bc2017dae9557ee7115c92 Mon Sep 17 00:00:00 2001 From: komalverma04 Date: Sun, 24 Mar 2024 20:39:56 +0530 Subject: [PATCH 3/5] Update checkers.rst Made Indentation consistent and added description on limitation of checker. --- clang/docs/analyzer/checkers.rst | 44 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 64b09bc6ecd1d8..85f3f33d68bc5d 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,27 +804,37 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding (PaddingChecker) -""" +optin.performance.Padding (C, C++, objC) + Check for excessively padded structs. -.. code-block:: objc - - struct TestStruct { - int data1; // 4 bytes - char data2; // 1 byte - char padding[27]; // 27 bytes of padding -}; // Total size: 32 bytes - - void TestFunction() { - struct TestStruct struct1; // Warning is generated due to excessive padding. -} - - // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. +This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance. + +.. code-block:: C + + #include + // #pragma pack(1) // Uncomment it to disable structure padding + struct TestStruct { + char data1; // 1 byte + char data2; // 1 byte + int data3; // 4 bytes + }; // Total size: 6 bytes + + int main () { + struct TestStruct struct1; + print("Structure size: %d",sizeof(struct1)); // Structure size: 8 + return 0; + } + +Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure; it increases the performance of the
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/AtariDreams requested changes to this pull request. Capitalize the O in ObjC https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/komalverma04 updated https://github.com/llvm/llvm-project/pull/86411 >From b6ca6f0ef83d03e299d6ee9a8ed9b8044477914e Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:14:44 -0700 Subject: [PATCH 1/4] Update checkers.rst Modification of documentation to demonstrate utilization of AllowedPad within PaddingChecker, along with its use and effects on code analysis. --- clang/docs/analyzer/checkers.rst | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index fe211514914272..64b09bc6ecd1d8 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (PaddingChecker) +""" Check for excessively padded structs. +.. code-block:: objc + + struct TestStruct { + int data1; // 4 bytes + char data2; // 1 byte + char padding[27]; // 27 bytes of padding +}; // Total size: 32 bytes + + void TestFunction() { + struct TestStruct struct1; // Warning is generated due to excessive padding. +} + + // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. + + // AllowedPad: Default Value: 24 bytes + + Usage: `AllowedPad=` + .. _optin-portability-UnixAPI: optin.portability.UnixAPI >From 403115cd960653a3afe0491d2855d35d377d9312 Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:20:46 -0700 Subject: [PATCH 2/4] Update Checkers.td Changed NotDocumented to HasDocumentation for Padding Checker under performance checker. --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 686e5e99f4a62c..c0e4e9a70c2ef3 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -908,7 +908,7 @@ def PaddingChecker : Checker<"Padding">, "24", Released> ]>, - Documentation; + Documentation; } // end: "padding" >From cdef12fa6a6b6c4833bc2017dae9557ee7115c92 Mon Sep 17 00:00:00 2001 From: komalverma04 Date: Sun, 24 Mar 2024 20:39:56 +0530 Subject: [PATCH 3/4] Update checkers.rst Made Indentation consistent and added description on limitation of checker. --- clang/docs/analyzer/checkers.rst | 44 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 64b09bc6ecd1d8..85f3f33d68bc5d 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,27 +804,37 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding (PaddingChecker) -""" +optin.performance.Padding (C, C++, objC) + Check for excessively padded structs. -.. code-block:: objc - - struct TestStruct { - int data1; // 4 bytes - char data2; // 1 byte - char padding[27]; // 27 bytes of padding -}; // Total size: 32 bytes - - void TestFunction() { - struct TestStruct struct1; // Warning is generated due to excessive padding. -} - - // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. +This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance. + +.. code-block:: C + + #include + // #pragma pack(1) // Uncomment it to disable structure padding + struct TestStruct { + char data1; // 1 byte + char data2; // 1 byte + int data3; // 4 bytes + }; // Total size: 6 bytes + + int main () { + struct TestStruct struct1; + print("Structure size: %d",sizeof(struct1)); // Structure size: 8 + return 0; + } + +Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure; it increases the performance of the
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/komalverma04 updated https://github.com/llvm/llvm-project/pull/86411 >From b6ca6f0ef83d03e299d6ee9a8ed9b8044477914e Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:14:44 -0700 Subject: [PATCH 1/3] Update checkers.rst Modification of documentation to demonstrate utilization of AllowedPad within PaddingChecker, along with its use and effects on code analysis. --- clang/docs/analyzer/checkers.rst | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index fe211514914272..64b09bc6ecd1d8 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (PaddingChecker) +""" Check for excessively padded structs. +.. code-block:: objc + + struct TestStruct { + int data1; // 4 bytes + char data2; // 1 byte + char padding[27]; // 27 bytes of padding +}; // Total size: 32 bytes + + void TestFunction() { + struct TestStruct struct1; // Warning is generated due to excessive padding. +} + + // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. + + // AllowedPad: Default Value: 24 bytes + + Usage: `AllowedPad=` + .. _optin-portability-UnixAPI: optin.portability.UnixAPI >From 403115cd960653a3afe0491d2855d35d377d9312 Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:20:46 -0700 Subject: [PATCH 2/3] Update Checkers.td Changed NotDocumented to HasDocumentation for Padding Checker under performance checker. --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 686e5e99f4a62c..c0e4e9a70c2ef3 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -908,7 +908,7 @@ def PaddingChecker : Checker<"Padding">, "24", Released> ]>, - Documentation; + Documentation; } // end: "padding" >From cdef12fa6a6b6c4833bc2017dae9557ee7115c92 Mon Sep 17 00:00:00 2001 From: komalverma04 Date: Sun, 24 Mar 2024 20:39:56 +0530 Subject: [PATCH 3/3] Update checkers.rst Made Indentation consistent and added description on limitation of checker. --- clang/docs/analyzer/checkers.rst | 44 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 64b09bc6ecd1d8..85f3f33d68bc5d 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,27 +804,37 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding (PaddingChecker) -""" +optin.performance.Padding (C, C++, objC) + Check for excessively padded structs. -.. code-block:: objc - - struct TestStruct { - int data1; // 4 bytes - char data2; // 1 byte - char padding[27]; // 27 bytes of padding -}; // Total size: 32 bytes - - void TestFunction() { - struct TestStruct struct1; // Warning is generated due to excessive padding. -} - - // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. +This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance. + +.. code-block:: C + + #include + // #pragma pack(1) // Uncomment it to disable structure padding + struct TestStruct { + char data1; // 1 byte + char data2; // 1 byte + int data3; // 4 bytes + }; // Total size: 6 bytes + + int main () { + struct TestStruct struct1; + print("Structure size: %d",sizeof(struct1)); // Structure size: 8 + return 0; + } + +Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure; it increases the performance of the
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
@@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (PaddingChecker) komalverma04 wrote: Thank you so much for your valuable feedbacks i will update it as you said. https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
@@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (PaddingChecker) +""" Check for excessively padded structs. +.. code-block:: objc + + struct TestStruct { + int data1; // 4 bytes + char data2; // 1 byte + char padding[27]; // 27 bytes of padding steakhal wrote: This is a field, thus it shouldn't be counted as padding. https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
@@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (PaddingChecker) +""" Check for excessively padded structs. +.. code-block:: objc + + struct TestStruct { + int data1; // 4 bytes + char data2; // 1 byte + char padding[27]; // 27 bytes of padding +}; // Total size: 32 bytes + + void TestFunction() { + struct TestStruct struct1; // Warning is generated due to excessive padding. +} + + // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. + + // AllowedPad: Default Value: 24 bytes + + Usage: `AllowedPad=` steakhal wrote: In other places we use double backticks. Why do you use one where? https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
@@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (PaddingChecker) steakhal wrote: It should be the languages within the parens, that the checker supports. I think this checker applies for C, C++ and probably even for ObjC as well. https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
@@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (PaddingChecker) +""" steakhal wrote: This line should be aligned with the line above. https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
@@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (PaddingChecker) +""" Check for excessively padded structs. +.. code-block:: objc steakhal wrote: This code is rather c. https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/steakhal requested changes to this pull request. The docs should also explain why is a problem to have padding bytes, and how to fix it. How does the generated html look like after your change for this file? (Have a look at my commits to this or to the releasenotes file for clang to see what commands to run to generate it) Does the checker have any limitations? Why is it even implemented in CSA? https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
@@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (PaddingChecker) +""" Check for excessively padded structs. +.. code-block:: objc + + struct TestStruct { + int data1; // 4 bytes + char data2; // 1 byte + char padding[27]; // 27 bytes of padding +}; // Total size: 32 bytes + + void TestFunction() { steakhal wrote: All lines in the example block should be indented nicely in a consistent manner. As of now, the indentation level varies. https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
github-actions[bot] wrote: ⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo. Please turn off [Keep my email addresses private](https://github.com/settings/emails) setting in your account. See [LLVM Discourse](https://discourse.llvm.org/t/hidden-emails-on-github-should-we-do-something-about-it) for more information. https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
llvmbot wrote: @llvm/pr-subscribers-clang Author: None (komalverma04) Changes # Added documentation for optin.performance.Padding. - Performance package has `PaddingChecker` checker. - It checks for excessively padded structs. - It has one option that is `AllowedPad`, an integer option. - It's default value is 24 bytes. - Reports are generated when padding exceeds `AllowedPad`. ## Purpose The purpose of this pull request is to improve the clarity and completeness of the documentation for PaddingChecker in the optin.performance.Padding checker. Closes #73675 --- Full diff: https://github.com/llvm/llvm-project/pull/86411.diff 2 Files Affected: - (modified) clang/docs/analyzer/checkers.rst (+20-2) - (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+1-1) ``diff diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index fe211514914272..64b09bc6ecd1d8 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (PaddingChecker) +""" Check for excessively padded structs. +.. code-block:: objc + + struct TestStruct { + int data1; // 4 bytes + char data2; // 1 byte + char padding[27]; // 27 bytes of padding +}; // Total size: 32 bytes + + void TestFunction() { + struct TestStruct struct1; // Warning is generated due to excessive padding. +} + + // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. + + // AllowedPad: Default Value: 24 bytes + + Usage: `AllowedPad=` + .. _optin-portability-UnixAPI: optin.portability.UnixAPI diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 686e5e99f4a62c..c0e4e9a70c2ef3 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -908,7 +908,7 @@ def PaddingChecker : Checker<"Padding">, "24", Released> ]>, - Documentation; + Documentation; } // end: "padding" `` https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/komalverma04 created https://github.com/llvm/llvm-project/pull/86411 # Added documentation for optin.performance.Padding. - Performance package has `PaddingChecker` checker. - It checks for excessively padded structs. - It has one option that is `AllowedPad`, an integer option. - It's default value is 24 bytes. - Reports are generated when padding exceeds `AllowedPad`. ## Purpose The purpose of this pull request is to improve the clarity and completeness of the documentation for PaddingChecker in the optin.performance.Padding checker. Closes #73675 >From b6ca6f0ef83d03e299d6ee9a8ed9b8044477914e Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:14:44 -0700 Subject: [PATCH 1/2] Update checkers.rst Modification of documentation to demonstrate utilization of AllowedPad within PaddingChecker, along with its use and effects on code analysis. --- clang/docs/analyzer/checkers.rst | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index fe211514914272..64b09bc6ecd1d8 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (PaddingChecker) +""" Check for excessively padded structs. +.. code-block:: objc + + struct TestStruct { + int data1; // 4 bytes + char data2; // 1 byte + char padding[27]; // 27 bytes of padding +}; // Total size: 32 bytes + + void TestFunction() { + struct TestStruct struct1; // Warning is generated due to excessive padding. +} + + // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. + + // AllowedPad: Default Value: 24 bytes + + Usage: `AllowedPad=` + .. _optin-portability-UnixAPI: optin.portability.UnixAPI >From 403115cd960653a3afe0491d2855d35d377d9312 Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:20:46 -0700 Subject: [PATCH 2/2] Update Checkers.td Changed NotDocumented to HasDocumentation for Padding Checker under performance checker. --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 686e5e99f4a62c..c0e4e9a70c2ef3 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -908,7 +908,7 @@ def PaddingChecker : Checker<"Padding">, "24", Released> ]>, - Documentation; + Documentation; } // end: "padding" ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits